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

squawk_linter/
ignore.rs
