diff --git a/Cargo.lock b/Cargo.lock index 5181d6fd..cf941d13 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#449673d07f016f97a0769461cfc340c94e66f955" +source = "git+https://github.com/future-architect/postgresql-cst-parser?branch=feat%2Fnew-apis-for-linter#cb10defcfea42f68db98e6b0c67a6fc54d88b58b" dependencies = [ "cstree", ] @@ -943,6 +943,20 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "uroborosql-lint" +version = "0.1.0" +dependencies = [ + "postgresql-cst-parser", +] + +[[package]] +name = "uroborosql-lint-cli" +version = "0.1.0" +dependencies = [ + "uroborosql-lint", +] + [[package]] name = "utf8parse" version = "0.2.2" diff --git a/Cargo.toml b/Cargo.toml index 8101fd29..e7389afd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,9 @@ members = [ "crates/uroborosql-fmt", "crates/uroborosql-fmt-cli", "crates/uroborosql-fmt-napi", - "crates/uroborosql-fmt-wasm" + "crates/uroborosql-fmt-wasm", + "crates/uroborosql-lint", + "crates/uroborosql-lint-cli" ] resolver = "2" @@ -16,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-cli/Cargo.toml b/crates/uroborosql-lint-cli/Cargo.toml new file mode 100644 index 00000000..b5b5ad5d --- /dev/null +++ b/crates/uroborosql-lint-cli/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "uroborosql-lint-cli" +version = "0.1.0" +authors.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +uroborosql-lint = { path = "../uroborosql-lint" } diff --git a/crates/uroborosql-lint-cli/src/main.rs b/crates/uroborosql-lint-cli/src/main.rs new file mode 100644 index 00000000..593e0542 --- /dev/null +++ b/crates/uroborosql-lint-cli/src/main.rs @@ -0,0 +1,63 @@ +use std::{env, fs, path::PathBuf, process}; + +use uroborosql_lint::{Diagnostic, LintError, Linter}; + +fn main() { + if let Err(err) = run() { + eprintln!("{err}"); + process::exit(1); + } +} + +fn run() -> Result<(), String> { + let mut args = env::args_os().skip(1); + let Some(path_os) = args.next() else { + return Err(format!( + "Usage: {} ...", + env::args() + .next() + .unwrap_or_else(|| "uroborosql-lint-cli".to_string()) + )); + }; + + if args.next().is_some() { + return Err("Only a single SQL file can be specified".into()); + } + + let linter = Linter::new(); + let mut exit_with_error = false; + + let path = PathBuf::from(path_os); + let display = path.display().to_string(); + + let sql = + fs::read_to_string(&path).map_err(|err| format!("Failed to read {}: {}", display, err))?; + + match linter.run(&sql) { + Ok(diagnostics) => { + for diagnostic in diagnostics { + print_diagnostic(&display, &diagnostic); + } + } + Err(LintError::ParseError(message)) => { + eprintln!("{}: failed to parse SQL: {}", display, message); + exit_with_error = true; + } + } + + if exit_with_error { + Err("Linting finished with errors".into()) + } else { + Ok(()) + } +} + +fn print_diagnostic(file: &str, diagnostic: &Diagnostic) { + let line = diagnostic.span.start.line + 1; + let column = diagnostic.span.start.column + 1; + + println!( + "{}:{}:{}: {}: {}", + file, line, column, diagnostic.rule_id, diagnostic.message + ); +} diff --git a/crates/uroborosql-lint/Cargo.toml b/crates/uroborosql-lint/Cargo.toml new file mode 100644 index 00000000..6ec0d9d4 --- /dev/null +++ b/crates/uroborosql-lint/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "uroborosql-lint" +version = "0.1.0" +authors.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +postgresql-cst-parser = { workspace = true } diff --git a/crates/uroborosql-lint/src/context.rs b/crates/uroborosql-lint/src/context.rs new file mode 100644 index 00000000..e1479c76 --- /dev/null +++ b/crates/uroborosql-lint/src/context.rs @@ -0,0 +1,22 @@ +use crate::diagnostic::Diagnostic; + +/// Mutable linting context shared across rules. +pub struct LintContext { + diagnostics: Vec, +} + +impl LintContext { + pub fn new(_source: &str) -> Self { + Self { + diagnostics: Vec::new(), + } + } + + pub fn report(&mut self, diagnostic: Diagnostic) { + self.diagnostics.push(diagnostic); + } + + pub fn into_diagnostics(self) -> Vec { + self.diagnostics + } +} diff --git a/crates/uroborosql-lint/src/diagnostic.rs b/crates/uroborosql-lint/src/diagnostic.rs new file mode 100644 index 00000000..b96d674a --- /dev/null +++ b/crates/uroborosql-lint/src/diagnostic.rs @@ -0,0 +1,62 @@ +use postgresql_cst_parser::tree_sitter::Range; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Severity { + Error, + Warning, + Info, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Position { + pub line: usize, + pub column: usize, + pub byte: usize, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SqlSpan { + pub start: Position, + pub end: Position, +} + +impl SqlSpan { + pub fn from_range(range: &Range) -> Self { + SqlSpan { + start: Position { + line: range.start_position.row, + column: range.start_position.column, + byte: range.start_byte, + }, + end: Position { + line: range.end_position.row, + column: range.end_position.column, + byte: range.end_byte, + }, + } + } +} + +#[derive(Debug, Clone)] +pub struct Diagnostic { + pub rule_id: &'static str, + pub message: String, + pub severity: Severity, + pub span: SqlSpan, +} + +impl Diagnostic { + pub fn new( + rule_id: &'static str, + severity: Severity, + message: impl Into, + range: &Range, + ) -> Self { + Self { + rule_id, + severity, + message: message.into(), + span: SqlSpan::from_range(range), + } + } +} diff --git a/crates/uroborosql-lint/src/lib.rs b/crates/uroborosql-lint/src/lib.rs new file mode 100644 index 00000000..5b47ef8b --- /dev/null +++ b/crates/uroborosql-lint/src/lib.rs @@ -0,0 +1,9 @@ +mod context; +mod diagnostic; +mod linter; +mod rule; +mod rules; +mod tree; + +pub use diagnostic::{Diagnostic, Severity, SqlSpan}; +pub use linter::{LintError, LintOptions, Linter}; diff --git a/crates/uroborosql-lint/src/linter.rs b/crates/uroborosql-lint/src/linter.rs new file mode 100644 index 00000000..ed129563 --- /dev/null +++ b/crates/uroborosql-lint/src/linter.rs @@ -0,0 +1,139 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, + rules::{ + MissingTwoWaySample, NoDistinct, NoFunctionOnColumnInJoinOrWhere, NoNotIn, NoUnionDistinct, + NoWildcardProjection, TooLargeInList, + }, + tree::collect_preorder, +}; +use postgresql_cst_parser::tree_sitter; +use std::collections::HashMap; + +#[derive(Debug)] +pub enum LintError { + ParseError(String), +} + +#[derive(Debug, Clone, Default)] +pub struct LintOptions { + severity_overrides: HashMap, +} + +impl LintOptions { + pub fn new() -> Self { + Self::default() + } + + pub fn severity_for(&self, rule_id: &str) -> Option { + self.severity_overrides.get(rule_id).copied() + } + + pub fn set_severity_override(&mut self, rule_id: impl Into, severity: Severity) { + self.severity_overrides.insert(rule_id.into(), severity); + } + + pub fn with_severity_override( + mut self, + rule_id: impl Into, + severity: Severity, + ) -> Self { + self.set_severity_override(rule_id, severity); + self + } +} + +pub struct Linter { + rules: Vec>, + options: LintOptions, +} + +impl Default for Linter { + fn default() -> Self { + Self::new() + } +} + +impl Linter { + pub fn new() -> Self { + Self::with_options(LintOptions::default()) + } + + pub fn with_options(options: LintOptions) -> Self { + Self::with_rules_and_options(default_rules(), options) + } + + pub fn with_rules(rules: Vec>) -> Self { + Self::with_rules_and_options(rules, LintOptions::default()) + } + + pub fn with_rules_and_options(rules: Vec>, options: LintOptions) -> Self { + Self { rules, options } + } + + pub fn run(&self, sql: &str) -> Result, LintError> { + let tree = tree_sitter::parse_2way(sql) + .map_err(|err| LintError::ParseError(format!("{err:?}")))?; + let root = tree.root_node(); + let nodes = collect_preorder(root.clone()); + let mut ctx = LintContext::new(sql); + + for rule in &self.rules { + let severity = self + .options + .severity_for(rule.name()) + .unwrap_or_else(|| rule.default_severity()); + + rule.run_once(&root, &mut ctx, severity); + + let targets = rule.target_kinds(); + if targets.is_empty() { + for node in &nodes { + rule.run_on_node(node, &mut ctx, severity); + } + } else { + for node in &nodes { + if targets.iter().any(|kind| node.kind() == *kind) { + rule.run_on_node(node, &mut ctx, severity); + } + } + } + } + + Ok(ctx.into_diagnostics()) + } +} + +fn default_rules() -> Vec> { + vec![ + Box::new(NoDistinct), + Box::new(NoNotIn), + Box::new(NoUnionDistinct), + Box::new(NoFunctionOnColumnInJoinOrWhere), + Box::new(NoWildcardProjection), + Box::new(MissingTwoWaySample), + Box::new(TooLargeInList), + ] +} + +#[cfg(test)] +pub mod tests { + use super::*; + use crate::diagnostic::Severity; + + pub fn run_with_rules(sql: &str, rules: Vec>) -> Vec { + Linter::with_rules(rules).run(sql).expect("lint ok") + } + + #[test] + fn applies_severity_override() { + let options = LintOptions::default().with_severity_override("no-distinct", Severity::Error); + let linter = Linter::with_options(options); + let sql = "SELECT DISTINCT id FROM users;"; + let diagnostics = linter.run(sql).expect("lint ok"); + + assert_eq!(diagnostics.len(), 1); + assert_eq!(diagnostics[0].severity, Severity::Error); + } +} diff --git a/crates/uroborosql-lint/src/rule.rs b/crates/uroborosql-lint/src/rule.rs new file mode 100644 index 00000000..61291b66 --- /dev/null +++ b/crates/uroborosql-lint/src/rule.rs @@ -0,0 +1,13 @@ +use crate::{context::LintContext, diagnostic::Severity}; +use postgresql_cst_parser::{syntax_kind::SyntaxKind, tree_sitter::Node}; + +pub trait Rule: Send + Sync { + fn name(&self) -> &'static str; + fn default_severity(&self) -> Severity; + fn target_kinds(&self) -> &'static [SyntaxKind] { + &[] + } + fn run_once<'tree>(&self, _root: &Node<'tree>, _ctx: &mut LintContext, _severity: Severity) {} + fn run_on_node<'tree>(&self, _node: &Node<'tree>, _ctx: &mut LintContext, _severity: Severity) { + } +} diff --git a/crates/uroborosql-lint/src/rules.rs b/crates/uroborosql-lint/src/rules.rs new file mode 100644 index 00000000..3ba58daf --- /dev/null +++ b/crates/uroborosql-lint/src/rules.rs @@ -0,0 +1,15 @@ +mod missing_two_way_sample; +mod no_distinct; +mod no_function_on_column_in_join_or_where; +mod no_not_in; +mod no_union_distinct; +mod no_wildcard_projection; +mod too_large_in_list; + +pub use missing_two_way_sample::MissingTwoWaySample; +pub use no_distinct::NoDistinct; +pub use no_function_on_column_in_join_or_where::NoFunctionOnColumnInJoinOrWhere; +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/missing_two_way_sample.rs b/crates/uroborosql-lint/src/rules/missing_two_way_sample.rs new file mode 100644 index 00000000..a56fb297 --- /dev/null +++ b/crates/uroborosql-lint/src/rules/missing_two_way_sample.rs @@ -0,0 +1,162 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, +}; +use postgresql_cst_parser::{syntax_kind::SyntaxKind, tree_sitter::Node}; + +/// Detects 2WaySQL bind parameter without sample values (e.g. `/*param*/`). +/// Rule source: https://future-architect.github.io/uroborosql-doc/background/#バインドパラメータ +pub struct MissingTwoWaySample; + +impl Rule for MissingTwoWaySample { + fn name(&self) -> &'static str { + "missing-two-way-sample" + } + + fn default_severity(&self) -> Severity { + Severity::Warning + } + + fn target_kinds(&self) -> &'static [SyntaxKind] { + &[SyntaxKind::C_COMMENT] + } + + fn run_on_node<'tree>(&self, node: &Node<'tree>, ctx: &mut LintContext, severity: Severity) { + assert_eq!(node.kind(), SyntaxKind::C_COMMENT); + + let comment = node; + + // 置換文字列はサンプル値がないケースもあるため除外 + if matches!(comment.text().chars().nth(2).unwrap(), '#' | '$') { + return; + } + + let Some(next_token) = comment.next_token() else { + return; + }; + + // サンプル値抜けと判定する条件： + // - next_token のテキストが空文字（パーサのエラー回復によって挿入されたトークン） + // - next_token がコメント（元のノード）に隣接している + if !next_token.text().is_empty() || !next_token.range().is_adjacent(&comment.range()) { + return; + } + + let diagnostic = Diagnostic::new( + self.name(), + severity, + "Sample value for bind parameter is missing.", + &next_token.range(), + ); + ctx.report(diagnostic); + } +} + +#[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(MissingTwoWaySample)]) + } + + #[test] + fn allows_numeric_sample() { + let sql = "SELECT /*param1*/1;"; + let diagnostics = run(sql); + assert!(diagnostics.is_empty()); + } + + #[test] + fn allows_empty_literal_sample() { + let sql = "SELECT /*param1*/'';"; + let diagnostics = run(sql); + assert!(diagnostics.is_empty()); + } + + #[test] + fn detects_missing_sample() { + let sql = "SELECT /*name*/ as name from t;"; + let diagnostics = run(sql); + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "missing-two-way-sample") + .expect("should detect missing sample"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], ""); + } + + #[test] + fn detects_missing_sample_from_lists() { + let sql = "SELECT /*id*/1 as id, /*name*/ as name from t;"; + let diagnostics = run(sql); + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "missing-two-way-sample") + .expect("should detect missing sample"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], ""); + } + + #[test] + fn detects_missing_sample_in_select_multiple() { + let sql = "SELECT /*col*/ , /*col2*/ FROM t;"; + let diagnostics = run(sql); + assert!(diagnostics + .iter() + .all(|diag| diag.rule_id == "missing-two-way-sample")); + + assert_eq!(diagnostics.len(), 2); + + assert!(diagnostics.iter().all(|diagnostic| { + let SqlSpan { start, end } = diagnostic.span; + &sql[start.byte..end.byte] == "" + })); + } + + #[test] + fn detects_missing_sample_in_where_clause() { + let sql = "SELECT * FROM t WHERE col = /*col*/ AND col2 = /*col2*/ ;"; + let diagnostics = run(sql); + assert!(diagnostics + .iter() + .all(|diag| diag.rule_id == "missing-two-way-sample")); + + assert_eq!(diagnostics.len(), 2); + + assert!(diagnostics.iter().all(|diagnostic| { + let SqlSpan { start, end } = diagnostic.span; + &sql[start.byte..end.byte] == "" + })); + } + + #[test] + fn ignores_control_comment() { + let sql = r#"SELECT 1 from t + where id = 1 + /*IF cond*/ + OR id = 2 + /*END*/;"#; + + let diagnostics = run(sql); + assert!(diagnostics.is_empty()); + } + + #[test] + fn ignores_placeholder_without_sample_value() { + let sql = "SELECT * FROM /*#table*/, /*$table*/ ;"; + let diagnostics = run(sql); + assert!(diagnostics.is_empty()); + } + + #[test] + fn ignores_normal_comments() { + let sql = "SELECT /* comment 0 */ * /* comment 1 */ /* comment 2 */ FROM /* comment 3 */ t /* comment 4 */ ;"; + let diagnostics = run(sql); + assert!(diagnostics.is_empty()); + } +} diff --git a/crates/uroborosql-lint/src/rules/no_distinct.rs b/crates/uroborosql-lint/src/rules/no_distinct.rs new file mode 100644 index 00000000..c5d508ec --- /dev/null +++ b/crates/uroborosql-lint/src/rules/no_distinct.rs @@ -0,0 +1,59 @@ +use postgresql_cst_parser::{syntax_kind::SyntaxKind, tree_sitter::Node}; + +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, +}; + +/// 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#distinct-%E5%8F%A5 +pub struct NoDistinct; + +impl Rule for NoDistinct { + fn name(&self) -> &'static str { + "no-distinct" + } + + fn default_severity(&self) -> Severity { + Severity::Warning + } + + fn target_kinds(&self) -> &'static [SyntaxKind] { + &[SyntaxKind::DISTINCT] + } + + fn run_on_node<'tree>(&self, node: &Node<'tree>, ctx: &mut LintContext, severity: Severity) { + let diagnostic = Diagnostic::new( + self.name(), + severity, + "DISTINCT is not recommended.", + &node.range(), + ); + ctx.report(diagnostic); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{diagnostic::Severity, linter::tests::run_with_rules, rule::Rule}; + + #[test] + fn detects_distinct_keyword() { + let sql = "SELECT DISTINCT id FROM users;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoDistinct) as Box]); + assert_eq!(diagnostics.len(), 1); + let diagnostic = &diagnostics[0]; + assert_eq!(diagnostic.rule_id, "no-distinct"); + assert_eq!(diagnostic.severity, Severity::Warning); + assert!(sql[diagnostic.span.start.byte..diagnostic.span.end.byte] + .eq_ignore_ascii_case("distinct")); + } + + #[test] + fn ignores_select_without_distinct() { + let sql = "SELECT id FROM users;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoDistinct) as Box]); + assert!(diagnostics.is_empty()); + } +} diff --git a/crates/uroborosql-lint/src/rules/no_function_on_column_in_join_or_where.rs b/crates/uroborosql-lint/src/rules/no_function_on_column_in_join_or_where.rs new file mode 100644 index 00000000..03dde4ad --- /dev/null +++ b/crates/uroborosql-lint/src/rules/no_function_on_column_in_join_or_where.rs @@ -0,0 +1,318 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, +}; +use postgresql_cst_parser::{ + syntax_kind::SyntaxKind, + tree_sitter::{Node, Range}, +}; + +/// Detects function usage on columns in JOIN or WHERE conditions. +/// 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#:~:text=1-,%E3%82%A4%E3%83%B3%E3%83%87%E3%83%83%E3%82%AF%E3%82%B9%E3%82%AB%E3%83%A9%E3%83%A0%E3%81%AB%E9%96%A2%E6%95%B0,-%E3%82%92%E9%80%9A%E3%81%97%E3%81%9F +pub struct NoFunctionOnColumnInJoinOrWhere; + +struct Report<'a> { + func_expr_range: Range, + columnref_text: &'a str, +} + +impl Rule for NoFunctionOnColumnInJoinOrWhere { + fn name(&self) -> &'static str { + "no-function-on-column-in-join-or-where" + } + + fn default_severity(&self) -> Severity { + Severity::Warning + } + + fn target_kinds(&self) -> &'static [SyntaxKind] { + &[SyntaxKind::columnref] + } + + fn run_on_node<'tree>(&self, node: &Node<'tree>, ctx: &mut LintContext, severity: Severity) { + assert_eq!(node.kind(), SyntaxKind::columnref); + + let Some(report) = detect(node) else { + return; + }; + + let Report { + func_expr_range, + columnref_text, + } = report; + + let message = format!("Functions in JOIN or WHERE conditions can prevent index usage; rewrite without wrapping the column. {}", columnref_text); + + let diagnostic = Diagnostic::new(self.name(), severity, message, &func_expr_range); + ctx.report(diagnostic); + } +} + +fn detect<'a>(columnref: &'a Node) -> Option> { + let func_expr = get_wrapping_func_expr(columnref)?; + + if !is_in_detection_range(&func_expr) { + return None; + } + + Some(Report { + func_expr_range: func_expr.range(), + columnref_text: columnref.text(), + }) +} + +/// Returns the nearest ancestor `func_expr` that directly wraps the given `columnref`. +/// The traversal stops when clauses that cannot appear inside JOIN/WHERE conditions are encountered. +fn get_wrapping_func_expr<'a>(columnref: &'a Node) -> Option> { + let mut node = columnref.parent(); + while let Some(current) = node { + match current.kind() { + SyntaxKind::func_expr => return Some(current), + // Stop if we enter a SELECT body before finding a wrapping func_expr. + SyntaxKind::select_no_parens + | SyntaxKind::filter_clause + | SyntaxKind::over_clause + // FILTER/OVER/WITHIN GROUP never appear inside JOIN/WHERE predicates. + | SyntaxKind::within_group_clause => return None, + _ => (), + } + + node = current.parent(); + } + + None +} + +fn is_in_detection_range(func_expr: &Node) -> bool { + // Traverse upward; if `join_qual` or `where_clause` is found, it's in the detection range + // If `select_no_parens` is encountered before reaching `join_qual` or `where_clause`, it's outside the detection range + + let mut node = func_expr.parent(); + while let Some(current) = node { + match current.kind() { + SyntaxKind::join_qual | SyntaxKind::where_clause => return true, + SyntaxKind::select_no_parens => return false, + _ => (), + } + node = current.parent(); + } + + false +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{linter::tests::run_with_rules, Diagnostic, SqlSpan}; + + fn run(sql: &str) -> Vec { + run_with_rules(sql, vec![Box::new(NoFunctionOnColumnInJoinOrWhere)]) + } + + mod where_clause { + use super::*; + + #[test] + fn allows_plain_column_comparisons() { + let sql = "SELECT * FROM users WHERE users.id = 1;"; + let diagnostics = run(sql); + + assert!(diagnostics.is_empty(),); + } + + #[test] + fn allows_function_with_constant() { + let sql = + "SELECT * FROM users WHERE users.created_at >= to_date('20160101', 'YYYYMMDD');"; + let diagnostics = run(sql); + + assert!(diagnostics.is_empty()); + } + + #[test] + fn detects_function_in_where_clause() { + let sql = "SELECT * FROM users WHERE lower(users.name) = 'foo';"; + let diagnostics = run(sql); + + assert!(diagnostics + .iter() + .any(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),); + + assert_eq!(diagnostics.len(), 1); + + let SqlSpan { start, end } = diagnostics[0].span; + assert_eq!(&sql[start.byte..end.byte], "lower(users.name)"); + } + + #[test] + fn detects_coalesce_usage() { + let sql = "SELECT * FROM users WHERE coalesce(users.deleted_at, users.updated_at) IS NOT NULL;"; + let diagnostics = run(sql); + + assert_eq!(diagnostics.len(), 2); + + assert!(diagnostics.iter().all(|diag| { + let SqlSpan { start, end } = diag.span; + &sql[start.byte..end.byte] == "coalesce(users.deleted_at, users.updated_at)" + })); + } + + #[test] + fn detects_function_with_mixed_arguments() { + let sql = "SELECT * FROM users WHERE coalesce(users.deleted_at, trim(users.name)) IS NOT NULL;"; + let diagnostics = run(sql); + + assert_eq!( + diagnostics.len(), + 2, + "coalesce and trim should both be reported when they wrap columns" + ); + + let spans: Vec<_> = diagnostics + .iter() + .map(|diag| &sql[diag.span.start.byte..diag.span.end.byte]) + .collect(); + + assert!( + spans.contains(&"trim(users.name)"), + "trim should be reported" + ); + + assert!( + spans.contains(&"coalesce(users.deleted_at, trim(users.name))"), + "outer coalesce should still be reported" + ); + } + + #[test] + fn detects_only_innermost_function_in_nested_chain() { + let sql = "SELECT * FROM users WHERE lower(trim(users.email)) = 'foo';"; + let diagnostics = run(sql); + + assert_eq!( + diagnostics.len(), + 1, + "only the innermost function wrapping the column should be flagged" + ); + + let SqlSpan { start, end } = diagnostics[0].span; + assert_eq!(&sql[start.byte..end.byte], "trim(users.email)"); + } + + #[test] + fn allows_function_on_other_branch_without_column_reference() { + let sql = + "SELECT * FROM users u JOIN vendors v ON u.id = v.user_id WHERE (u.name = v.name) OR 'const' = lower('CONST');"; + let diagnostics = run(sql); + + assert!( + diagnostics.is_empty(), + "functions that do not reference columns should be allowed even when other branches compare columns" + ); + } + + #[test] + fn detects_function_on_both_sides() { + let sql = "SELECT * FROM users u1 JOIN users u2 ON trim(u1.email) = trim(u2.email);"; + let diagnostics = run(sql); + + assert!(diagnostics + .iter() + .all(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),); + + assert_eq!( + diagnostics.len(), + 2, + "expected two diagnostics for functions on both sides" + ); + + let spans: Vec<_> = diagnostics + .iter() + .map(|diag| &sql[diag.span.start.byte..diag.span.end.byte]) + .collect(); + assert!( + spans.iter().any(|s| s.contains("trim(u1.email)")), + "expected trim() function on left side to be detected" + ); + assert!( + spans.iter().any(|s| s.contains("trim(u2.email)")), + "expected trim() function on right side to be detected" + ); + } + } + + mod join_qual { + + use super::*; + + #[test] + fn allows_function_with_constant() { + let sql = + "SELECT * FROM t1 JOIN t2 ON t1.created_at >= to_date('20160101', 'YYYYMMDD');"; + let diagnostics = run(sql); + + assert!(diagnostics.is_empty(),); + } + + #[test] + fn detects_function_in_join_condition() { + let sql = "SELECT * FROM t1 JOIN t2 ON lower(t1.name) = t2.name;"; + let diagnostics = run(sql); + + assert!(diagnostics + .iter() + .any(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),); + + assert_eq!(diagnostics.len(), 1); + + let SqlSpan { start, end } = diagnostics[0].span; + assert_eq!(&sql[start.byte..end.byte], "lower(t1.name)"); + } + + #[test] + fn detects_function_on_both_sides() { + let sql = "SELECT * FROM t1 JOIN t2 ON trim(t1.email) = trim(t2.email);"; + let diagnostics = run(sql); + + assert!(diagnostics + .iter() + .all(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),); + + assert_eq!(diagnostics.len(), 2,); + + let spans: Vec<_> = diagnostics + .iter() + .map(|diag| &sql[diag.span.start.byte..diag.span.end.byte]) + .collect(); + assert!( + spans.iter().any(|s| s.contains("trim(t1.email)")), + "expected trim() function on left side to be detected" + ); + assert!( + spans.iter().any(|s| s.contains("trim(t2.email)")), + "expected trim() function on right side to be detected" + ); + } + } + + mod other_location { + use super::*; + + #[test] + fn allows_function_in_select_column() { + let sql = "SELECT func(col) FROM tbl;"; + let diagnostics = run(sql); + + assert!(diagnostics.is_empty(),); + } + + #[test] + fn allows_function_in_subquery() { + let sql = "SELECT * FROM tbl WHERE col IN (SELECT func(col) FROM tbl);"; + let diagnostics = run(sql); + + assert!(diagnostics.is_empty(),); + } + } +} 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..f811f8c9 --- /dev/null +++ b/crates/uroborosql-lint/src/rules/no_not_in.rs @@ -0,0 +1,125 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, + tree::prev_node_skipping_comments, +}; +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` sequence. + // 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())) +} + +#[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"); + } +} diff --git a/crates/uroborosql-lint/src/rules/no_union_distinct.rs b/crates/uroborosql-lint/src/rules/no_union_distinct.rs new file mode 100644 index 00000000..8bca8626 --- /dev/null +++ b/crates/uroborosql-lint/src/rules/no_union_distinct.rs @@ -0,0 +1,154 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, +}; +use postgresql_cst_parser::{ + syntax_kind::SyntaxKind, + tree_sitter::{Node, Range}, +}; + +/// 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#union-%E5%8F%A5 +pub struct NoUnionDistinct; + +impl Rule for NoUnionDistinct { + fn name(&self) -> &'static str { + "no-union-distinct" + } + + fn default_severity(&self) -> Severity { + Severity::Warning + } + + fn target_kinds(&self) -> &'static [SyntaxKind] { + &[SyntaxKind::UNION] + } + + fn run_on_node<'tree>(&self, node: &Node<'tree>, ctx: &mut LintContext, severity: Severity) { + let Some(range) = detect_violation_range(node) else { + return; + }; + + let diagnostic = Diagnostic::new( + self.name(), + severity, + "Use of `UNION DISTINCT` is not recommended. (`UNION` is implicitly `UNION DISTINCT`)", + &range, + ); + ctx.report(diagnostic); + } +} + +fn detect_violation_range(node_union: &Node<'_>) -> Option { + assert_eq!(node_union.kind(), SyntaxKind::UNION); + + let mut cursor = node_union.walk(); + cursor.goto_next_sibling(); + + // skip comments + while cursor.node().is_comment() { + cursor.goto_next_sibling(); + } + + // cursor -> set_quantifier | SELECT (select_clause) + match cursor.node().kind() { + SyntaxKind::set_quantifier => { + // set_quantifer has ALL or DISTINCT + + let has_all = cursor + .node() + .children() + .iter() + .any(|child| child.kind() == SyntaxKind::ALL); + + if has_all { + // `UNION ALL` is allowed + None + } else { + // `UNION DISTINCT` is NOT allowed + Some(extend_range(node_union.range(), cursor.node().range())) + } + } + SyntaxKind::SELECT => { + // Only `UNION` pattern also means `UNION DISTINCT` implicitly + Some(node_union.range()) + } + _ => unreachable!( + "CST structure error: union should be followed by set_quantifier or SELECT" + ), + } +} + +fn extend_range(base: Range, extension: Range) -> Range { + Range { + start_byte: base.start_byte, + end_byte: extension.end_byte, + start_position: base.start_position, + end_position: extension.end_position, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{linter::tests::run_with_rules, SqlSpan}; + + #[test] + fn detects_union_without_all() { + let sql = "SELECT 1 UNION SELECT 2;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoUnionDistinct)]); + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-union-distinct") + .expect("should detect UNION"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "UNION"); + } + + #[test] + fn detects_union_distinct() { + let sql = "SELECT 1 UNION DISTINCT SELECT 2;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoUnionDistinct)]); + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-union-distinct") + .expect("should detect UNION"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "UNION DISTINCT"); + } + + #[test] + fn detects_union_distinct_with_comment() { + let sql = "SELECT 1 UNION /* comment */ DISTINCT SELECT 2;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoUnionDistinct)]); + let diagnostic = diagnostics + .iter() + .find(|diag| diag.rule_id == "no-union-distinct") + .expect("should detect UNION DISTINCT"); + + let SqlSpan { start, end } = diagnostic.span; + assert_eq!(&sql[start.byte..end.byte], "UNION /* comment */ DISTINCT"); + } + + #[test] + fn allows_union_all() { + let sql = "SELECT 1 UNION ALL SELECT 2;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoUnionDistinct)]); + assert!( + diagnostics + .iter() + .all(|diag| diag.rule_id != "no-union-distinct"), + "UNION ALL should not trigger no-union-distinct rule" + ); + } + + #[test] + fn allows_union_all_with_comment() { + let sql = "SELECT 1 UNION /* comment */ ALL SELECT 2;"; + let diagnostics = run_with_rules(sql, vec![Box::new(NoUnionDistinct)]); + + assert_eq!(diagnostics.len(), 0); + } +} 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..d604cdcf --- /dev/null +++ b/crates/uroborosql-lint/src/rules/no_wildcard_projection.rs @@ -0,0 +1,132 @@ +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); + + // 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)] +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 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] + 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"); + } +} diff --git a/crates/uroborosql-lint/src/rules/too_large_in_list.rs b/crates/uroborosql-lint/src/rules/too_large_in_list.rs new file mode 100644 index 00000000..674c0872 --- /dev/null +++ b/crates/uroborosql-lint/src/rules/too_large_in_list.rs @@ -0,0 +1,114 @@ +use crate::{ + context::LintContext, + diagnostic::{Diagnostic, Severity}, + rule::Rule, +}; +use postgresql_cst_parser::{syntax_kind::SyntaxKind, tree_sitter::Node}; + +const MAX_IN_ELEMENTS: usize = 100; + +/// 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#in-%E5%8F%A5-1 +pub struct TooLargeInList; + +impl Rule for TooLargeInList { + fn name(&self) -> &'static str { + "too-large-in-list" + } + + 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(count) = count_expr_list_elements(node) else { + return; + }; + + if count <= MAX_IN_ELEMENTS { + return; + } + + let message = format!("IN list has {count} elements (limit: {MAX_IN_ELEMENTS})"); + + let diagnostic = Diagnostic::new(self.name(), severity, message, &node.range()); + ctx.report(diagnostic); + } +} + +fn count_expr_list_elements(node: &Node<'_>) -> Option { + // find expr_list among children + let expr_list = node + .children() + .into_iter() + .find(|child| child.kind() == SyntaxKind::expr_list)?; + + let count = expr_list + .children() + .into_iter() + .filter(|child| !child.is_comment()) + .filter(|child| child.kind() == SyntaxKind::a_expr) + .count(); + + Some(count) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::linter::tests::run_with_rules; + + fn run(sql: &str) -> Vec { + run_with_rules(sql, vec![Box::new(TooLargeInList)]) + } + + fn repetitions(count: usize) -> String { + (0..count) + .map(|i| i.to_string()) + .collect::>() + .join(", ") + } + + #[test] + fn allows_at_threshold() { + let sql = format!( + "SELECT * FROM users WHERE id IN ({});", + repetitions(MAX_IN_ELEMENTS) + ); + let diagnostics = run(&sql); + assert!(diagnostics.is_empty()); + } + + #[test] + fn warns_over_threshold() { + let sql = format!( + "SELECT * FROM users WHERE id IN ({});", + repetitions(MAX_IN_ELEMENTS + 1) + ); + let diagnostics = run(&sql); + assert_eq!(diagnostics.len(), 1); + assert_eq!(diagnostics[0].rule_id, "too-large-in-list"); + } + + #[test] + fn ignores_subquery() { + let sql = "SELECT * FROM users WHERE id IN (SELECT id FROM admins);"; + let diagnostics = run(sql); + assert!(diagnostics.is_empty()); + } + + #[test] + fn handles_comments_inside_list() { + let sql = format!( + "SELECT * FROM users WHERE id IN (1, /*a*/ 2, 3 {});", + (4..=(MAX_IN_ELEMENTS + 1)) + .map(|i| format!(", /*c*/ {}", i)) + .collect::() + ); + let diagnostics = run(&sql); + assert_eq!(diagnostics.len(), 1); + } +} diff --git a/crates/uroborosql-lint/src/tree.rs b/crates/uroborosql-lint/src/tree.rs new file mode 100644 index 00000000..d71c5650 --- /dev/null +++ b/crates/uroborosql-lint/src/tree.rs @@ -0,0 +1,47 @@ +use postgresql_cst_parser::tree_sitter::Node; + +/// Collect all nodes in preorder traversal. +pub fn collect_preorder<'tree>(root: Node<'tree>) -> Vec> { + let mut nodes = Vec::new(); + walk_preorder(root, &mut nodes, |vec, node| vec.push(node)); + nodes +} + +fn walk_preorder<'tree, T>( + root: Node<'tree>, + state: &mut T, + mut visit: impl FnMut(&mut T, Node<'tree>), +) { + let mut cursor = root.walk(); + + loop { + visit(state, cursor.node()); + + if cursor.goto_first_child() { + continue; + } + + loop { + if cursor.goto_next_sibling() { + break; + } + + if !cursor.goto_parent() { + return; + } + } + } +} + +/// 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; + } +}