squawk_linter/
ignore.rs

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

squawk_linter/
ignore.rs
