From cb5800f1a2b2932f36123c99b9562048b37d6341 Mon Sep 17 00:00:00 2001 From: Taishi Naka Date: Fri, 31 Oct 2025 18:31:55 +0900 Subject: [PATCH 1/5] feat: no-not-in rule --- Cargo.lock | 14 +- Cargo.toml | 1 + crates/uroborosql-fmt/Cargo.toml | 2 +- crates/uroborosql-lint/Cargo.toml | 2 +- crates/uroborosql-lint/src/linter.rs | 3 +- crates/uroborosql-lint/src/rules.rs | 2 + crates/uroborosql-lint/src/rules/no_not_in.rs | 135 ++++++++++++++++++ 7 files changed, 145 insertions(+), 14 deletions(-) create mode 100644 crates/uroborosql-lint/src/rules/no_not_in.rs diff --git a/Cargo.lock b/Cargo.lock index a5efe20d..64bb8a33 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -586,15 +586,7 @@ dependencies = [ [[package]] name = "postgresql-cst-parser" version = "0.2.0" -source = "git+https://github.com/future-architect/postgresql-cst-parser?branch=feat%2Fnew-apis-for-linter#07a3eac6ba4c5bfb0097caf225701b342d85c53c" -dependencies = [ - "cstree", -] - -[[package]] -name = "postgresql-cst-parser" -version = "0.2.0" -source = "git+https://github.com/future-architect/postgresql-cst-parser#449673d07f016f97a0769461cfc340c94e66f955" +source = "git+https://github.com/future-architect/postgresql-cst-parser?branch=feat%2Fnew-apis-for-linter#47aa0d7121be2d194a0860dcf9f9d0c7f392151a" dependencies = [ "cstree", ] @@ -913,7 +905,7 @@ dependencies = [ "indexmap 1.9.3", "itertools", "once_cell", - "postgresql-cst-parser 0.2.0 (git+https://github.com/future-architect/postgresql-cst-parser)", + "postgresql-cst-parser", "regex", "serde", "serde_json", @@ -955,7 +947,7 @@ dependencies = [ name = "uroborosql-lint" version = "0.1.0" dependencies = [ - "postgresql-cst-parser 0.2.0 (git+https://github.com/future-architect/postgresql-cst-parser?branch=feat%2Fnew-apis-for-linter)", + "postgresql-cst-parser", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e2a97b79..e7389afd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ repository = "https://github.com/future-architect/uroborosql-fmt" [workspace.dependencies] # Internal crates uroborosql-fmt = { path = "./crates/uroborosql-fmt" } +postgresql-cst-parser = { git = "https://github.com/future-architect/postgresql-cst-parser", branch = "feat/new-apis-for-linter" } [profile.release] lto = true diff --git a/crates/uroborosql-fmt/Cargo.toml b/crates/uroborosql-fmt/Cargo.toml index e2b3d1ac..e5c730f2 100644 --- a/crates/uroborosql-fmt/Cargo.toml +++ b/crates/uroborosql-fmt/Cargo.toml @@ -21,7 +21,7 @@ serde_json = "1.0.91" thiserror = "1.0.38" # git config --global core.longpaths true を管理者権限で実行しないとけない -postgresql-cst-parser = { git = "https://github.com/future-architect/postgresql-cst-parser" } +postgresql-cst-parser = { workspace = true } [dev-dependencies] console = "0.15.10" diff --git a/crates/uroborosql-lint/Cargo.toml b/crates/uroborosql-lint/Cargo.toml index 80365738..6ec0d9d4 100644 --- a/crates/uroborosql-lint/Cargo.toml +++ b/crates/uroborosql-lint/Cargo.toml @@ -7,4 +7,4 @@ license.workspace = true repository.workspace = true [dependencies] -postgresql-cst-parser = { git = "https://github.com/future-architect/postgresql-cst-parser", branch = "feat/new-apis-for-linter" } +postgresql-cst-parser = { workspace = true } diff --git a/crates/uroborosql-lint/src/linter.rs b/crates/uroborosql-lint/src/linter.rs index 0faedc13..f55c0be5 100644 --- a/crates/uroborosql-lint/src/linter.rs +++ b/crates/uroborosql-lint/src/linter.rs @@ -2,7 +2,7 @@ use crate::{ context::LintContext, diagnostic::{Diagnostic, Severity}, rule::Rule, - rules::{NoDistinct, NoUnionDistinct, TooLargeInList}, + rules::{NoDistinct, NoNotIn, NoUnionDistinct, TooLargeInList}, tree::collect_preorder, }; use postgresql_cst_parser::tree_sitter; @@ -105,6 +105,7 @@ impl Linter { fn default_rules() -> Vec> { vec![ Box::new(NoDistinct), + Box::new(NoNotIn), Box::new(NoUnionDistinct), Box::new(TooLargeInList), ] diff --git a/crates/uroborosql-lint/src/rules.rs b/crates/uroborosql-lint/src/rules.rs index 6d9e9a25..b19f03e6 100644 --- a/crates/uroborosql-lint/src/rules.rs +++ b/crates/uroborosql-lint/src/rules.rs @@ -1,7 +1,9 @@ mod no_distinct; +mod no_not_in; mod no_union_distinct; mod too_large_in_list; pub use no_distinct::NoDistinct; +pub use no_not_in::NoNotIn; pub use no_union_distinct::NoUnionDistinct; pub use too_large_in_list::TooLargeInList; diff --git a/crates/uroborosql-lint/src/rules/no_not_in.rs b/crates/uroborosql-lint/src/rules/no_not_in.rs new file mode 100644 index 00000000..f8ed0823 --- /dev/null +++ b/crates/uroborosql-lint/src/rules/no_not_in.rs @@ -0,0 +1,135 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, +}; +use postgresql_cst_parser::{ + syntax_kind::SyntaxKind, + tree_sitter::{Node, Range}, +}; + +/// Detects NOT IN expressions. +/// Rule source: https://future-architect.github.io/coding-standards/documents/forSQL/SQL%E3%82%B3%E3%83%BC%E3%83%87%E3%82%A3%E3%83%B3%E3%82%B0%E8%A6%8F%E7%B4%84%EF%BC%88PostgreSQL%EF%BC%89.html#not-in-%E5%8F%A5 +pub struct NoNotIn; + +impl Rule for NoNotIn { + fn name(&self) -> &'static str { + "no-not-in" + } + + fn default_severity(&self) -> Severity { + Severity::Warning + } + + fn target_kinds(&self) -> &'static [SyntaxKind] { + &[SyntaxKind::in_expr] + } + + fn run_on_node<'tree>(&self, node: &Node<'tree>, ctx: &mut LintContext, severity: Severity) { + let Some(range) = detect_not_in(node) else { + return; + }; + + let diagnostic = Diagnostic::new( + self.name(), + severity, + "Avoid using NOT IN; prefer NOT EXISTS or other alternatives to handle NULL correctly.", + &range, + ); + ctx.report(diagnostic); + } +} + +fn detect_not_in(node: &Node<'_>) -> Option { + // Detects `NOT_LA IN_P in_expr` sequense. + // We traverse siblings backwards, so the expected order is `in_expr`, `IN_P`, `NOT_LA`. + + let in_expr_node = node; + if in_expr_node.kind() != SyntaxKind::in_expr { + return None; + } + + // IN_P + let in_node = prev_node_skipping_comments(in_expr_node)?; + if in_node.kind() != SyntaxKind::IN_P { + return None; + } + + // NOT_LA + let not_node = prev_node_skipping_comments(&in_node)?; + if not_node.kind() != SyntaxKind::NOT_LA { + return None; + } + + Some(not_node.range().extended_by(&in_expr_node.range())) +} + +fn prev_node_skipping_comments<'a>(node: &Node<'a>) -> Option> { + let mut current = node.clone(); + loop { + let prev = current.prev_sibling()?; + if !prev.is_comment() { + return Some(prev); + } + current = prev; + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{linter::tests::run_with_rules, SqlSpan}; + + #[test] + fn detects_simple_not_in() { + let sql = "SELECT value FROM users WHERE id NOT IN (1, 2);"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoNotIn)]); + + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-not-in") + .expect("expected NOT IN to be detected"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "NOT IN (1, 2)"); + } + + #[test] + fn detects_not_in_with_comment() { + let sql = "SELECT value FROM users WHERE id NOT /* comment */ IN (1);"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoNotIn)]); + + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-not-in") + .expect("expected NOT IN to be detected"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "NOT /* comment */ IN (1)"); + } + + #[test] + fn detects_not_in_with_subquery() { + let sql = "SELECT value FROM users WHERE id NOT IN (SELECT id FROM admins);"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoNotIn)]); + + assert!( + diagnostics.iter().any(|diag| diag.rule_id == "no-not-in"), + "expected NOT IN subquery to be detected" + ); + } + + #[test] + fn allows_in_without_not() { + let sql = "SELECT value FROM users WHERE id IN (1, 2);"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoNotIn)]); + assert!(diagnostics.is_empty()); + } + + #[test] + fn allows_not_between() { + let sql = "SELECT value FROM users WHERE id NOT BETWEEN 1 AND 5;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoNotIn)]); + assert!(diagnostics.is_empty(), "NOT BETWEEN should be allowed"); + } +} From c69a37b4d646644ff0d4fffcc029e724fc5f7439 Mon Sep 17 00:00:00 2001 From: Taishi Naka Date: Tue, 4 Nov 2025 15:38:28 +0900 Subject: [PATCH 2/5] feat: no-wildcard --- crates/uroborosql-lint/src/linter.rs | 3 +- crates/uroborosql-lint/src/rules.rs | 2 + .../src/rules/no_wildcard_projection.rs | 188 ++++++++++++++++++ 3 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 crates/uroborosql-lint/src/rules/no_wildcard_projection.rs diff --git a/crates/uroborosql-lint/src/linter.rs b/crates/uroborosql-lint/src/linter.rs index f55c0be5..5c686112 100644 --- a/crates/uroborosql-lint/src/linter.rs +++ b/crates/uroborosql-lint/src/linter.rs @@ -2,7 +2,7 @@ use crate::{ context::LintContext, diagnostic::{Diagnostic, Severity}, rule::Rule, - rules::{NoDistinct, NoNotIn, NoUnionDistinct, TooLargeInList}, + rules::{NoDistinct, NoNotIn, NoUnionDistinct, NoWildcardProjection, TooLargeInList}, tree::collect_preorder, }; use postgresql_cst_parser::tree_sitter; @@ -107,6 +107,7 @@ fn default_rules() -> Vec> { Box::new(NoDistinct), Box::new(NoNotIn), Box::new(NoUnionDistinct), + Box::new(NoWildcardProjection), Box::new(TooLargeInList), ] } diff --git a/crates/uroborosql-lint/src/rules.rs b/crates/uroborosql-lint/src/rules.rs index b19f03e6..d595289b 100644 --- a/crates/uroborosql-lint/src/rules.rs +++ b/crates/uroborosql-lint/src/rules.rs @@ -1,9 +1,11 @@ mod no_distinct; mod no_not_in; mod no_union_distinct; +mod no_wildcard_projection; mod too_large_in_list; pub use no_distinct::NoDistinct; pub use no_not_in::NoNotIn; pub use no_union_distinct::NoUnionDistinct; +pub use no_wildcard_projection::NoWildcardProjection; pub use too_large_in_list::TooLargeInList; diff --git a/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs b/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs new file mode 100644 index 00000000..b5008934 --- /dev/null +++ b/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs @@ -0,0 +1,188 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, +}; +use postgresql_cst_parser::{ + syntax_kind::SyntaxKind, + tree_sitter::{Node, Range}, +}; + +/// Detects wildcard projections. (e.g. `SELECT *`, `SELECT u.*`, `RETURNING *`) +/// Rule source: https://future-architect.github.io/coding-standards/documents/forSQL/SQL%E3%82%B3%E3%83%BC%E3%83%87%E3%82%A3%E3%83%B3%E3%82%B0%E8%A6%8F%E7%B4%84%EF%BC%88PostgreSQL%EF%BC%89.html#%E6%A4%9C%E7%B4%A2:~:text=%E3%82%92%E6%8C%87%E5%AE%9A%E3%81%99%E3%82%8B-,%E5%85%A8%E5%88%97%E3%83%AF%E3%82%A4%E3%83%AB%E3%83%89%E3%82%AB%E3%83%BC%E3%83%89%E3%80%8C*%E3%80%8D%E3%81%AE%E4%BD%BF%E7%94%A8%E3%81%AF%E3%81%9B%E3%81%9A%E3%80%81%E3%82%AB%E3%83%A9%E3%83%A0%E5%90%8D%E3%82%92%E6%98%8E%E8%A8%98%E3%81%99%E3%82%8B,-%E3%82%A4%E3%83%B3%E3%83%87%E3%83%83%E3%82%AF%E3%82%B9%E3%81%AB%E3%82%88%E3%82%8B%E6%A4%9C%E7%B4%A2 +pub struct NoWildcardProjection; + +impl Rule for NoWildcardProjection { + fn name(&self) -> &'static str { + "no-wildcard-projection" + } + + fn default_severity(&self) -> Severity { + Severity::Warning + } + + fn target_kinds(&self) -> &'static [SyntaxKind] { + &[SyntaxKind::target_el] + } + + fn run_on_node<'tree>(&self, node: &Node<'tree>, ctx: &mut LintContext, severity: Severity) { + let Some(range) = detect_wildcard(node) else { + return; + }; + + let diagnostic = Diagnostic::new( + self.name(), + severity, + "Wildcard projections are not allowed; list the columns explicitly.", + &range, + ); + ctx.report(diagnostic); + } +} + +fn detect_wildcard(target_el_node: &Node<'_>) -> Option { + assert_eq!(target_el_node.kind(), SyntaxKind::target_el); + // target_el: + // - '*' + // ^^^ Wildcard + // - a_expr AS ColLabel + // - a_expr BareColLabel + // - a_expr + // ^^^^^^ If a_expr contains a columnref, a wildcard may appear + // + // source: https://github.com/postgres/postgres/blob/65f4976189b6cbe9aa93fc5f4b1eb7a2040b6301/src/backend/parser/gram.y#L17364-L17401 + + let mut cursor = target_el_node.walk(); + cursor.goto_first_child(); + + match cursor.node().kind() { + SyntaxKind::Star => Some(cursor.node().range()), + SyntaxKind::a_expr => { + let a_expr = cursor.node(); + let columnref = get_columnref_from_a_expr(&a_expr)?; + + // columnref: + // - ColId + // - ColId indirection + // - e.g.: `a.field`, `a.field[1]`, `a.*` + // + // source: https://github.com/postgres/postgres/blob/65f4976189b6cbe9aa93fc5f4b1eb7a2040b6301/src/backend/parser/gram.y#L17041-L17049 + + let indirection = columnref + .children() + .iter() + .find(|child| child.kind() == SyntaxKind::indirection) + .cloned()?; + + // indirection: list of indirection_el + let last_indirection_el = indirection.children().last()?.clone(); + + // indirection_el: + // - '.' attr_name + // - '.' '*' + // - '[' a_expr ']' + // - '[' opt_slice_bound ':' opt_slice_bound ']' + // + // source: https://github.com/postgres/postgres/blob/65f4976189b6cbe9aa93fc5f4b1eb7a2040b6301/src/backend/parser/gram.y#L17051-L17078 + let last_child = last_indirection_el.children().last().cloned()?; + + // possible: attr_name, '*', ']' + if last_child.kind() == SyntaxKind::Star { + Some(last_indirection_el.range()) + } else { + None + } + } + _ => None, + } +} + +/// Retrieves the `columnref` node from an `a_expr` node if it exists. +fn get_columnref_from_a_expr<'a>(a_expr: &'a Node<'a>) -> Option> { + // current: a_expr + // + // possible structure: + // - a_expr + // - c_expr + // - columnref + + let mut cursor = a_expr.walk(); + cursor.goto_first_child(); + + match cursor.node().kind() { + SyntaxKind::c_expr => { + cursor.goto_first_child(); + if cursor.node().kind() == SyntaxKind::columnref { + return Some(cursor.node()); + } + + None + } + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{linter::tests::run_with_rules, SqlSpan}; + + fn run(sql: &str) -> Vec { + run_with_rules(sql, vec![Box::new(NoWildcardProjection)]) + } + + #[test] + fn detects_select_star() { + let sql = "SELECT * FROM users;"; + let diagnostics = run(sql); + + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-wildcard-projection") + .expect("should detect SELECT *"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "*"); + } + + #[test] + fn detects_returning_star() { + let sql = "INSERT INTO users(id) VALUES (1) RETURNING *;"; + let diagnostics = run(sql); + + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-wildcard-projection") + .expect("should detect RETURNING *"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "*"); + } + + #[test] + fn detects_table_star() { + let sql = "SELECT u.* FROM users u;"; + let diagnostics = run(sql); + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-wildcard-projection") + .expect("should detect .*"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], ".*"); + } + + #[test] + fn allows_explicit_columns() { + let sql = "SELECT id, name FROM users;"; + let diagnostics = run(sql); + assert!(diagnostics.is_empty()); + } + + #[test] + fn allows_count_star() { + let sql = "SELECT count(*) FROM users;"; + let diagnostics = run(sql); + assert!(diagnostics.is_empty(), "count(*) should be allowed"); + } +} From b0db14bb9448f59344287f82bdb0249aeaad665e Mon Sep 17 00:00:00 2001 From: Taishi Naka Date: Tue, 4 Nov 2025 19:05:48 +0900 Subject: [PATCH 3/5] change star detection logic --- Cargo.lock | 2 +- .../src/rules/no_wildcard_projection.rs | 100 ++++-------------- 2 files changed, 23 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 64bb8a33..a3d6c1d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -586,7 +586,7 @@ dependencies = [ [[package]] name = "postgresql-cst-parser" version = "0.2.0" -source = "git+https://github.com/future-architect/postgresql-cst-parser?branch=feat%2Fnew-apis-for-linter#47aa0d7121be2d194a0860dcf9f9d0c7f392151a" +source = "git+https://github.com/future-architect/postgresql-cst-parser?branch=feat%2Fnew-apis-for-linter#4b36aef5c17b4ac1930a07fb05630642831f79be" dependencies = [ "cstree", ] diff --git a/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs b/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs index b5008934..d604cdcf 100644 --- a/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs +++ b/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs @@ -42,84 +42,15 @@ impl Rule for NoWildcardProjection { fn detect_wildcard(target_el_node: &Node<'_>) -> Option { assert_eq!(target_el_node.kind(), SyntaxKind::target_el); - // target_el: - // - '*' - // ^^^ Wildcard - // - a_expr AS ColLabel - // - a_expr BareColLabel - // - a_expr - // ^^^^^^ If a_expr contains a columnref, a wildcard may appear - // - // source: https://github.com/postgres/postgres/blob/65f4976189b6cbe9aa93fc5f4b1eb7a2040b6301/src/backend/parser/gram.y#L17364-L17401 - - let mut cursor = target_el_node.walk(); - cursor.goto_first_child(); - - match cursor.node().kind() { - SyntaxKind::Star => Some(cursor.node().range()), - SyntaxKind::a_expr => { - let a_expr = cursor.node(); - let columnref = get_columnref_from_a_expr(&a_expr)?; - - // columnref: - // - ColId - // - ColId indirection - // - e.g.: `a.field`, `a.field[1]`, `a.*` - // - // source: https://github.com/postgres/postgres/blob/65f4976189b6cbe9aa93fc5f4b1eb7a2040b6301/src/backend/parser/gram.y#L17041-L17049 - - let indirection = columnref - .children() - .iter() - .find(|child| child.kind() == SyntaxKind::indirection) - .cloned()?; - - // indirection: list of indirection_el - let last_indirection_el = indirection.children().last()?.clone(); - - // indirection_el: - // - '.' attr_name - // - '.' '*' - // - '[' a_expr ']' - // - '[' opt_slice_bound ':' opt_slice_bound ']' - // - // source: https://github.com/postgres/postgres/blob/65f4976189b6cbe9aa93fc5f4b1eb7a2040b6301/src/backend/parser/gram.y#L17051-L17078 - let last_child = last_indirection_el.children().last().cloned()?; - - // possible: attr_name, '*', ']' - if last_child.kind() == SyntaxKind::Star { - Some(last_indirection_el.range()) - } else { - None - } - } - _ => None, - } -} -/// Retrieves the `columnref` node from an `a_expr` node if it exists. -fn get_columnref_from_a_expr<'a>(a_expr: &'a Node<'a>) -> Option> { - // current: a_expr - // - // possible structure: - // - a_expr - // - c_expr - // - columnref - - let mut cursor = a_expr.walk(); - cursor.goto_first_child(); - - match cursor.node().kind() { - SyntaxKind::c_expr => { - cursor.goto_first_child(); - if cursor.node().kind() == SyntaxKind::columnref { - return Some(cursor.node()); - } - - None - } - _ => None, + // If the last node (including the entire subtree) under target_el is '*', it is considered a wildcard. + let last_node = target_el_node.last_node()?; + + if last_node.kind() == SyntaxKind::Star { + return Some(last_node.range()); } + + None } #[cfg(test)] @@ -166,10 +97,23 @@ mod tests { let diagnostic = diagnostics .iter() .find(|diag| diag.rule_id == "no-wildcard-projection") - .expect("should detect .*"); + .expect("should detect *"); let SqlSpan { start, end } = diagnostic.span; - assert_eq!(&sql[start.byte..end.byte], ".*"); + assert_eq!(&sql[start.byte..end.byte], "*"); + } + + #[test] + fn detects_parenthesized_star() { + let sql = "SELECT (u).* FROM users u;"; + let diagnostics = run(sql); + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-wildcard-projection") + .expect("should detect *"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "*"); } #[test] From baeb0d0f97b2fbb50df8eb89ecd6b463f5b98bd9 Mon Sep 17 00:00:00 2001 From: Taishi Naka Date: Tue, 4 Nov 2025 19:13:09 +0900 Subject: [PATCH 4/5] refactor(lint): extract prev_node_skipping_comments to common utilities --- crates/uroborosql-lint/src/rules/no_not_in.rs | 12 +----------- crates/uroborosql-lint/src/tree.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/uroborosql-lint/src/rules/no_not_in.rs b/crates/uroborosql-lint/src/rules/no_not_in.rs index f8ed0823..2860fe7b 100644 --- a/crates/uroborosql-lint/src/rules/no_not_in.rs +++ b/crates/uroborosql-lint/src/rules/no_not_in.rs @@ -2,6 +2,7 @@ use crate::{ context::LintContext, diagnostic::{Diagnostic, Severity}, rule::Rule, + tree::prev_node_skipping_comments, }; use postgresql_cst_parser::{ syntax_kind::SyntaxKind, @@ -64,17 +65,6 @@ fn detect_not_in(node: &Node<'_>) -> Option { Some(not_node.range().extended_by(&in_expr_node.range())) } -fn prev_node_skipping_comments<'a>(node: &Node<'a>) -> Option> { - let mut current = node.clone(); - loop { - let prev = current.prev_sibling()?; - if !prev.is_comment() { - return Some(prev); - } - current = prev; - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/uroborosql-lint/src/tree.rs b/crates/uroborosql-lint/src/tree.rs index 186ddefd..d71c5650 100644 --- a/crates/uroborosql-lint/src/tree.rs +++ b/crates/uroborosql-lint/src/tree.rs @@ -32,3 +32,16 @@ fn walk_preorder<'tree, T>( } } } + +/// Returns the previous sibling node, skipping comment nodes. +/// Returns `None` if no non-comment sibling is found. +pub fn prev_node_skipping_comments<'a>(node: &Node<'a>) -> Option> { + let mut current = node.clone(); + loop { + let prev = current.prev_sibling()?; + if !prev.is_comment() { + return Some(prev); + } + current = prev; + } +} From 6b2bdd297899b5376c6e74f26d4b5a93db63763c Mon Sep 17 00:00:00 2001 From: Taishi Naka Date: Thu, 13 Nov 2025 14:04:47 +0900 Subject: [PATCH 5/5] fix typo --- crates/uroborosql-lint/src/rules/no_not_in.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/uroborosql-lint/src/rules/no_not_in.rs b/crates/uroborosql-lint/src/rules/no_not_in.rs index 2860fe7b..f811f8c9 100644 --- a/crates/uroborosql-lint/src/rules/no_not_in.rs +++ b/crates/uroborosql-lint/src/rules/no_not_in.rs @@ -42,7 +42,7 @@ impl Rule for NoNotIn { } fn detect_not_in(node: &Node<'_>) -> Option { - // Detects `NOT_LA IN_P in_expr` sequense. + // Detects `NOT_LA IN_P in_expr` sequence. // We traverse siblings backwards, so the expected order is `in_expr`, `IN_P`, `NOT_LA`. let in_expr_node = node;