Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion crates/squawk_linter/src/ignore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -128,6 +128,30 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
}
}

const DISABLE_ASSUME_IN_TRANSACTION: &str = "squawk-disable-assume-in-transaction";

pub fn has_disable_assume_in_transaction(file: &SyntaxNode) -> bool {
for event in file.preorder_with_tokens() {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also seems like we'd only want to check the first 20 lines rather than have to look at every comment in the file (including ones that are nested inside other nodes)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore is a little different since lint errors can occur ~anywhere

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 true;
}
}
}
_ => (),
}
}
false
}

#[cfg(test)]
mod test {

Expand Down Expand Up @@ -612,4 +636,36 @@ alter table t drop column c cascade;
]
");
}

#[test]
fn disable_assume_in_transaction() {
use super::has_disable_assume_in_transaction;
let sql = "-- squawk-disable-assume-in-transaction\nSELECT 1;";
let parse = squawk_syntax::SourceFile::parse(sql);
assert!(has_disable_assume_in_transaction(&parse.syntax_node()));
}

#[test]
fn disable_assume_in_transaction_c_style_comment() {
use super::has_disable_assume_in_transaction;
let sql = "/* squawk-disable-assume-in-transaction */\nSELECT 1;";
let parse = squawk_syntax::SourceFile::parse(sql);
assert!(has_disable_assume_in_transaction(&parse.syntax_node()));
}

#[test]
fn disable_assume_in_transaction_with_trailing_comment() {
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!(has_disable_assume_in_transaction(&parse.syntax_node()));
}

#[test]
fn transaction_override_none_when_absent() {
use super::has_disable_assume_in_transaction;
let sql = "SELECT 1;";
let parse = squawk_syntax::SourceFile::parse(sql);
assert!(!has_disable_assume_in_transaction(&parse.syntax_node()));
}
}
5 changes: 5 additions & 0 deletions crates/squawk_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use enum_iterator::Sequence;
use enum_iterator::all;
pub use ignore::Ignore;
use ignore::find_ignores;
use ignore::has_disable_assume_in_transaction;
use ignore_index::IgnoreIndex;
use rowan::TextRange;
use rowan::TextSize;
Expand Down Expand Up @@ -360,6 +361,10 @@ impl Linter {

#[must_use]
pub fn lint(&mut self, file: &Parse<SourceFile>, text: &str) -> Vec<Violation> {
if has_disable_assume_in_transaction(&file.syntax_node()) {
self.settings.assume_in_transaction = false;
}

if self.rules.contains(&Rule::AddingFieldWithDefault) {
adding_field_with_default(self, file);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
},
);
}
}
15 changes: 15 additions & 0 deletions crates/squawk_linter/src/rules/transaction_nesting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,19 @@ 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
Comment thread
reteps marked this conversation as resolved.
BEGIN;
SELECT 1;
COMMIT;
"#;
let settings = LinterSettings {
assume_in_transaction: true,
..Default::default()
};
lint_ok_with(sql, settings);
}
}
9 changes: 9 additions & 0 deletions docs/docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ 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);
```

## Files

Files can be excluded from linting via the `--exclude-path` flag. Glob matching is supported and the flag can be provided multiple times.
Expand Down
Loading