Skip to content

Commit d9964c1

Browse files
authored
feat: add dblint with splinter (#631)
as simple as it gets for now. will wire up pglinter next and then check if we can refactor linters to share even more fixed the generated documentation too, which makes up for most of the changes.
1 parent 1763d52 commit d9964c1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+1431
-162
lines changed

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/pgls_cli/Cargo.toml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,10 @@ mimalloc = "0.1.43"
5252
tikv-jemallocator = "0.6.0"
5353

5454
[dev-dependencies]
55-
assert_cmd = "2.0.16"
56-
insta = { workspace = true, features = ["yaml"] }
55+
assert_cmd = "2.0.16"
56+
insta = { workspace = true, features = ["yaml"] }
57+
pgls_test_utils = { workspace = true }
58+
sqlx = { workspace = true }
5759

5860
[lib]
5961
doctest = false

crates/pgls_cli/src/commands/dblint.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::time::Instant;
33
use crate::cli_options::CliOptions;
44
use crate::reporter::Report;
55
use crate::{CliDiagnostic, CliSession, VcsIntegration};
6+
use pgls_analyse::RuleCategoriesBuilder;
67
use pgls_configuration::PartialConfiguration;
78
use pgls_diagnostics::Error;
89
use pgls_workspace::features::diagnostics::{PullDatabaseDiagnosticsParams, PullDiagnosticsResult};
@@ -24,10 +25,17 @@ pub fn dblint(
2425

2526
let start = Instant::now();
2627

28+
let params = PullDatabaseDiagnosticsParams {
29+
categories: RuleCategoriesBuilder::default().all().build(),
30+
max_diagnostics,
31+
only: Vec::new(), // Uses configuration settings
32+
skip: Vec::new(), // Uses configuration settings
33+
};
34+
2735
let PullDiagnosticsResult {
2836
diagnostics,
2937
skipped_diagnostics,
30-
} = workspace.pull_db_diagnostics(PullDatabaseDiagnosticsParams { max_diagnostics })?;
38+
} = workspace.pull_db_diagnostics(params)?;
3139

3240
let report = Report::new(
3341
diagnostics.into_iter().map(Error::from).collect(),

crates/pgls_cli/src/commands/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub enum PgLSCommand {
2424
#[bpaf(command)]
2525
Version(#[bpaf(external(cli_options), hide_usage)] CliOptions),
2626

27-
/// Runs everything to the requested files.
27+
/// Lints your database schema.
2828
#[bpaf(command)]
2929
Dblint {
3030
#[bpaf(external(partial_configuration), hide_usage, optional)]
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
use assert_cmd::Command;
2+
use insta::assert_snapshot;
3+
use sqlx::PgPool;
4+
use std::process::ExitStatus;
5+
6+
const BIN: &str = "postgres-language-server";
7+
8+
/// Get database URL from the pool's connect options
9+
/// Uses the known docker-compose credentials (postgres:postgres)
10+
fn get_database_url(pool: &PgPool) -> String {
11+
let opts = pool.connect_options();
12+
format!(
13+
"postgres://postgres:postgres@{}:{}/{}",
14+
opts.get_host(),
15+
opts.get_port(),
16+
opts.get_database().unwrap_or("postgres")
17+
)
18+
}
19+
20+
#[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")]
21+
#[cfg_attr(
22+
target_os = "windows",
23+
ignore = "snapshot expectations only validated on unix-like platforms"
24+
)]
25+
async fn dblint_empty_database_snapshot(test_db: PgPool) {
26+
let url = get_database_url(&test_db);
27+
let output = run_dblint(&url, &[]);
28+
assert_snapshot!(output);
29+
}
30+
31+
#[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")]
32+
#[cfg_attr(
33+
target_os = "windows",
34+
ignore = "snapshot expectations only validated on unix-like platforms"
35+
)]
36+
async fn dblint_detects_issues_snapshot(test_db: PgPool) {
37+
// Setup: create table without primary key (triggers noPrimaryKey rule)
38+
sqlx::raw_sql("CREATE TABLE test_no_pk (id int, name text)")
39+
.execute(&test_db)
40+
.await
41+
.expect("Failed to create test table");
42+
43+
let url = get_database_url(&test_db);
44+
let output = run_dblint(&url, &[]);
45+
assert_snapshot!(output);
46+
}
47+
48+
#[test]
49+
#[cfg_attr(
50+
target_os = "windows",
51+
ignore = "snapshot expectations only validated on unix-like platforms"
52+
)]
53+
fn dblint_no_database_snapshot() {
54+
// Test that dblint completes gracefully when no database is configured
55+
let mut cmd = Command::cargo_bin(BIN).expect("binary not built");
56+
let output = cmd
57+
.args(["dblint", "--disable-db", "--log-level", "none"])
58+
.output()
59+
.expect("failed to run CLI");
60+
61+
let normalized = normalize_output(
62+
output.status,
63+
&String::from_utf8_lossy(&output.stdout),
64+
&String::from_utf8_lossy(&output.stderr),
65+
);
66+
assert_snapshot!(normalized);
67+
}
68+
69+
fn run_dblint(url: &str, args: &[&str]) -> String {
70+
let mut cmd = Command::cargo_bin(BIN).expect("binary not built");
71+
let mut full_args = vec!["dblint", "--connection-string", url, "--log-level", "none"];
72+
full_args.extend_from_slice(args);
73+
74+
let output = cmd.args(full_args).output().expect("failed to run CLI");
75+
76+
normalize_output(
77+
output.status,
78+
&String::from_utf8_lossy(&output.stdout),
79+
&String::from_utf8_lossy(&output.stderr),
80+
)
81+
}
82+
83+
fn normalize_output(status: ExitStatus, stdout: &str, stderr: &str) -> String {
84+
let normalized_stdout = normalize_durations(stdout);
85+
let status_label = if status.success() {
86+
"success"
87+
} else {
88+
"failure"
89+
};
90+
format!(
91+
"status: {status_label}\nstdout:\n{}\nstderr:\n{}\n",
92+
normalized_stdout.trim_end(),
93+
stderr.trim_end()
94+
)
95+
}
96+
97+
fn normalize_durations(input: &str) -> String {
98+
let mut content = input.to_owned();
99+
100+
let mut search_start = 0;
101+
while let Some(relative) = content[search_start..].find(" in ") {
102+
let start = search_start + relative + 4;
103+
if let Some(end_rel) = content[start..].find('.') {
104+
let end = start + end_rel;
105+
if content[start..end].chars().any(|c| c.is_ascii_digit()) {
106+
content.replace_range(start..end, "<duration>");
107+
search_start = start + "<duration>".len() + 1;
108+
continue;
109+
}
110+
search_start = end + 1;
111+
} else {
112+
break;
113+
}
114+
}
115+
116+
content
117+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
source: crates/pgls_cli/tests/assert_dblint.rs
3+
expression: output
4+
snapshot_kind: text
5+
---
6+
status: success
7+
stdout:
8+
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.
9+
10+
Command completed in <duration>.
11+
stderr:
12+
splinter/performance/noPrimaryKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
13+
14+
i Table \`public.test_no_pk\` does not have a primary key
15+
16+
Detects if a table does not have a primary key. Tables without a primary key can be inefficient to interact with at scale.
17+
18+
i table: public.test_no_pk
19+
20+
i Remediation: https://supabase.com/docs/guides/database/database-linter?lint=0004_no_primary_key
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
source: crates/pgls_cli/tests/assert_dblint.rs
3+
expression: output
4+
snapshot_kind: text
5+
---
6+
status: success
7+
stdout:
8+
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.
9+
10+
Command completed in <duration>.
11+
stderr:
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
source: crates/pgls_cli/tests/assert_dblint.rs
3+
expression: normalized
4+
snapshot_kind: text
5+
---
6+
status: success
7+
stdout:
8+
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.
9+
10+
Command completed in <duration>.
11+
stderr:

crates/pgls_configuration/src/lib.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ pub use rules::{
4242
RuleWithFixOptions, RuleWithOptions,
4343
};
4444
use serde::{Deserialize, Serialize};
45+
use splinter::{
46+
PartialSplinterConfiguration, SplinterConfiguration, partial_splinter_configuration,
47+
};
4548
pub use typecheck::{
4649
PartialTypecheckConfiguration, TypecheckConfiguration, partial_typecheck_configuration,
4750
};
@@ -86,6 +89,10 @@ pub struct Configuration {
8689
#[partial(type, bpaf(external(partial_linter_configuration), optional))]
8790
pub linter: LinterConfiguration,
8891

92+
/// The configuration for splinter
93+
#[partial(type, bpaf(external(partial_splinter_configuration), optional))]
94+
pub splinter: SplinterConfiguration,
95+
8996
/// The configuration for type checking
9097
#[partial(type, bpaf(external(partial_typecheck_configuration), optional))]
9198
pub typecheck: TypecheckConfiguration,
@@ -127,6 +134,10 @@ impl PartialConfiguration {
127134
}),
128135
..Default::default()
129136
}),
137+
splinter: Some(PartialSplinterConfiguration {
138+
enabled: Some(true),
139+
..Default::default()
140+
}),
130141
typecheck: Some(PartialTypecheckConfiguration {
131142
..Default::default()
132143
}),

crates/pgls_configuration/src/linter/rules.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ impl std::str::FromStr for RuleGroup {
4646
}
4747
#[derive(Clone, Debug, Default, Deserialize, Eq, Merge, PartialEq, Serialize)]
4848
#[cfg_attr(feature = "schema", derive(JsonSchema))]
49+
#[cfg_attr(feature = "schema", schemars(rename = "LinterRules"))]
4950
#[serde(rename_all = "camelCase", deny_unknown_fields)]
5051
pub struct Rules {
5152
#[doc = r" It enables the lint rules recommended by Postgres Language Server. `true` by default."]

0 commit comments

Comments
 (0)