Skip to content

Commit d34d4d4

Browse files
authored
chore: minor fixes for dblint + docs (#637)
1 parent 7eb0b54 commit d34d4d4

File tree

29 files changed

+821
-103
lines changed

29 files changed

+821
-103
lines changed

.github/workflows/pull_request.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ jobs:
239239
run: bun run test
240240
- name: Run cli test
241241
working-directory: packages/@postgres-language-server/cli
242+
env:
243+
PGLS_BINARY: ${{ github.workspace }}/target/release/postgres-language-server
242244
run: bun run test
243245

244246
test-wasm:

Cargo.lock

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

crates/pgls_cli/tests/snapshots/assert_dblint__dblint_detects_issues_snapshot.snap

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ snapshot_kind: text
55
---
66
status: success
77
stdout:
8-
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.
9-
108
Command completed in <duration>.
119
stderr:
1210
splinter/performance/noPrimaryKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

crates/pgls_cli/tests/snapshots/assert_dblint__dblint_empty_database_snapshot.snap

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,5 @@ snapshot_kind: text
55
---
66
status: success
77
stdout:
8-
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.
9-
108
Command completed in <duration>.
119
stderr:

crates/pgls_cli/tests/snapshots/assert_dblint__dblint_no_database_snapshot.snap

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,5 @@ snapshot_kind: text
55
---
66
status: success
77
stdout:
8-
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.
9-
108
Command completed in <duration>.
119
stderr:

crates/pgls_configuration/src/lib.rs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,11 @@ impl PartialConfiguration {
139139
..Default::default()
140140
}),
141141
typecheck: Some(PartialTypecheckConfiguration {
142+
enabled: Some(true),
142143
..Default::default()
143144
}),
144145
plpgsql_check: Some(PartialPlPgSqlCheckConfiguration {
145-
..Default::default()
146+
enabled: Some(true),
146147
}),
147148
db: Some(PartialDatabaseConfiguration {
148149
connection_string: None,
@@ -197,3 +198,39 @@ impl ConfigurationPathHint {
197198
matches!(self, Self::FromLsp(_))
198199
}
199200
}
201+
202+
#[cfg(test)]
203+
mod tests {
204+
use super::*;
205+
206+
#[test]
207+
fn init_config_has_no_empty_objects() {
208+
let config = PartialConfiguration::init();
209+
let json_value: serde_json::Value =
210+
serde_json::to_value(&config).expect("failed to serialize config");
211+
212+
fn find_empty_objects(value: &serde_json::Value, path: &str) -> Vec<String> {
213+
let mut empty_paths = Vec::new();
214+
if let serde_json::Value::Object(map) = value {
215+
if map.is_empty() && !path.is_empty() {
216+
empty_paths.push(path.to_string());
217+
}
218+
for (key, val) in map {
219+
let new_path = if path.is_empty() {
220+
key.clone()
221+
} else {
222+
format!("{path}.{key}")
223+
};
224+
empty_paths.extend(find_empty_objects(val, &new_path));
225+
}
226+
}
227+
empty_paths
228+
}
229+
230+
let empty_objects = find_empty_objects(&json_value, "");
231+
assert!(
232+
empty_objects.is_empty(),
233+
"init() configuration should not contain empty objects. Found at: {empty_objects:?}"
234+
);
235+
}
236+
}

crates/pgls_configuration/src/splinter/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
mod options;
55
pub use options::SplinterRuleOptions;
66
mod rules;
7+
use biome_deserialize::StringSet;
78
use biome_deserialize_macros::{Merge, Partial};
89
use bpaf::Bpaf;
910
pub use rules::*;
@@ -16,6 +17,11 @@ pub struct SplinterConfiguration {
1617
#[doc = r" if `false`, it disables the feature and the linter won't be executed. `true` by default"]
1718
#[partial(bpaf(hide))]
1819
pub enabled: bool,
20+
#[doc = r" A list of glob patterns for database objects to ignore across all rules."]
21+
#[doc = r" Patterns use Unix-style globs where `*` matches any sequence of characters."]
22+
#[doc = r#" Format: `schema.object_name`, e.g., "public.my_table", "audit.*""#]
23+
#[partial(bpaf(hide))]
24+
pub ignore: StringSet,
1925
#[doc = r" List of rules"]
2026
#[partial(bpaf(pure(Default::default()), optional, hide))]
2127
pub rules: Rules,
@@ -24,11 +30,24 @@ impl SplinterConfiguration {
2430
pub const fn is_disabled(&self) -> bool {
2531
!self.enabled
2632
}
33+
#[doc = r" Build a matcher from the global ignore patterns."]
34+
#[doc = r" Returns None if no patterns are configured."]
35+
pub fn get_global_ignore_matcher(&self) -> Option<pgls_matcher::Matcher> {
36+
if self.ignore.is_empty() {
37+
return None;
38+
}
39+
let mut m = pgls_matcher::Matcher::new(pgls_matcher::MatchOptions::default());
40+
for p in self.ignore.iter() {
41+
let _ = m.add_pattern(p);
42+
}
43+
Some(m)
44+
}
2745
}
2846
impl Default for SplinterConfiguration {
2947
fn default() -> Self {
3048
Self {
3149
enabled: true,
50+
ignore: Default::default(),
3251
rules: Default::default(),
3352
}
3453
}

crates/pgls_splinter/Cargo.toml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ serde_json.workspace = true
2121
sqlx.workspace = true
2222

2323
[dev-dependencies]
24-
insta.workspace = true
25-
pgls_console.workspace = true
26-
pgls_test_utils.workspace = true
24+
biome_deserialize.workspace = true
25+
insta.workspace = true
26+
pgls_console.workspace = true
27+
pgls_test_utils.workspace = true
2728

2829
[lib]
2930
doctest = false

crates/pgls_splinter/src/lib.rs

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub mod rule;
66
pub mod rules;
77

88
use pgls_analyse::{AnalysisFilter, RegistryVisitor, RuleMeta};
9-
use pgls_configuration::splinter::Rules;
9+
use pgls_configuration::splinter::SplinterConfiguration;
1010
use pgls_schema_cache::SchemaCache;
1111
use sqlx::PgPool;
1212

@@ -18,8 +18,8 @@ pub use rule::SplinterRule;
1818
pub struct SplinterParams<'a> {
1919
pub conn: &'a PgPool,
2020
pub schema_cache: Option<&'a SchemaCache>,
21-
/// Optional rules configuration for per-rule database object filtering
22-
pub rules_config: Option<&'a Rules>,
21+
/// Optional splinter configuration for global and per-rule database object filtering
22+
pub config: Option<&'a SplinterConfiguration>,
2323
}
2424

2525
/// Visitor that collects enabled splinter rules based on filter
@@ -138,24 +138,36 @@ pub async fn run_splinter(
138138

139139
let mut diagnostics: Vec<SplinterDiagnostic> = results.into_iter().map(Into::into).collect();
140140

141-
if let Some(rules_config) = params.rules_config {
142-
let rule_matchers = rules_config.get_ignore_matchers();
141+
if let Some(config) = params.config {
142+
// Build global ignore matcher if patterns exist
143+
let global_ignore_matcher = config.get_global_ignore_matcher();
143144

144-
if !rule_matchers.is_empty() {
145+
// Get per-rule ignore matchers
146+
let rule_matchers = config.rules.get_ignore_matchers();
147+
148+
// Filter diagnostics based on global and per-rule ignore patterns
149+
if global_ignore_matcher.is_some() || !rule_matchers.is_empty() {
145150
diagnostics.retain(|diag| {
146-
let rule_name = diag.category.name().split('/').next_back().unwrap_or("");
151+
let object_identifier = match (&diag.advices.schema, &diag.advices.object_name) {
152+
(Some(schema), Some(name)) => format!("{schema}.{name}"),
153+
_ => return true, // Keep diagnostics without schema.name
154+
};
155+
156+
// Check global ignore first
157+
if let Some(ref matcher) = global_ignore_matcher {
158+
if matcher.matches(&object_identifier) {
159+
return false;
160+
}
161+
}
147162

163+
// Then check per-rule ignore
164+
let rule_name = diag.category.name().split('/').next_back().unwrap_or("");
148165
if let Some(matcher) = rule_matchers.get(rule_name) {
149-
if let (Some(schema), Some(name)) =
150-
(&diag.advices.schema, &diag.advices.object_name)
151-
{
152-
let object_identifier = format!("{schema}.{name}");
153-
154-
if matcher.matches(&object_identifier) {
155-
return false;
156-
}
166+
if matcher.matches(&object_identifier) {
167+
return false;
157168
}
158169
}
170+
159171
true
160172
});
161173
}

0 commit comments

Comments
 (0)