From 9fff511aa8cc34b15b629326f5681a6d4b46aa5b Mon Sep 17 00:00:00 2001 From: Max Bigin Date: Tue, 7 Apr 2026 23:41:07 +0200 Subject: [PATCH 1/2] linter: add require-table-schema rule --- crates/squawk_linter/src/ignore.rs | 37 ++++-- crates/squawk_linter/src/lib.rs | 7 ++ crates/squawk_linter/src/rules/mod.rs | 2 + .../src/rules/require_table_schema.rs | 113 ++++++++++++++++++ ...e_table_schema__test__alter_table_err.snap | 8 ++ ...ble_schema__test__create_table_as_err.snap | 8 ++ ..._table_schema__test__create_table_err.snap | 8 ++ ...re_table_schema__test__drop_table_err.snap | 8 ++ 8 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 crates/squawk_linter/src/rules/require_table_schema.rs create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__alter_table_err.snap create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_as_err.snap create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__create_table_err.snap create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_table_schema__test__drop_table_err.snap diff --git a/crates/squawk_linter/src/ignore.rs b/crates/squawk_linter/src/ignore.rs index 9e7cffc4..0797a827 100644 --- a/crates/squawk_linter/src/ignore.rs +++ b/crates/squawk_linter/src/ignore.rs @@ -157,7 +157,7 @@ alter table t drop column c cascade; let sql = " -- fooo bar -- buzz --- squawk-ignore prefer-robust-stmts, require-timeout-settings +-- squawk-ignore prefer-robust-stmts, require-timeout-settings, require-table-schema create table x(); select 1; @@ -242,15 +242,16 @@ alter table t drop column c cascade; fn ignore_multiple_stmts() { let mut linter = Linter::with_all_rules(); let sql = r#" --- squawk-ignore ban-char-field,prefer-robust-stmts,require-timeout-settings +-- squawk-ignore ban-char-field,prefer-robust-stmts,require-timeout-settings,require-table-schema alter table t add column c char; +-- squawk-ignore require-table-schema ALTER TABLE foo -- squawk-ignore adding-field-with-default,prefer-robust-stmts -ADD COLUMN bar numeric GENERATED +ADD COLUMN bar numeric GENERATED ALWAYS AS (bar + baz) STORED; --- squawk-ignore prefer-robust-stmts +-- squawk-ignore prefer-robust-stmts,require-table-schema create table users ( ); "#; @@ -311,6 +312,13 @@ create table users ( }, ), }, + Violation { + code: RequireTableSchema, + message: "Table name is not schema-qualified. Use schema.table (e.g., public.my_table).", + text_range: 0..31, + help: None, + fix: None, + }, Violation { code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", @@ -357,7 +365,7 @@ create table users ( fn regression_unknown_name() { let mut linter = Linter::with_all_rules(); let sql = r#" --- squawk-ignore prefer-robust-stmts, require-timeout-settings +-- squawk-ignore prefer-robust-stmts, require-timeout-settings, require-table-schema create table test_table ( -- squawk-ignore prefer-timestamp-tz created_at timestamp default current_timestamp, @@ -485,7 +493,9 @@ alter table t2 drop column c2 cascade; [ RequireTimeoutSettings, RequireTimeoutSettings, + RequireTableSchema, PreferRobustStmts, + RequireTableSchema, PreferRobustStmts, ] "); @@ -511,7 +521,9 @@ alter table t2 drop column c2 cascade; [ RequireTimeoutSettings, RequireTimeoutSettings, + RequireTableSchema, PreferRobustStmts, + RequireTableSchema, PreferRobustStmts, ] "); @@ -544,6 +556,10 @@ alter table t2 drop column c2 cascade; RequireTimeoutSettings, "Missing `set statement_timeout` before potentially slow operations", ), + ( + RequireTableSchema, + "Table name is not schema-qualified. Use schema.table (e.g., public.my_table).", + ), ( BanDropColumn, "Dropping a column may break existing clients.", @@ -552,6 +568,10 @@ alter table t2 drop column c2 cascade; PreferRobustStmts, "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.", ), + ( + RequireTableSchema, + "Table name is not schema-qualified. Use schema.table (e.g., public.my_table).", + ), ( BanDropColumn, "Dropping a column may break existing clients.", @@ -580,11 +600,13 @@ alter table t2 drop column c2 cascade; .map(|x| x.code) .collect(); - assert_debug_snapshot!(errors, @" + assert_debug_snapshot!(errors, @r" [ RequireTimeoutSettings, RequireTimeoutSettings, + RequireTableSchema, PreferRobustStmts, + RequireTableSchema, PreferRobustStmts, ] "); @@ -605,10 +627,11 @@ alter table t drop column c cascade; .map(|x| x.code) .collect(); - assert_debug_snapshot!(errors, @" + assert_debug_snapshot!(errors, @r" [ RequireTimeoutSettings, RequireTimeoutSettings, + RequireTableSchema, ] "); } diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index 633a7dc4..c9bb196f 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -55,6 +55,7 @@ use rules::require_concurrent_index_creation; use rules::require_concurrent_index_deletion; use rules::require_enum_value_ordering; use rules::require_timeout_settings; +use rules::require_table_schema; use rules::transaction_nesting; // xtask:new-rule:rule-import @@ -92,6 +93,7 @@ pub enum Rule { RequireTimeoutSettings, BanUncommittedTransaction, RequireEnumValueOrdering, + RequireTableSchema, // xtask:new-rule:error-name } @@ -135,6 +137,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 +201,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 +432,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 diff --git a/crates/squawk_linter/src/rules/mod.rs b/crates/squawk_linter/src/rules/mod.rs index 1cc5fe98..626306e9 100644 --- a/crates/squawk_linter/src/rules/mod.rs +++ b/crates/squawk_linter/src/rules/mod.rs @@ -28,6 +28,7 @@ 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_timeout_settings; +pub(crate) mod require_table_schema; pub(crate) mod transaction_nesting; // xtask:new-rule:mod-decl @@ -61,5 +62,6 @@ pub(crate) use require_concurrent_index_creation::require_concurrent_index_creat 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_timeout_settings::require_timeout_settings; +pub(crate) use require_table_schema::require_table_schema; 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; + ╰╴━━━━━━━━━━━━━━━━━━━ From 1a2db47cd2ace46d8ad018c7a7995c8158f849ef Mon Sep 17 00:00:00 2001 From: Max Bigin Date: Tue, 7 Apr 2026 23:55:26 +0200 Subject: [PATCH 2/2] linter: make require-table-schema opt-in by default --- crates/squawk_linter/src/ignore.rs | 37 +++++---------------------- crates/squawk_linter/src/lib.rs | 18 ++++++++++--- crates/squawk_linter/src/rules/mod.rs | 4 +-- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/crates/squawk_linter/src/ignore.rs b/crates/squawk_linter/src/ignore.rs index 0797a827..9e7cffc4 100644 --- a/crates/squawk_linter/src/ignore.rs +++ b/crates/squawk_linter/src/ignore.rs @@ -157,7 +157,7 @@ alter table t drop column c cascade; let sql = " -- fooo bar -- buzz --- squawk-ignore prefer-robust-stmts, require-timeout-settings, require-table-schema +-- squawk-ignore prefer-robust-stmts, require-timeout-settings create table x(); select 1; @@ -242,16 +242,15 @@ alter table t drop column c cascade; fn ignore_multiple_stmts() { let mut linter = Linter::with_all_rules(); let sql = r#" --- squawk-ignore ban-char-field,prefer-robust-stmts,require-timeout-settings,require-table-schema +-- squawk-ignore ban-char-field,prefer-robust-stmts,require-timeout-settings alter table t add column c char; --- squawk-ignore require-table-schema ALTER TABLE foo -- squawk-ignore adding-field-with-default,prefer-robust-stmts -ADD COLUMN bar numeric GENERATED +ADD COLUMN bar numeric GENERATED ALWAYS AS (bar + baz) STORED; --- squawk-ignore prefer-robust-stmts,require-table-schema +-- squawk-ignore prefer-robust-stmts create table users ( ); "#; @@ -312,13 +311,6 @@ create table users ( }, ), }, - Violation { - code: RequireTableSchema, - message: "Table name is not schema-qualified. Use schema.table (e.g., public.my_table).", - text_range: 0..31, - help: None, - fix: None, - }, Violation { code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", @@ -365,7 +357,7 @@ create table users ( fn regression_unknown_name() { let mut linter = Linter::with_all_rules(); let sql = r#" --- squawk-ignore prefer-robust-stmts, require-timeout-settings, require-table-schema +-- squawk-ignore prefer-robust-stmts, require-timeout-settings create table test_table ( -- squawk-ignore prefer-timestamp-tz created_at timestamp default current_timestamp, @@ -493,9 +485,7 @@ alter table t2 drop column c2 cascade; [ RequireTimeoutSettings, RequireTimeoutSettings, - RequireTableSchema, PreferRobustStmts, - RequireTableSchema, PreferRobustStmts, ] "); @@ -521,9 +511,7 @@ alter table t2 drop column c2 cascade; [ RequireTimeoutSettings, RequireTimeoutSettings, - RequireTableSchema, PreferRobustStmts, - RequireTableSchema, PreferRobustStmts, ] "); @@ -556,10 +544,6 @@ alter table t2 drop column c2 cascade; RequireTimeoutSettings, "Missing `set statement_timeout` before potentially slow operations", ), - ( - RequireTableSchema, - "Table name is not schema-qualified. Use schema.table (e.g., public.my_table).", - ), ( BanDropColumn, "Dropping a column may break existing clients.", @@ -568,10 +552,6 @@ alter table t2 drop column c2 cascade; PreferRobustStmts, "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.", ), - ( - RequireTableSchema, - "Table name is not schema-qualified. Use schema.table (e.g., public.my_table).", - ), ( BanDropColumn, "Dropping a column may break existing clients.", @@ -600,13 +580,11 @@ alter table t2 drop column c2 cascade; .map(|x| x.code) .collect(); - assert_debug_snapshot!(errors, @r" + assert_debug_snapshot!(errors, @" [ RequireTimeoutSettings, RequireTimeoutSettings, - RequireTableSchema, PreferRobustStmts, - RequireTableSchema, PreferRobustStmts, ] "); @@ -627,11 +605,10 @@ alter table t drop column c cascade; .map(|x| x.code) .collect(); - assert_debug_snapshot!(errors, @r" + assert_debug_snapshot!(errors, @" [ RequireTimeoutSettings, RequireTimeoutSettings, - RequireTableSchema, ] "); } diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index c9bb196f..eac2c3ca 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -54,8 +54,8 @@ 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_timeout_settings; use rules::require_table_schema; +use rules::require_timeout_settings; use rules::transaction_nesting; // xtask:new-rule:rule-import @@ -97,6 +97,14 @@ pub enum Rule { // 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; @@ -459,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 626306e9..bd5069f2 100644 --- a/crates/squawk_linter/src/rules/mod.rs +++ b/crates/squawk_linter/src/rules/mod.rs @@ -27,8 +27,8 @@ 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_timeout_settings; pub(crate) mod require_table_schema; +pub(crate) mod require_timeout_settings; pub(crate) mod transaction_nesting; // xtask:new-rule:mod-decl @@ -61,7 +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_timeout_settings::require_timeout_settings; 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