From 953dc80aac4cd45b3b2767e4abceada00d25dc95 Mon Sep 17 00:00:00 2001 From: Peter Stenger Date: Thu, 12 Mar 2026 11:20:09 -0400 Subject: [PATCH 1/2] linter: add file-level override for `assume_in_transaction` via comments Add `-- squawk-disable-assume-in-transaction` and `-- squawk-enable-assume-in-transaction` comments to override the global `assume_in_transaction` setting on a per-file basis. This is useful when a migration tool wraps files in transactions by default but specific files need to opt out (e.g., for `CREATE INDEX CONCURRENTLY`). Closes #990 Co-Authored-By: Claude Opus 4.6 --- crates/squawk_linter/src/ignore.rs | 70 ++++++++++++++++++- crates/squawk_linter/src/lib.rs | 5 ++ ...oncurrent_index_creation_in_transaction.rs | 16 +++++ ...ble_assume_in_transaction_flags_begin.snap | 17 +++++ .../src/rules/transaction_nesting.rs | 32 +++++++++ docs/docs/cli.md | 16 +++++ 6 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap diff --git a/crates/squawk_linter/src/ignore.rs b/crates/squawk_linter/src/ignore.rs index 01ce5c2bf..63ff85199 100644 --- a/crates/squawk_linter/src/ignore.rs +++ b/crates/squawk_linter/src/ignore.rs @@ -19,7 +19,7 @@ pub struct Ignore { pub kind: IgnoreKind, } -fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> { +pub(crate) fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> { let range = token.text_range(); if token.kind() == SyntaxKind::COMMENT { let text = token.text(); @@ -128,6 +128,34 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { } } +const DISABLE_ASSUME_IN_TRANSACTION: &str = "squawk-disable-assume-in-transaction"; +const ENABLE_ASSUME_IN_TRANSACTION: &str = "squawk-enable-assume-in-transaction"; + +pub fn find_transaction_override(file: &SyntaxNode) -> Option { + for event in file.preorder_with_tokens() { + match event { + rowan::WalkEvent::Enter(NodeOrToken::Token(token)) + if token.kind() == SyntaxKind::COMMENT => + { + if let Some((body, _range)) = comment_body(&token) { + let trimmed = body.trim(); + let trimmed = trimmed + .find("--") + .map_or(trimmed, |idx| trimmed[..idx].trim_end()); + if trimmed == DISABLE_ASSUME_IN_TRANSACTION { + return Some(false); + } + if trimmed == ENABLE_ASSUME_IN_TRANSACTION { + return Some(true); + } + } + } + _ => (), + } + } + None +} + #[cfg(test)] mod test { @@ -612,4 +640,44 @@ alter table t drop column c cascade; ] "); } + + #[test] + fn disable_assume_in_transaction() { + use super::find_transaction_override; + let sql = "-- squawk-disable-assume-in-transaction\nSELECT 1;"; + let parse = squawk_syntax::SourceFile::parse(sql); + assert_eq!(find_transaction_override(&parse.syntax_node()), Some(false)); + } + + #[test] + fn enable_assume_in_transaction() { + use super::find_transaction_override; + let sql = "-- squawk-enable-assume-in-transaction\nSELECT 1;"; + let parse = squawk_syntax::SourceFile::parse(sql); + assert_eq!(find_transaction_override(&parse.syntax_node()), Some(true)); + } + + #[test] + fn disable_assume_in_transaction_c_style_comment() { + use super::find_transaction_override; + let sql = "/* squawk-disable-assume-in-transaction */\nSELECT 1;"; + let parse = squawk_syntax::SourceFile::parse(sql); + assert_eq!(find_transaction_override(&parse.syntax_node()), Some(false)); + } + + #[test] + fn disable_assume_in_transaction_with_trailing_comment() { + use super::find_transaction_override; + let sql = "-- squawk-disable-assume-in-transaction -- not in a transaction\nSELECT 1;"; + let parse = squawk_syntax::SourceFile::parse(sql); + assert_eq!(find_transaction_override(&parse.syntax_node()), Some(false)); + } + + #[test] + fn transaction_override_none_when_absent() { + use super::find_transaction_override; + let sql = "SELECT 1;"; + let parse = squawk_syntax::SourceFile::parse(sql); + assert_eq!(find_transaction_override(&parse.syntax_node()), None); + } } diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index e72654b6c..ca7b45247 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -5,6 +5,7 @@ use enum_iterator::Sequence; use enum_iterator::all; pub use ignore::Ignore; use ignore::find_ignores; +use ignore::find_transaction_override; use ignore_index::IgnoreIndex; use rowan::TextRange; use rowan::TextSize; @@ -332,6 +333,10 @@ impl Linter { #[must_use] pub fn lint(&mut self, file: &Parse, text: &str) -> Vec { + if let Some(override_value) = find_transaction_override(&file.syntax_node()) { + self.settings.assume_in_transaction = override_value; + } + if self.rules.contains(&Rule::AddingFieldWithDefault) { adding_field_with_default(self, file); } diff --git a/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs b/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs index fe5566532..164e2bda0 100644 --- a/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs +++ b/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs @@ -134,4 +134,20 @@ mod test { }, ); } + + #[test] + fn squawk_disable_assume_in_transaction_overrides() { + let sql = r#" +-- squawk-disable-assume-in-transaction +CREATE INDEX CONCURRENTLY "field_name_idx" ON "table_name" ("field_name"); +ALTER TABLE "table_name" ADD CONSTRAINT "field_name_id" UNIQUE USING INDEX "field_name_idx"; + "#; + lint_ok_with( + sql, + LinterSettings { + assume_in_transaction: true, + ..Default::default() + }, + ); + } } diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap new file mode 100644 index 000000000..f95ebe4c2 --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap @@ -0,0 +1,17 @@ +--- +source: crates/squawk_linter/src/rules/transaction_nesting.rs +assertion_line: 195 +expression: "lint_errors_with(sql, LinterSettings\n{ assume_in_transaction: false, ..Default::default() },)" +--- +warning[transaction-nesting]: There is an existing transaction already in progress, managed by your migration tool. + ╭▸ +3 │ BEGIN; + │ ━━━━━ + │ + ╰ help: Put migration statements in separate files to have them be in separate transactions or don't use the assume-in-transaction setting. +warning[transaction-nesting]: Attempting to end the transaction that is managed by your migration tool + ╭▸ +5 │ COMMIT; + │ ━━━━━━ + │ + ╰ help: Put migration statements in separate files to have them be in separate transactions or don't use the assume-in-transaction setting. diff --git a/crates/squawk_linter/src/rules/transaction_nesting.rs b/crates/squawk_linter/src/rules/transaction_nesting.rs index f63239f92..39100ebfb 100644 --- a/crates/squawk_linter/src/rules/transaction_nesting.rs +++ b/crates/squawk_linter/src/rules/transaction_nesting.rs @@ -168,4 +168,36 @@ SELECT 1; }; lint_ok_with(sql, settings); } + + #[test] + fn squawk_disable_assume_in_transaction_allows_begin_commit() { + let sql = r#" +-- squawk-disable-assume-in-transaction +BEGIN; +SELECT 1; +COMMIT; + "#; + let settings = LinterSettings { + assume_in_transaction: true, + ..Default::default() + }; + lint_ok_with(sql, settings); + } + + #[test] + fn squawk_enable_assume_in_transaction_flags_begin() { + let sql = r#" +-- squawk-enable-assume-in-transaction +BEGIN; +SELECT 1; +COMMIT; + "#; + assert_snapshot!(lint_errors_with( + sql, + LinterSettings { + assume_in_transaction: false, + ..Default::default() + }, + )); + } } diff --git a/docs/docs/cli.md b/docs/docs/cli.md index 208e1af4e..58e18b8e9 100644 --- a/docs/docs/cli.md +++ b/docs/docs/cli.md @@ -54,6 +54,22 @@ alter table t drop column c cascade; create table t (a int); ``` +### Overriding `assume_in_transaction` via comments + +If you have `assume_in_transaction = true` set globally (via config or CLI flag), you can disable it for a specific file with `squawk-disable-assume-in-transaction`: + +```sql +-- squawk-disable-assume-in-transaction +CREATE INDEX CONCURRENTLY IF NOT EXISTS my_idx ON my_table (col); +``` + +Similarly, you can enable it for a specific file with `squawk-enable-assume-in-transaction`: + +```sql +-- squawk-enable-assume-in-transaction +ALTER TABLE my_table ADD COLUMN my_col integer; +``` + ## Files Files can be excluded from linting via the `--exclude-path` flag. Glob matching is supported and the flag can be provided multiple times. From de43b534df906f6427df28b7649836cf840029ae Mon Sep 17 00:00:00 2001 From: Peter Stenger Date: Fri, 1 May 2026 14:42:28 -0500 Subject: [PATCH 2/2] linter: remove `squawk-enable-assume-in-transaction` override Only the `squawk-disable-assume-in-transaction` file-level override remains. Renames `find_transaction_override` to `has_disable_assume_in_transaction` since it now only signals one direction. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/squawk_linter/src/ignore.rs | 34 ++++++------------- crates/squawk_linter/src/lib.rs | 6 ++-- ...ble_assume_in_transaction_flags_begin.snap | 17 ---------- .../src/rules/transaction_nesting.rs | 17 ---------- docs/docs/cli.md | 7 ---- 5 files changed, 14 insertions(+), 67 deletions(-) delete mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap diff --git a/crates/squawk_linter/src/ignore.rs b/crates/squawk_linter/src/ignore.rs index 63ff85199..a40607b88 100644 --- a/crates/squawk_linter/src/ignore.rs +++ b/crates/squawk_linter/src/ignore.rs @@ -129,9 +129,8 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { } const DISABLE_ASSUME_IN_TRANSACTION: &str = "squawk-disable-assume-in-transaction"; -const ENABLE_ASSUME_IN_TRANSACTION: &str = "squawk-enable-assume-in-transaction"; -pub fn find_transaction_override(file: &SyntaxNode) -> Option { +pub fn has_disable_assume_in_transaction(file: &SyntaxNode) -> bool { for event in file.preorder_with_tokens() { match event { rowan::WalkEvent::Enter(NodeOrToken::Token(token)) @@ -143,17 +142,14 @@ pub fn find_transaction_override(file: &SyntaxNode) -> Option { .find("--") .map_or(trimmed, |idx| trimmed[..idx].trim_end()); if trimmed == DISABLE_ASSUME_IN_TRANSACTION { - return Some(false); - } - if trimmed == ENABLE_ASSUME_IN_TRANSACTION { - return Some(true); + return true; } } } _ => (), } } - None + false } #[cfg(test)] @@ -643,41 +639,33 @@ alter table t drop column c cascade; #[test] fn disable_assume_in_transaction() { - use super::find_transaction_override; + use super::has_disable_assume_in_transaction; let sql = "-- squawk-disable-assume-in-transaction\nSELECT 1;"; let parse = squawk_syntax::SourceFile::parse(sql); - assert_eq!(find_transaction_override(&parse.syntax_node()), Some(false)); - } - - #[test] - fn enable_assume_in_transaction() { - use super::find_transaction_override; - let sql = "-- squawk-enable-assume-in-transaction\nSELECT 1;"; - let parse = squawk_syntax::SourceFile::parse(sql); - assert_eq!(find_transaction_override(&parse.syntax_node()), Some(true)); + assert!(has_disable_assume_in_transaction(&parse.syntax_node())); } #[test] fn disable_assume_in_transaction_c_style_comment() { - use super::find_transaction_override; + use super::has_disable_assume_in_transaction; let sql = "/* squawk-disable-assume-in-transaction */\nSELECT 1;"; let parse = squawk_syntax::SourceFile::parse(sql); - assert_eq!(find_transaction_override(&parse.syntax_node()), Some(false)); + assert!(has_disable_assume_in_transaction(&parse.syntax_node())); } #[test] fn disable_assume_in_transaction_with_trailing_comment() { - use super::find_transaction_override; + use super::has_disable_assume_in_transaction; let sql = "-- squawk-disable-assume-in-transaction -- not in a transaction\nSELECT 1;"; let parse = squawk_syntax::SourceFile::parse(sql); - assert_eq!(find_transaction_override(&parse.syntax_node()), Some(false)); + assert!(has_disable_assume_in_transaction(&parse.syntax_node())); } #[test] fn transaction_override_none_when_absent() { - use super::find_transaction_override; + use super::has_disable_assume_in_transaction; let sql = "SELECT 1;"; let parse = squawk_syntax::SourceFile::parse(sql); - assert_eq!(find_transaction_override(&parse.syntax_node()), None); + assert!(!has_disable_assume_in_transaction(&parse.syntax_node())); } } diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index ca7b45247..2f9262fbd 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -5,7 +5,7 @@ use enum_iterator::Sequence; use enum_iterator::all; pub use ignore::Ignore; use ignore::find_ignores; -use ignore::find_transaction_override; +use ignore::has_disable_assume_in_transaction; use ignore_index::IgnoreIndex; use rowan::TextRange; use rowan::TextSize; @@ -333,8 +333,8 @@ impl Linter { #[must_use] pub fn lint(&mut self, file: &Parse, text: &str) -> Vec { - if let Some(override_value) = find_transaction_override(&file.syntax_node()) { - self.settings.assume_in_transaction = override_value; + if has_disable_assume_in_transaction(&file.syntax_node()) { + self.settings.assume_in_transaction = false; } if self.rules.contains(&Rule::AddingFieldWithDefault) { diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap deleted file mode 100644 index f95ebe4c2..000000000 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__transaction_nesting__test__squawk_enable_assume_in_transaction_flags_begin.snap +++ /dev/null @@ -1,17 +0,0 @@ ---- -source: crates/squawk_linter/src/rules/transaction_nesting.rs -assertion_line: 195 -expression: "lint_errors_with(sql, LinterSettings\n{ assume_in_transaction: false, ..Default::default() },)" ---- -warning[transaction-nesting]: There is an existing transaction already in progress, managed by your migration tool. - ╭▸ -3 │ BEGIN; - │ ━━━━━ - │ - ╰ help: Put migration statements in separate files to have them be in separate transactions or don't use the assume-in-transaction setting. -warning[transaction-nesting]: Attempting to end the transaction that is managed by your migration tool - ╭▸ -5 │ COMMIT; - │ ━━━━━━ - │ - ╰ help: Put migration statements in separate files to have them be in separate transactions or don't use the assume-in-transaction setting. diff --git a/crates/squawk_linter/src/rules/transaction_nesting.rs b/crates/squawk_linter/src/rules/transaction_nesting.rs index 39100ebfb..52240f048 100644 --- a/crates/squawk_linter/src/rules/transaction_nesting.rs +++ b/crates/squawk_linter/src/rules/transaction_nesting.rs @@ -183,21 +183,4 @@ COMMIT; }; lint_ok_with(sql, settings); } - - #[test] - fn squawk_enable_assume_in_transaction_flags_begin() { - let sql = r#" --- squawk-enable-assume-in-transaction -BEGIN; -SELECT 1; -COMMIT; - "#; - assert_snapshot!(lint_errors_with( - sql, - LinterSettings { - assume_in_transaction: false, - ..Default::default() - }, - )); - } } diff --git a/docs/docs/cli.md b/docs/docs/cli.md index 58e18b8e9..84ebf4181 100644 --- a/docs/docs/cli.md +++ b/docs/docs/cli.md @@ -63,13 +63,6 @@ If you have `assume_in_transaction = true` set globally (via config or CLI flag) CREATE INDEX CONCURRENTLY IF NOT EXISTS my_idx ON my_table (col); ``` -Similarly, you can enable it for a specific file with `squawk-enable-assume-in-transaction`: - -```sql --- squawk-enable-assume-in-transaction -ALTER TABLE my_table ADD COLUMN my_col integer; -``` - ## Files Files can be excluded from linting via the `--exclude-path` flag. Glob matching is supported and the flag can be provided multiple times.