diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index 633a7dc4..eac2c3ca 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -54,6 +54,7 @@ use rules::renaming_table; use rules::require_concurrent_index_creation; use rules::require_concurrent_index_deletion; use rules::require_enum_value_ordering; +use rules::require_table_schema; use rules::require_timeout_settings; use rules::transaction_nesting; // xtask:new-rule:rule-import @@ -92,9 +93,18 @@ pub enum Rule { RequireTimeoutSettings, BanUncommittedTransaction, RequireEnumValueOrdering, + RequireTableSchema, // xtask:new-rule:error-name } +impl Rule { + /// Rules that are opt-in are not enabled by default. + /// They must be explicitly included via configuration. + pub fn is_opt_in(&self) -> bool { + matches!(self, Rule::RequireTableSchema) + } +} + impl TryFrom<&str> for Rule { type Error = String; @@ -135,6 +145,7 @@ impl TryFrom<&str> for Rule { "require-timeout-settings" => Ok(Rule::RequireTimeoutSettings), "ban-uncommitted-transaction" => Ok(Rule::BanUncommittedTransaction), "require-enum-value-ordering" => Ok(Rule::RequireEnumValueOrdering), + "require-table-schema" => Ok(Rule::RequireTableSchema), // xtask:new-rule:str-name _ => Err(format!("Unknown violation name: {s}")), } @@ -198,6 +209,7 @@ impl fmt::Display for Rule { Rule::RequireTimeoutSettings => "require-timeout-settings", Rule::BanUncommittedTransaction => "ban-uncommitted-transaction", Rule::RequireEnumValueOrdering => "require-enum-value-ordering", + Rule::RequireTableSchema => "require-table-schema", // xtask:new-rule:variant-to-name }; write!(f, "{val}") @@ -428,6 +440,9 @@ impl Linter { if self.rules.contains(&Rule::RequireEnumValueOrdering) { require_enum_value_ordering(self, file); } + if self.rules.contains(&Rule::RequireTableSchema) { + require_table_schema(self, file); + } // xtask:new-rule:rule-call // locate any ignores in the file @@ -452,12 +467,16 @@ impl Linter { } pub fn with_all_rules() -> Self { - let rules = all::().collect::>(); + let rules = all::() + .filter(|r| !r.is_opt_in()) + .collect::>(); Linter::from(rules) } pub fn without_rules(exclude: &[Rule]) -> Self { - let all_rules = all::().collect::>(); + let all_rules = all::() + .filter(|r| !r.is_opt_in()) + .collect::>(); let mut exclude_set = FxHashSet::with_capacity_and_hasher(exclude.len(), FxBuildHasher); for e in exclude { exclude_set.insert(e); diff --git a/crates/squawk_linter/src/rules/mod.rs b/crates/squawk_linter/src/rules/mod.rs index 1cc5fe98..bd5069f2 100644 --- a/crates/squawk_linter/src/rules/mod.rs +++ b/crates/squawk_linter/src/rules/mod.rs @@ -27,6 +27,7 @@ pub(crate) mod renaming_table; pub(crate) mod require_concurrent_index_creation; pub(crate) mod require_concurrent_index_deletion; pub(crate) mod require_enum_value_ordering; +pub(crate) mod require_table_schema; pub(crate) mod require_timeout_settings; pub(crate) mod transaction_nesting; // xtask:new-rule:mod-decl @@ -60,6 +61,7 @@ pub(crate) use renaming_table::renaming_table; pub(crate) use require_concurrent_index_creation::require_concurrent_index_creation; pub(crate) use require_concurrent_index_deletion::require_concurrent_index_deletion; pub(crate) use require_enum_value_ordering::require_enum_value_ordering; +pub(crate) use require_table_schema::require_table_schema; pub(crate) use require_timeout_settings::require_timeout_settings; pub(crate) use transaction_nesting::transaction_nesting; // xtask:new-rule:export diff --git a/crates/squawk_linter/src/rules/require_table_schema.rs b/crates/squawk_linter/src/rules/require_table_schema.rs new file mode 100644 index 00000000..39add30a --- /dev/null +++ b/crates/squawk_linter/src/rules/require_table_schema.rs @@ -0,0 +1,113 @@ +use squawk_syntax::{ + Parse, SourceFile, + ast::{self, AstNode}, +}; + +use crate::{Linter, Rule, Violation}; + +pub(crate) fn require_table_schema(ctx: &mut Linter, parse: &Parse) { + let file = parse.tree(); + for stmt in file.stmts() { + match stmt { + ast::Stmt::CreateTable(create_table) => { + check_path(ctx, create_table.path(), create_table.syntax()); + } + ast::Stmt::CreateTableAs(create_table_as) => { + check_path(ctx, create_table_as.path(), create_table_as.syntax()); + } + ast::Stmt::AlterTable(alter_table) => { + let path = alter_table.relation_name().and_then(|r| r.path()); + check_path(ctx, path, alter_table.syntax()); + } + ast::Stmt::DropTable(drop_table) => { + check_path(ctx, drop_table.path(), drop_table.syntax()); + } + _ => {} + } + } +} + +fn check_path(ctx: &mut Linter, path: Option, syntax: &squawk_syntax::SyntaxNode) { + if let Some(path) = path { + if path.qualifier().is_none() { + ctx.report(Violation::for_node( + Rule::RequireTableSchema, + "Table name is not schema-qualified. Use schema.table (e.g., public.my_table)." + .into(), + syntax, + )); + } + } +} + +#[cfg(test)] +mod test { + use insta::assert_snapshot; + + use crate::Rule; + use crate::test_utils::{lint_errors, lint_ok}; + + #[test] + fn create_table_err() { + let sql = r#" +CREATE TABLE my_table (id int); +"#; + assert_snapshot!(lint_errors(sql, Rule::RequireTableSchema)); + } + + #[test] + fn create_table_ok() { + let sql = r#" +CREATE TABLE public.my_table (id int); +"#; + lint_ok(sql, Rule::RequireTableSchema); + } + + #[test] + fn alter_table_err() { + let sql = r#" +ALTER TABLE my_table ADD COLUMN name text; +"#; + assert_snapshot!(lint_errors(sql, Rule::RequireTableSchema)); + } + + #[test] + fn alter_table_ok() { + let sql = r#" +ALTER TABLE public.my_table ADD COLUMN name text; +"#; + lint_ok(sql, Rule::RequireTableSchema); + } + + #[test] + fn drop_table_err() { + let sql = r#" +DROP TABLE my_table; +"#; + assert_snapshot!(lint_errors(sql, Rule::RequireTableSchema)); + } + + #[test] + fn drop_table_ok() { + let sql = r#" +DROP TABLE public.my_table; +"#; + lint_ok(sql, Rule::RequireTableSchema); + } + + #[test] + fn create_table_as_err() { + let sql = r#" +CREATE TABLE my_table AS SELECT 1; +"#; + assert_snapshot!(lint_errors(sql, Rule::RequireTableSchema)); + } + + #[test] + fn create_table_as_ok() { + let sql = r#" +CREATE TABLE public.my_table AS SELECT 1; +"#; + lint_ok(sql, Rule::RequireTableSchema); + } +} diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__alter_table_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__alter_table_err.snap new file mode 100644 index 00000000..3923ab72 --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__alter_table_err.snap @@ -0,0 +1,8 @@ +--- +source: crates/squawk_linter/src/rules/require_table_schema.rs +expression: "lint_errors(sql, Rule::RequireTableSchema)" +--- +warning[require-table-schema]: Table name is not schema-qualified. Use schema.table (e.g., public.my_table). + ╭▸ +2 │ ALTER TABLE my_table ADD COLUMN name text; + ╰╴━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_as_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_as_err.snap new file mode 100644 index 00000000..740a37cd --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_as_err.snap @@ -0,0 +1,8 @@ +--- +source: crates/squawk_linter/src/rules/require_table_schema.rs +expression: "lint_errors(sql, Rule::RequireTableSchema)" +--- +warning[require-table-schema]: Table name is not schema-qualified. Use schema.table (e.g., public.my_table). + ╭▸ +2 │ CREATE TABLE my_table AS SELECT 1; + ╰╴━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_err.snap new file mode 100644 index 00000000..b3583635 --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_err.snap @@ -0,0 +1,8 @@ +--- +source: crates/squawk_linter/src/rules/require_table_schema.rs +expression: "lint_errors(sql, Rule::RequireTableSchema)" +--- +warning[require-table-schema]: Table name is not schema-qualified. Use schema.table (e.g., public.my_table). + ╭▸ +2 │ CREATE TABLE my_table (id int); + ╰╴━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__drop_table_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__drop_table_err.snap new file mode 100644 index 00000000..a38c54b4 --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__drop_table_err.snap @@ -0,0 +1,8 @@ +--- +source: crates/squawk_linter/src/rules/require_table_schema.rs +expression: "lint_errors(sql, Rule::RequireTableSchema)" +--- +warning[require-table-schema]: Table name is not schema-qualified. Use schema.table (e.g., public.my_table). + ╭▸ +2 │ DROP TABLE my_table; + ╰╴━━━━━━━━━━━━━━━━━━━