Skip to main content

squawk_linter/
ignore.rs

1use std::collections::HashSet;
2
3use rowan::{NodeOrToken, TextRange, TextSize};
4use squawk_syntax::{SyntaxKind, SyntaxNode, SyntaxToken};
5
6use crate::{Linter, Rule, Violation};
7
8#[derive(Debug)]
9pub enum IgnoreKind {
10    File,
11    Line,
12}
13
14#[derive(Debug)]
15pub struct Ignore {
16    pub range: TextRange,
17    pub violation_names: HashSet<Rule>,
18    pub ignore_all: bool,
19    pub kind: IgnoreKind,
20}
21
22fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> {
23    let range = token.text_range();
24    if token.kind() == SyntaxKind::COMMENT {
25        let text = token.text();
26        if let Some(trimmed) = text.strip_prefix("--")
27            && let Some(start) = range.start().checked_add(2.into())
28        {
29            let end = range.end();
30            let updated_range = TextRange::new(start, end);
31            return Some((trimmed, updated_range));
32        }
33        if let Some(trimmed) = text.strip_prefix("/*").and_then(|x| x.strip_suffix("*/"))
34            && let Some(start) = range.start().checked_add(2.into())
35            && let Some(end) = range.end().checked_sub(2.into())
36        {
37            let updated_range = TextRange::new(start, end);
38            return Some((trimmed, updated_range));
39        }
40    }
41    None
42}
43
44// TODO: maybe in a future version we can rename this to squawk-ignore-line
45pub const IGNORE_LINE_TEXT: &str = "squawk-ignore";
46pub const IGNORE_FILE_TEXT: &str = "squawk-ignore-file";
47
48pub fn ignore_rule_info(token: &SyntaxToken) -> Option<(&str, TextRange, IgnoreKind)> {
49    if let Some((comment_body, range)) = comment_body(token) {
50        let without_start = comment_body.trim_start();
51        let trim_start_size = comment_body.len() - without_start.len();
52
53        let without_end = without_start
54            .find("--")
55            .map_or(without_start, |idx| &without_start[..idx])
56            .trim_end();
57        let trim_end_size = without_start.len() - without_end.len();
58
59        for (prefix, kind) in [
60            (IGNORE_FILE_TEXT, IgnoreKind::File),
61            (IGNORE_LINE_TEXT, IgnoreKind::Line),
62        ] {
63            if let Some(without_prefix) = without_end.strip_prefix(prefix) {
64                let start = range.start() + TextSize::new((trim_start_size + prefix.len()) as u32);
65                let end = range.end() - TextSize::new(trim_end_size as u32);
66
67                let range = TextRange::new(start, end);
68                return Some((without_prefix, range, kind));
69            }
70        }
71    }
72    None
73}
74
75pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
76    for event in file.preorder_with_tokens() {
77        match event {
78            rowan::WalkEvent::Enter(NodeOrToken::Token(token))
79                if token.kind() == SyntaxKind::COMMENT =>
80            {
81                if let Some((rule_names, range, kind)) = ignore_rule_info(&token) {
82                    let mut set = HashSet::new();
83                    let mut offset = 0usize;
84                    // We have a specific check instead of going off of empty
85                    // rules in case we have invalid rule names specified in the
86                    // ignore.
87                    let ignore_all = rule_names.trim().is_empty();
88
89                    // we need to keep track of our offset and report specific
90                    // ranges for any unknown names we encounter, which makes
91                    // this more complicated
92                    for x in rule_names.split(",") {
93                        if x.is_empty() {
94                            continue;
95                        }
96                        if let Ok(violation_name) = Rule::try_from(x.trim()) {
97                            set.insert(violation_name);
98                        } else {
99                            let without_start = x.trim_start();
100                            let trim_start_size = x.len() - without_start.len();
101                            let trimmed = without_start.trim_end();
102
103                            let range = range.checked_add(TextSize::new(offset as u32)).unwrap();
104
105                            let start = range.start() + TextSize::new(trim_start_size as u32);
106                            let end = start + TextSize::new(trimmed.len() as u32);
107                            let range = TextRange::new(start, end);
108
109                            ctx.report(Violation::for_range(
110                                Rule::UnusedIgnore,
111                                format!("unknown name {trimmed}"),
112                                range,
113                            ));
114                        }
115
116                        offset += x.len() + 1;
117                    }
118                    ctx.ignore(Ignore {
119                        range,
120                        violation_names: set,
121                        ignore_all,
122                        kind,
123                    });
124                }
125            }
126            _ => (),
127        }
128    }
129}
130
131#[cfg(test)]
132mod test {
133
134    use insta::assert_debug_snapshot;
135
136    use super::IgnoreKind;
137    use crate::{Linter, Rule, find_ignores};
138
139    #[test]
140    fn single_ignore() {
141        let sql = r#"
142-- squawk-ignore ban-drop-column
143alter table t drop column c cascade;
144        "#;
145        let parse = squawk_syntax::SourceFile::parse(sql);
146
147        let mut linter = Linter::from([]);
148        find_ignores(&mut linter, &parse.syntax_node());
149
150        assert_eq!(linter.ignores.len(), 1);
151        let ignore = &linter.ignores[0];
152        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
153    }
154
155    #[test]
156    fn multiple_sql_comments_with_ignore_is_ok() {
157        let sql = "
158-- fooo bar
159-- buzz
160-- squawk-ignore prefer-robust-stmts, require-timeout-settings
161create table x();
162
163select 1;
164";
165
166        let parse = squawk_syntax::SourceFile::parse(sql);
167        let mut linter = Linter::with_all_rules();
168        find_ignores(&mut linter, &parse.syntax_node());
169
170        assert_eq!(linter.ignores.len(), 1);
171        let ignore = &linter.ignores[0];
172        assert!(
173            ignore.violation_names.contains(&Rule::PreferRobustStmts),
174            "Make sure we picked up the ignore"
175        );
176
177        let errors = linter.lint(&parse, sql);
178
179        assert_eq!(
180            errors,
181            vec![],
182            "We shouldn't have any errors because we have the ignore setup"
183        );
184    }
185
186    #[test]
187    fn single_ignore_c_style_comment() {
188        let sql = r#"
189/* squawk-ignore ban-drop-column */
190alter table t drop column c cascade;
191        "#;
192        let parse = squawk_syntax::SourceFile::parse(sql);
193
194        let mut linter = Linter::from([]);
195
196        find_ignores(&mut linter, &parse.syntax_node());
197
198        assert_eq!(linter.ignores.len(), 1);
199        let ignore = &linter.ignores[0];
200        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
201    }
202
203    #[test]
204    fn multi_ignore() {
205        let sql = r#"
206-- squawk-ignore ban-drop-column, renaming-column,ban-drop-database
207alter table t drop column c cascade;
208        "#;
209        let parse = squawk_syntax::SourceFile::parse(sql);
210
211        let mut linter = Linter::from([]);
212
213        find_ignores(&mut linter, &parse.syntax_node());
214
215        assert_eq!(linter.ignores.len(), 1);
216        let ignore = &linter.ignores[0];
217        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
218        assert!(ignore.violation_names.contains(&Rule::RenamingColumn));
219        assert!(ignore.violation_names.contains(&Rule::BanDropDatabase));
220    }
221
222    #[test]
223    fn multi_ignore_c_style_comment() {
224        let sql = r#"
225/* squawk-ignore ban-drop-column, renaming-column,ban-drop-database */
226alter table t drop column c cascade;
227        "#;
228        let parse = squawk_syntax::SourceFile::parse(sql);
229
230        let mut linter = Linter::from([]);
231
232        find_ignores(&mut linter, &parse.syntax_node());
233
234        assert_eq!(linter.ignores.len(), 1);
235        let ignore = &linter.ignores[0];
236        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
237        assert!(ignore.violation_names.contains(&Rule::RenamingColumn));
238        assert!(ignore.violation_names.contains(&Rule::BanDropDatabase));
239    }
240
241    #[test]
242    fn ignore_multiple_stmts() {
243        let mut linter = Linter::with_all_rules();
244        let sql = r#"
245-- squawk-ignore ban-char-field,prefer-robust-stmts,require-timeout-settings
246alter table t add column c char;
247
248ALTER TABLE foo
249-- squawk-ignore adding-field-with-default,prefer-robust-stmts
250ADD COLUMN bar numeric GENERATED 
251  ALWAYS AS (bar + baz) STORED;
252
253-- squawk-ignore prefer-robust-stmts
254create table users (
255);
256"#;
257
258        let parse = squawk_syntax::SourceFile::parse(sql);
259        let errors = linter.lint(&parse, sql);
260        assert_eq!(errors.len(), 0);
261    }
262
263    #[test]
264    fn starting_line_aka_zero() {
265        let mut linter = Linter::with_all_rules();
266        let sql = r#"alter table t add column c char;"#;
267
268        let parse = squawk_syntax::SourceFile::parse(sql);
269        let errors = linter.lint(&parse, sql);
270        assert_debug_snapshot!(errors, @r#"
271        [
272            Violation {
273                code: RequireTimeoutSettings,
274                message: "Missing `set lock_timeout` before potentially slow operations",
275                text_range: 0..31,
276                help: Some(
277                    "Configure a `lock_timeout` before this statement.",
278                ),
279                fix: Some(
280                    Fix {
281                        title: "Add lock timeout",
282                        edits: [
283                            Edit {
284                                text_range: 0..0,
285                                text: Some(
286                                    "set lock_timeout = '1s';\n",
287                                ),
288                            },
289                        ],
290                    },
291                ),
292            },
293            Violation {
294                code: RequireTimeoutSettings,
295                message: "Missing `set statement_timeout` before potentially slow operations",
296                text_range: 0..31,
297                help: Some(
298                    "Configure a `statement_timeout` before this statement",
299                ),
300                fix: Some(
301                    Fix {
302                        title: "Add statement timeout",
303                        edits: [
304                            Edit {
305                                text_range: 0..0,
306                                text: Some(
307                                    "set statement_timeout = '5s';\n",
308                                ),
309                            },
310                        ],
311                    },
312                ),
313            },
314            Violation {
315                code: PreferRobustStmts,
316                message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.",
317                text_range: 14..31,
318                help: None,
319                fix: Some(
320                    Fix {
321                        title: "Insert `if not exists`",
322                        edits: [
323                            Edit {
324                                text_range: 24..24,
325                                text: Some(
326                                    " if not exists",
327                                ),
328                            },
329                        ],
330                    },
331                ),
332            },
333            Violation {
334                code: BanCharField,
335                message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
336                text_range: 27..31,
337                help: None,
338                fix: Some(
339                    Fix {
340                        title: "Replace with `text`",
341                        edits: [
342                            Edit {
343                                text_range: 27..31,
344                                text: Some(
345                                    "text",
346                                ),
347                            },
348                        ],
349                    },
350                ),
351            },
352        ]
353        "#);
354    }
355
356    #[test]
357    fn regression_unknown_name() {
358        let mut linter = Linter::with_all_rules();
359        let sql = r#"
360-- squawk-ignore prefer-robust-stmts, require-timeout-settings
361create table test_table (
362  -- squawk-ignore prefer-timestamp-tz
363  created_at timestamp default current_timestamp,
364  other_field text
365);
366        "#;
367
368        let parse = squawk_syntax::SourceFile::parse(sql);
369        let errors = linter.lint(&parse, sql);
370        assert_debug_snapshot!(errors, @"[]");
371        assert_eq!(errors.len(), 0);
372    }
373
374    #[test]
375    fn file_single_rule() {
376        let sql = r#"
377-- squawk-ignore-file ban-drop-column
378alter table t drop column c cascade;
379        "#;
380        let parse = squawk_syntax::SourceFile::parse(sql);
381
382        let mut linter = Linter::from([]);
383        find_ignores(&mut linter, &parse.syntax_node());
384
385        assert_eq!(linter.ignores.len(), 1);
386        let ignore = &linter.ignores[0];
387        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
388        assert!(matches!(ignore.kind, IgnoreKind::File));
389    }
390
391    #[test]
392    fn file_ignore_with_all_rules() {
393        let sql = r#"
394-- squawk-ignore-file
395alter table t drop column c cascade;
396        "#;
397        let parse = squawk_syntax::SourceFile::parse(sql);
398
399        let mut linter = Linter::from([]);
400        find_ignores(&mut linter, &parse.syntax_node());
401
402        assert_eq!(linter.ignores.len(), 1);
403        let ignore = &linter.ignores[0];
404        assert!(matches!(ignore.kind, IgnoreKind::File));
405        assert!(ignore.violation_names.is_empty());
406
407        let errors: Vec<_> = linter
408            .lint(&parse, sql)
409            .into_iter()
410            .map(|x| x.code)
411            .collect();
412        assert!(errors.is_empty());
413    }
414
415    #[test]
416    fn file_ignore_with_multiple_rules() {
417        let sql = r#"
418-- squawk-ignore-file ban-drop-column, renaming-column
419alter table t drop column c cascade;
420        "#;
421        let parse = squawk_syntax::SourceFile::parse(sql);
422
423        let mut linter = Linter::from([]);
424        find_ignores(&mut linter, &parse.syntax_node());
425
426        assert_eq!(linter.ignores.len(), 1);
427        let ignore = &linter.ignores[0];
428        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
429        assert!(ignore.violation_names.contains(&Rule::RenamingColumn));
430        assert!(matches!(ignore.kind, IgnoreKind::File));
431    }
432
433    #[test]
434    fn file_ignore_anywhere_works() {
435        let sql = r#"
436alter table t add column x int;
437-- squawk-ignore-file ban-drop-column
438alter table t drop column c cascade;
439        "#;
440        let parse = squawk_syntax::SourceFile::parse(sql);
441
442        let mut linter = Linter::from([]);
443        find_ignores(&mut linter, &parse.syntax_node());
444
445        assert_eq!(linter.ignores.len(), 1);
446        let ignore = &linter.ignores[0];
447        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
448        assert!(matches!(ignore.kind, IgnoreKind::File));
449    }
450
451    #[test]
452    fn file_ignore_c_style_comment() {
453        let sql = r#"
454/* squawk-ignore-file ban-drop-column */
455alter table t drop column c cascade;
456        "#;
457        let parse = squawk_syntax::SourceFile::parse(sql);
458
459        let mut linter = Linter::from([]);
460        find_ignores(&mut linter, &parse.syntax_node());
461
462        assert_eq!(linter.ignores.len(), 1);
463        let ignore = &linter.ignores[0];
464        assert!(ignore.violation_names.contains(&Rule::BanDropColumn));
465        assert!(matches!(ignore.kind, IgnoreKind::File));
466    }
467
468    #[test]
469    fn file_level_only_ignores_specific_rules() {
470        let mut linter = Linter::with_all_rules();
471        let sql = r#"
472-- squawk-ignore-file ban-drop-column
473alter table t drop column c cascade;
474alter table t2 drop column c2 cascade;
475        "#;
476
477        let parse = squawk_syntax::SourceFile::parse(sql);
478        let errors: Vec<_> = linter
479            .lint(&parse, sql)
480            .into_iter()
481            .map(|x| x.code)
482            .collect();
483
484        assert_debug_snapshot!(errors, @r"
485        [
486            RequireTimeoutSettings,
487            RequireTimeoutSettings,
488            PreferRobustStmts,
489            PreferRobustStmts,
490        ]
491        ");
492    }
493
494    #[test]
495    fn file_ignore_at_end_of_file_is_fine() {
496        let mut linter = Linter::with_all_rules();
497        let sql = r#"
498alter table t drop column c cascade;
499alter table t2 drop column c2 cascade;
500-- squawk-ignore-file ban-drop-column
501        "#;
502
503        let parse = squawk_syntax::SourceFile::parse(sql);
504        let errors: Vec<_> = linter
505            .lint(&parse, sql)
506            .into_iter()
507            .map(|x| x.code)
508            .collect();
509
510        assert_debug_snapshot!(errors, @r"
511        [
512            RequireTimeoutSettings,
513            RequireTimeoutSettings,
514            PreferRobustStmts,
515            PreferRobustStmts,
516        ]
517        ");
518    }
519
520    #[test]
521    fn file_ignore_with_invalid_rules() {
522        let mut linter = Linter::with_all_rules();
523        let sql = r#"
524-- squawk-ignore-file ban-ban-ban-drop-column ignore-something hmm
525alter table t drop column c cascade;
526alter table t2 drop column c2 cascade;
527        "#;
528
529        let parse = squawk_syntax::SourceFile::parse(sql);
530        let errors = linter.lint(&parse, sql);
531        let errors: Vec<_> = errors.iter().map(|x| (&x.code, &x.message)).collect();
532
533        assert_debug_snapshot!(errors, @r#"
534        [
535            (
536                UnusedIgnore,
537                "unknown name ban-ban-ban-drop-column ignore-something hmm",
538            ),
539            (
540                RequireTimeoutSettings,
541                "Missing `set lock_timeout` before potentially slow operations",
542            ),
543            (
544                RequireTimeoutSettings,
545                "Missing `set statement_timeout` before potentially slow operations",
546            ),
547            (
548                BanDropColumn,
549                "Dropping a column may break existing clients.",
550            ),
551            (
552                PreferRobustStmts,
553                "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.",
554            ),
555            (
556                BanDropColumn,
557                "Dropping a column may break existing clients.",
558            ),
559            (
560                PreferRobustStmts,
561                "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.",
562            ),
563        ]
564        "#);
565    }
566
567    #[test]
568    fn file_ignore_with_trailing_comment() {
569        let mut linter = Linter::with_all_rules();
570        let sql = r#"
571-- squawk-ignore-file ban-drop-column -- some comment here
572alter table t drop column c cascade;
573alter table t2 drop column c2 cascade;
574        "#;
575
576        let parse = squawk_syntax::SourceFile::parse(sql);
577        let errors: Vec<_> = linter
578            .lint(&parse, sql)
579            .into_iter()
580            .map(|x| x.code)
581            .collect();
582
583        assert_debug_snapshot!(errors, @"
584        [
585            RequireTimeoutSettings,
586            RequireTimeoutSettings,
587            PreferRobustStmts,
588            PreferRobustStmts,
589        ]
590        ");
591    }
592
593    #[test]
594    fn line_ignore_with_trailing_comment() {
595        let mut linter = Linter::with_all_rules();
596        let sql = r#"
597-- squawk-ignore ban-drop-column,prefer-robust-stmts -- drop is intentional
598alter table t drop column c cascade;
599        "#;
600
601        let parse = squawk_syntax::SourceFile::parse(sql);
602        let errors: Vec<_> = linter
603            .lint(&parse, sql)
604            .into_iter()
605            .map(|x| x.code)
606            .collect();
607
608        assert_debug_snapshot!(errors, @"
609        [
610            RequireTimeoutSettings,
611            RequireTimeoutSettings,
612        ]
613        ");
614    }
615}