Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ jobs:
run: bun run test
- name: Run cli test
working-directory: packages/@postgres-language-server/cli
env:
PGLS_BINARY: ${{ github.workspace }}/target/release/postgres-language-server
run: bun run test

test-wasm:
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ snapshot_kind: text
---
status: success
stdout:
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.

Command completed in <duration>.
stderr:
splinter/performance/noPrimaryKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,5 @@ snapshot_kind: text
---
status: success
stdout:
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.

Command completed in <duration>.
stderr:
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,5 @@ snapshot_kind: text
---
status: success
stdout:
Warning: Deprecated config filename detected. Use 'postgres-language-server.jsonc'.

Command completed in <duration>.
stderr:
39 changes: 38 additions & 1 deletion crates/pgls_configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ impl PartialConfiguration {
..Default::default()
}),
typecheck: Some(PartialTypecheckConfiguration {
enabled: Some(true),
..Default::default()
}),
plpgsql_check: Some(PartialPlPgSqlCheckConfiguration {
..Default::default()
enabled: Some(true),
}),
db: Some(PartialDatabaseConfiguration {
connection_string: None,
Expand Down Expand Up @@ -197,3 +198,39 @@ impl ConfigurationPathHint {
matches!(self, Self::FromLsp(_))
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn init_config_has_no_empty_objects() {
let config = PartialConfiguration::init();
let json_value: serde_json::Value =
serde_json::to_value(&config).expect("failed to serialize config");

fn find_empty_objects(value: &serde_json::Value, path: &str) -> Vec<String> {
let mut empty_paths = Vec::new();
if let serde_json::Value::Object(map) = value {
if map.is_empty() && !path.is_empty() {
empty_paths.push(path.to_string());
}
for (key, val) in map {
let new_path = if path.is_empty() {
key.clone()
} else {
format!("{path}.{key}")
};
empty_paths.extend(find_empty_objects(val, &new_path));
}
}
empty_paths
}

let empty_objects = find_empty_objects(&json_value, "");
assert!(
empty_objects.is_empty(),
"init() configuration should not contain empty objects. Found at: {empty_objects:?}"
);
}
}
19 changes: 19 additions & 0 deletions crates/pgls_configuration/src/splinter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
mod options;
pub use options::SplinterRuleOptions;
mod rules;
use biome_deserialize::StringSet;
use biome_deserialize_macros::{Merge, Partial};
use bpaf::Bpaf;
pub use rules::*;
Expand All @@ -16,6 +17,11 @@ pub struct SplinterConfiguration {
#[doc = r" if `false`, it disables the feature and the linter won't be executed. `true` by default"]
#[partial(bpaf(hide))]
pub enabled: bool,
#[doc = r" A list of glob patterns for database objects to ignore across all rules."]
#[doc = r" Patterns use Unix-style globs where `*` matches any sequence of characters."]
#[doc = r#" Format: `schema.object_name`, e.g., "public.my_table", "audit.*""#]
#[partial(bpaf(hide))]
pub ignore: StringSet,
#[doc = r" List of rules"]
#[partial(bpaf(pure(Default::default()), optional, hide))]
pub rules: Rules,
Expand All @@ -24,11 +30,24 @@ impl SplinterConfiguration {
pub const fn is_disabled(&self) -> bool {
!self.enabled
}
#[doc = r" Build a matcher from the global ignore patterns."]
#[doc = r" Returns None if no patterns are configured."]
pub fn get_global_ignore_matcher(&self) -> Option<pgls_matcher::Matcher> {
if self.ignore.is_empty() {
return None;
}
let mut m = pgls_matcher::Matcher::new(pgls_matcher::MatchOptions::default());
for p in self.ignore.iter() {
let _ = m.add_pattern(p);
}
Some(m)
}
}
impl Default for SplinterConfiguration {
fn default() -> Self {
Self {
enabled: true,
ignore: Default::default(),
rules: Default::default(),
}
}
Expand Down
7 changes: 4 additions & 3 deletions crates/pgls_splinter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ serde_json.workspace = true
sqlx.workspace = true

[dev-dependencies]
insta.workspace = true
pgls_console.workspace = true
pgls_test_utils.workspace = true
biome_deserialize.workspace = true
insta.workspace = true
pgls_console.workspace = true
pgls_test_utils.workspace = true

[lib]
doctest = false
42 changes: 27 additions & 15 deletions crates/pgls_splinter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub mod rule;
pub mod rules;

use pgls_analyse::{AnalysisFilter, RegistryVisitor, RuleMeta};
use pgls_configuration::splinter::Rules;
use pgls_configuration::splinter::SplinterConfiguration;
use pgls_schema_cache::SchemaCache;
use sqlx::PgPool;

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

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

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

if let Some(rules_config) = params.rules_config {
let rule_matchers = rules_config.get_ignore_matchers();
if let Some(config) = params.config {
// Build global ignore matcher if patterns exist
let global_ignore_matcher = config.get_global_ignore_matcher();

if !rule_matchers.is_empty() {
// Get per-rule ignore matchers
let rule_matchers = config.rules.get_ignore_matchers();

// Filter diagnostics based on global and per-rule ignore patterns
if global_ignore_matcher.is_some() || !rule_matchers.is_empty() {
diagnostics.retain(|diag| {
let rule_name = diag.category.name().split('/').next_back().unwrap_or("");
let object_identifier = match (&diag.advices.schema, &diag.advices.object_name) {
(Some(schema), Some(name)) => format!("{schema}.{name}"),
_ => return true, // Keep diagnostics without schema.name
};

// Check global ignore first
if let Some(ref matcher) = global_ignore_matcher {
if matcher.matches(&object_identifier) {
return false;
}
}

// Then check per-rule ignore
let rule_name = diag.category.name().split('/').next_back().unwrap_or("");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: we might filter out rules with the same name but different categories, e.g.

lint/safety/rlsDislabed – findet statements wo DISABLE ROW LEVEL SECURITY steht
splinter/safety/rlsDisabled – findet tables, die schon existieren, bei denen RLS disabled ist

wenn du jetzt rlsDisabled irgendwo rein packst, disablest du beides, willst aber vllt. nur die zweite.

Klingt erstmal nach edge-case, könnte uns aber beißen

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

die configs sind separat, und let rule_matchers = config.rules.get_ignore_matchers(); zieht sich nur die ignore config fuer den splinter.

if let Some(matcher) = rule_matchers.get(rule_name) {
if let (Some(schema), Some(name)) =
(&diag.advices.schema, &diag.advices.object_name)
{
let object_identifier = format!("{schema}.{name}");

if matcher.matches(&object_identifier) {
return false;
}
if matcher.matches(&object_identifier) {
return false;
}
}

true
});
}
Expand Down
Loading
Loading