Skip to content

Commit 5fb2b5e

Browse files
authored
linter: add file level ignores for rule violations (#585)
```sql -- squawk-ignore-file ``` will ignore all rules for the file ```sql -- squawk-ignore-file prefer-robust-stmts ``` will ignore a specific rule for the file and ```sql -- squawk-ignore prefer-robust-stmts ``` still ignores the rule on the following line
1 parent effbb99 commit 5fb2b5e

File tree

3 files changed

+214
-19
lines changed

3 files changed

+214
-19
lines changed

crates/squawk_linter/src/ignore.rs

Lines changed: 168 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,17 @@ use squawk_syntax::{SyntaxKind, SyntaxNode, SyntaxToken};
55

66
use crate::{Linter, Rule, Violation};
77

8+
#[derive(Debug)]
9+
pub enum IgnoreKind {
10+
File,
11+
Line,
12+
}
13+
814
#[derive(Debug)]
915
pub struct Ignore {
1016
pub range: TextRange,
1117
pub violation_names: HashSet<Rule>,
18+
pub kind: IgnoreKind,
1219
}
1320

1421
fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> {
@@ -34,21 +41,27 @@ fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> {
3441
None
3542
}
3643

37-
const IGNORE_TEXT: &str = "squawk-ignore";
44+
const IGNORE_LINE_TEXT: &str = "squawk-ignore";
45+
const IGNORE_FILE_TEXT: &str = "squawk-ignore-file";
3846

39-
fn ignore_rule_names(token: &SyntaxToken) -> Option<(&str, TextRange)> {
47+
fn ignore_rule_names(token: &SyntaxToken) -> Option<(&str, TextRange, IgnoreKind)> {
4048
if let Some((comment_body, range)) = comment_body(token) {
4149
let without_start = comment_body.trim_start();
4250
let trim_start_size = comment_body.len() - without_start.len();
4351
let trimmed_comment = without_start.trim_end();
4452
let trim_end_size = without_start.len() - trimmed_comment.len();
4553

46-
if let Some(without_prefix) = trimmed_comment.strip_prefix(IGNORE_TEXT) {
47-
let range = TextRange::new(
48-
range.start() + TextSize::new((trim_start_size + IGNORE_TEXT.len()) as u32),
49-
range.end() - TextSize::new(trim_end_size as u32),
50-
);
51-
return Some((without_prefix, range));
54+
for (prefix, kind) in [
55+
(IGNORE_FILE_TEXT, IgnoreKind::File),
56+
(IGNORE_LINE_TEXT, IgnoreKind::Line),
57+
] {
58+
if let Some(without_prefix) = trimmed_comment.strip_prefix(prefix) {
59+
let range = TextRange::new(
60+
range.start() + TextSize::new((trim_start_size + prefix.len()) as u32),
61+
range.end() - TextSize::new(trim_end_size as u32),
62+
);
63+
return Some((without_prefix, range, kind));
64+
}
5265
}
5366
}
5467
None
@@ -60,7 +73,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
6073
rowan::WalkEvent::Enter(NodeOrToken::Token(token))
6174
if token.kind() == SyntaxKind::COMMENT =>
6275
{
63-
if let Some((rule_names, range)) = ignore_rule_names(&token) {
76+
if let Some((rule_names, range, kind)) = ignore_rule_names(&token) {
6477
let mut set = HashSet::new();
6578
let mut offset = 0usize;
6679

@@ -97,6 +110,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
97110
ctx.ignore(Ignore {
98111
range,
99112
violation_names: set,
113+
kind,
100114
});
101115
}
102116
}
@@ -108,6 +122,9 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
108122
#[cfg(test)]
109123
mod test {
110124

125+
use insta::assert_debug_snapshot;
126+
127+
use super::IgnoreKind;
111128
use crate::{Linter, Rule, find_ignores};
112129

113130
#[test]
@@ -229,4 +246,146 @@ create table test_table (
229246
let errors = linter.lint(parse, sql);
230247
assert_eq!(errors.len(), 0);
231248
}
249+
250+
#[test]
251+
fn file_single_rule() {
252+
let sql = r#"
253+
-- squawk-ignore-file ban-drop-column
254+
alter table t drop column c cascade;
255+
"#;
256+
let parse = squawk_syntax::SourceFile::parse(sql);
257+
258+
let mut linter = Linter::from([]);
259+
find_ignores(&mut linter, &parse.syntax_node());
260+
261+
assert_eq!(linter.ignores.len(), 1);
262+
let ignore = &linter.ignores[0];
263+
assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
264+
assert!(matches!(ignore.kind, IgnoreKind::File));
265+
}
266+
267+
#[test]
268+
fn file_ignore_with_all_rules() {
269+
let sql = r#"
270+
-- squawk-ignore-file
271+
alter table t drop column c cascade;
272+
"#;
273+
let parse = squawk_syntax::SourceFile::parse(sql);
274+
275+
let mut linter = Linter::from([]);
276+
find_ignores(&mut linter, &parse.syntax_node());
277+
278+
assert_eq!(linter.ignores.len(), 1);
279+
let ignore = &linter.ignores[0];
280+
assert!(matches!(ignore.kind, IgnoreKind::File));
281+
assert!(ignore.violation_names.is_empty());
282+
283+
let errors: Vec<_> = linter
284+
.lint(parse, sql)
285+
.into_iter()
286+
.map(|x| x.code.clone())
287+
.collect();
288+
assert!(errors.is_empty());
289+
}
290+
291+
#[test]
292+
fn file_ignore_with_multiple_rules() {
293+
let sql = r#"
294+
-- squawk-ignore-file ban-drop-column, renaming-column
295+
alter table t drop column c cascade;
296+
"#;
297+
let parse = squawk_syntax::SourceFile::parse(sql);
298+
299+
let mut linter = Linter::from([]);
300+
find_ignores(&mut linter, &parse.syntax_node());
301+
302+
assert_eq!(linter.ignores.len(), 1);
303+
let ignore = &linter.ignores[0];
304+
assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
305+
assert!(ignore.violation_names.contains(&Rule::RenamingColumn));
306+
assert!(matches!(ignore.kind, IgnoreKind::File));
307+
}
308+
309+
#[test]
310+
fn file_ignore_anywhere_works() {
311+
let sql = r#"
312+
alter table t add column x int;
313+
-- squawk-ignore-file ban-drop-column
314+
alter table t drop column c cascade;
315+
"#;
316+
let parse = squawk_syntax::SourceFile::parse(sql);
317+
318+
let mut linter = Linter::from([]);
319+
find_ignores(&mut linter, &parse.syntax_node());
320+
321+
assert_eq!(linter.ignores.len(), 1);
322+
let ignore = &linter.ignores[0];
323+
assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
324+
assert!(matches!(ignore.kind, IgnoreKind::File));
325+
}
326+
327+
#[test]
328+
fn file_ignore_c_style_comment() {
329+
let sql = r#"
330+
/* squawk-ignore-file ban-drop-column */
331+
alter table t drop column c cascade;
332+
"#;
333+
let parse = squawk_syntax::SourceFile::parse(sql);
334+
335+
let mut linter = Linter::from([]);
336+
find_ignores(&mut linter, &parse.syntax_node());
337+
338+
assert_eq!(linter.ignores.len(), 1);
339+
let ignore = &linter.ignores[0];
340+
assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
341+
assert!(matches!(ignore.kind, IgnoreKind::File));
342+
}
343+
344+
#[test]
345+
fn file_level_only_ignores_specific_rules() {
346+
let mut linter = Linter::with_all_rules();
347+
let sql = r#"
348+
-- squawk-ignore-file ban-drop-column
349+
alter table t drop column c cascade;
350+
alter table t2 drop column c2 cascade;
351+
"#;
352+
353+
let parse = squawk_syntax::SourceFile::parse(sql);
354+
let errors: Vec<_> = linter
355+
.lint(parse, sql)
356+
.into_iter()
357+
.map(|x| x.code.clone())
358+
.collect();
359+
360+
assert_debug_snapshot!(errors, @r"
361+
[
362+
PreferRobustStmts,
363+
PreferRobustStmts,
364+
]
365+
");
366+
}
367+
368+
#[test]
369+
fn file_ignore_at_end_of_file_is_fine() {
370+
let mut linter = Linter::with_all_rules();
371+
let sql = r#"
372+
alter table t drop column c cascade;
373+
alter table t2 drop column c2 cascade;
374+
-- squawk-ignore-file ban-drop-column
375+
"#;
376+
377+
let parse = squawk_syntax::SourceFile::parse(sql);
378+
let errors: Vec<_> = linter
379+
.lint(parse, sql)
380+
.into_iter()
381+
.map(|x| x.code.clone())
382+
.collect();
383+
384+
assert_debug_snapshot!(errors, @r"
385+
[
386+
PreferRobustStmts,
387+
PreferRobustStmts,
388+
]
389+
");
390+
}
232391
}

crates/squawk_linter/src/ignore_index.rs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,22 @@ use std::{
66
use line_index::LineIndex;
77
use rowan::TextRange;
88

9-
use crate::{Ignore, Rule};
9+
use crate::{Ignore, Rule, ignore::IgnoreKind};
1010

1111
pub(crate) struct IgnoreIndex {
12-
line_to_ignored_names: HashMap<u32, HashSet<Rule>>,
12+
line_to_ignored: HashMap<u32, HashSet<Rule>>,
13+
file_ignored: HashSet<Rule>,
14+
ignore_all: bool,
1315
line_index: LineIndex,
1416
}
1517

1618
impl fmt::Debug for IgnoreIndex {
1719
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1820
writeln!(f, "IgnoreIndex:")?;
19-
let mut keys = self.line_to_ignored_names.keys().collect::<Vec<_>>();
21+
let mut keys = self.line_to_ignored.keys().collect::<Vec<_>>();
2022
keys.sort();
2123
for line in keys {
22-
if let Some(set) = &self.line_to_ignored_names.get(line) {
24+
if let Some(set) = &self.line_to_ignored.get(line) {
2325
writeln!(f, " {line}: {set:?}")?;
2426
}
2527
}
@@ -30,23 +32,42 @@ impl fmt::Debug for IgnoreIndex {
3032
impl IgnoreIndex {
3133
pub(crate) fn new(text: &str, ignores: &[Ignore]) -> Self {
3234
let line_index = LineIndex::new(text);
33-
let mut line_to_ignored_names: HashMap<u32, HashSet<Rule>> = HashMap::new();
35+
let mut line_to_ignored: HashMap<u32, HashSet<Rule>> = HashMap::new();
36+
let mut file_ignored: HashSet<Rule> = HashSet::new();
37+
let mut ignore_all = false;
3438
for ignore in ignores {
35-
let line = line_index.line_col(ignore.range.start()).line;
36-
line_to_ignored_names.insert(line, ignore.violation_names.clone());
39+
match ignore.kind {
40+
IgnoreKind::File => {
41+
if ignore.violation_names.is_empty() {
42+
// When a squawk-ignore-file comment has no rules, it means we should disable all the rules
43+
ignore_all = true;
44+
} else {
45+
file_ignored.extend(ignore.violation_names.clone());
46+
}
47+
}
48+
IgnoreKind::Line => {
49+
let line = line_index.line_col(ignore.range.start()).line;
50+
line_to_ignored.insert(line, ignore.violation_names.clone());
51+
}
52+
}
3753
}
3854
// TODO: we want to report unused ignores
3955
Self {
40-
line_to_ignored_names,
56+
line_to_ignored,
57+
file_ignored,
58+
ignore_all,
4159
line_index,
4260
}
4361
}
4462

4563
pub(crate) fn contains(&self, range: TextRange, item: Rule) -> bool {
64+
if self.ignore_all || self.file_ignored.contains(&item) {
65+
return true;
66+
}
4667
// TODO: hmmm basically we want to ensure that either it's on the line before or it's inside the start of the node. we parse stuff so that the comment ends up inside the node :/
4768
let line = self.line_index.line_col(range.start()).line;
4869
for line in [line, if line == 0 { 0 } else { line - 1 }] {
49-
if let Some(set) = self.line_to_ignored_names.get(&line) {
70+
if let Some(set) = self.line_to_ignored.get(&line) {
5071
if set.contains(&item) {
5172
return true;
5273
}

squawk-vscode/package.json

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,22 @@
8888
"postgres"
8989
]
9090
}
91-
]
91+
],
92+
"configuration": {
93+
"title": "Squawk",
94+
"properties": {
95+
"squawk.trace.server": {
96+
"type": "string",
97+
"enum": [
98+
"off",
99+
"messages",
100+
"verbose"
101+
],
102+
"default": "off",
103+
"description": "Trace the communication between VS Code and the Squawk language server"
104+
}
105+
}
106+
}
92107
},
93108
"scripts": {
94109
"vscode:prepublish": "pnpm run package",

0 commit comments

Comments
 (0)