diff --git a/crates/squawk_linter/src/ignore.rs b/crates/squawk_linter/src/ignore.rs index 13887def..85e23803 100644 --- a/crates/squawk_linter/src/ignore.rs +++ b/crates/squawk_linter/src/ignore.rs @@ -5,10 +5,17 @@ use squawk_syntax::{SyntaxKind, SyntaxNode, SyntaxToken}; use crate::{Linter, Rule, Violation}; +#[derive(Debug)] +pub enum IgnoreKind { + File, + Line, +} + #[derive(Debug)] pub struct Ignore { pub range: TextRange, pub violation_names: HashSet, + pub kind: IgnoreKind, } fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> { @@ -34,21 +41,27 @@ fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> { None } -const IGNORE_TEXT: &str = "squawk-ignore"; +const IGNORE_LINE_TEXT: &str = "squawk-ignore"; +const IGNORE_FILE_TEXT: &str = "squawk-ignore-file"; -fn ignore_rule_names(token: &SyntaxToken) -> Option<(&str, TextRange)> { +fn ignore_rule_names(token: &SyntaxToken) -> Option<(&str, TextRange, IgnoreKind)> { if let Some((comment_body, range)) = comment_body(token) { let without_start = comment_body.trim_start(); let trim_start_size = comment_body.len() - without_start.len(); let trimmed_comment = without_start.trim_end(); let trim_end_size = without_start.len() - trimmed_comment.len(); - if let Some(without_prefix) = trimmed_comment.strip_prefix(IGNORE_TEXT) { - let range = TextRange::new( - range.start() + TextSize::new((trim_start_size + IGNORE_TEXT.len()) as u32), - range.end() - TextSize::new(trim_end_size as u32), - ); - return Some((without_prefix, range)); + for (prefix, kind) in [ + (IGNORE_FILE_TEXT, IgnoreKind::File), + (IGNORE_LINE_TEXT, IgnoreKind::Line), + ] { + if let Some(without_prefix) = trimmed_comment.strip_prefix(prefix) { + let range = TextRange::new( + range.start() + TextSize::new((trim_start_size + prefix.len()) as u32), + range.end() - TextSize::new(trim_end_size as u32), + ); + return Some((without_prefix, range, kind)); + } } } None @@ -60,7 +73,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { rowan::WalkEvent::Enter(NodeOrToken::Token(token)) if token.kind() == SyntaxKind::COMMENT => { - if let Some((rule_names, range)) = ignore_rule_names(&token) { + if let Some((rule_names, range, kind)) = ignore_rule_names(&token) { let mut set = HashSet::new(); let mut offset = 0usize; @@ -97,6 +110,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { ctx.ignore(Ignore { range, violation_names: set, + kind, }); } } @@ -108,6 +122,9 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { #[cfg(test)] mod test { + use insta::assert_debug_snapshot; + + use super::IgnoreKind; use crate::{Linter, Rule, find_ignores}; #[test] @@ -229,4 +246,146 @@ create table test_table ( let errors = linter.lint(parse, sql); assert_eq!(errors.len(), 0); } + + #[test] + fn file_single_rule() { + let sql = r#" +-- squawk-ignore-file ban-drop-column +alter table t drop column c cascade; + "#; + let parse = squawk_syntax::SourceFile::parse(sql); + + let mut linter = Linter::from([]); + find_ignores(&mut linter, &parse.syntax_node()); + + assert_eq!(linter.ignores.len(), 1); + let ignore = &linter.ignores[0]; + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); + assert!(matches!(ignore.kind, IgnoreKind::File)); + } + + #[test] + fn file_ignore_with_all_rules() { + let sql = r#" +-- squawk-ignore-file +alter table t drop column c cascade; + "#; + let parse = squawk_syntax::SourceFile::parse(sql); + + let mut linter = Linter::from([]); + find_ignores(&mut linter, &parse.syntax_node()); + + assert_eq!(linter.ignores.len(), 1); + let ignore = &linter.ignores[0]; + assert!(matches!(ignore.kind, IgnoreKind::File)); + assert!(ignore.violation_names.is_empty()); + + let errors: Vec<_> = linter + .lint(parse, sql) + .into_iter() + .map(|x| x.code.clone()) + .collect(); + assert!(errors.is_empty()); + } + + #[test] + fn file_ignore_with_multiple_rules() { + let sql = r#" +-- squawk-ignore-file ban-drop-column, renaming-column +alter table t drop column c cascade; + "#; + let parse = squawk_syntax::SourceFile::parse(sql); + + let mut linter = Linter::from([]); + find_ignores(&mut linter, &parse.syntax_node()); + + assert_eq!(linter.ignores.len(), 1); + let ignore = &linter.ignores[0]; + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); + assert!(ignore.violation_names.contains(&Rule::RenamingColumn)); + assert!(matches!(ignore.kind, IgnoreKind::File)); + } + + #[test] + fn file_ignore_anywhere_works() { + let sql = r#" +alter table t add column x int; +-- squawk-ignore-file ban-drop-column +alter table t drop column c cascade; + "#; + let parse = squawk_syntax::SourceFile::parse(sql); + + let mut linter = Linter::from([]); + find_ignores(&mut linter, &parse.syntax_node()); + + assert_eq!(linter.ignores.len(), 1); + let ignore = &linter.ignores[0]; + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); + assert!(matches!(ignore.kind, IgnoreKind::File)); + } + + #[test] + fn file_ignore_c_style_comment() { + let sql = r#" +/* squawk-ignore-file ban-drop-column */ +alter table t drop column c cascade; + "#; + let parse = squawk_syntax::SourceFile::parse(sql); + + let mut linter = Linter::from([]); + find_ignores(&mut linter, &parse.syntax_node()); + + assert_eq!(linter.ignores.len(), 1); + let ignore = &linter.ignores[0]; + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); + assert!(matches!(ignore.kind, IgnoreKind::File)); + } + + #[test] + fn file_level_only_ignores_specific_rules() { + let mut linter = Linter::with_all_rules(); + let sql = r#" +-- squawk-ignore-file ban-drop-column +alter table t drop column c cascade; +alter table t2 drop column c2 cascade; + "#; + + let parse = squawk_syntax::SourceFile::parse(sql); + let errors: Vec<_> = linter + .lint(parse, sql) + .into_iter() + .map(|x| x.code.clone()) + .collect(); + + assert_debug_snapshot!(errors, @r" + [ + PreferRobustStmts, + PreferRobustStmts, + ] + "); + } + + #[test] + fn file_ignore_at_end_of_file_is_fine() { + let mut linter = Linter::with_all_rules(); + let sql = r#" +alter table t drop column c cascade; +alter table t2 drop column c2 cascade; +-- squawk-ignore-file ban-drop-column + "#; + + let parse = squawk_syntax::SourceFile::parse(sql); + let errors: Vec<_> = linter + .lint(parse, sql) + .into_iter() + .map(|x| x.code.clone()) + .collect(); + + assert_debug_snapshot!(errors, @r" + [ + PreferRobustStmts, + PreferRobustStmts, + ] + "); + } } diff --git a/crates/squawk_linter/src/ignore_index.rs b/crates/squawk_linter/src/ignore_index.rs index 309b934a..af22838a 100644 --- a/crates/squawk_linter/src/ignore_index.rs +++ b/crates/squawk_linter/src/ignore_index.rs @@ -6,20 +6,22 @@ use std::{ use line_index::LineIndex; use rowan::TextRange; -use crate::{Ignore, Rule}; +use crate::{Ignore, Rule, ignore::IgnoreKind}; pub(crate) struct IgnoreIndex { - line_to_ignored_names: HashMap>, + line_to_ignored: HashMap>, + file_ignored: HashSet, + ignore_all: bool, line_index: LineIndex, } impl fmt::Debug for IgnoreIndex { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { writeln!(f, "IgnoreIndex:")?; - let mut keys = self.line_to_ignored_names.keys().collect::>(); + let mut keys = self.line_to_ignored.keys().collect::>(); keys.sort(); for line in keys { - if let Some(set) = &self.line_to_ignored_names.get(line) { + if let Some(set) = &self.line_to_ignored.get(line) { writeln!(f, " {line}: {set:?}")?; } } @@ -30,23 +32,42 @@ impl fmt::Debug for IgnoreIndex { impl IgnoreIndex { pub(crate) fn new(text: &str, ignores: &[Ignore]) -> Self { let line_index = LineIndex::new(text); - let mut line_to_ignored_names: HashMap> = HashMap::new(); + let mut line_to_ignored: HashMap> = HashMap::new(); + let mut file_ignored: HashSet = HashSet::new(); + let mut ignore_all = false; for ignore in ignores { - let line = line_index.line_col(ignore.range.start()).line; - line_to_ignored_names.insert(line, ignore.violation_names.clone()); + match ignore.kind { + IgnoreKind::File => { + if ignore.violation_names.is_empty() { + // When a squawk-ignore-file comment has no rules, it means we should disable all the rules + ignore_all = true; + } else { + file_ignored.extend(ignore.violation_names.clone()); + } + } + IgnoreKind::Line => { + let line = line_index.line_col(ignore.range.start()).line; + line_to_ignored.insert(line, ignore.violation_names.clone()); + } + } } // TODO: we want to report unused ignores Self { - line_to_ignored_names, + line_to_ignored, + file_ignored, + ignore_all, line_index, } } pub(crate) fn contains(&self, range: TextRange, item: Rule) -> bool { + if self.ignore_all || self.file_ignored.contains(&item) { + return true; + } // 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 :/ let line = self.line_index.line_col(range.start()).line; for line in [line, if line == 0 { 0 } else { line - 1 }] { - if let Some(set) = self.line_to_ignored_names.get(&line) { + if let Some(set) = self.line_to_ignored.get(&line) { if set.contains(&item) { return true; } diff --git a/squawk-vscode/package.json b/squawk-vscode/package.json index 9da3c9a4..5f619784 100644 --- a/squawk-vscode/package.json +++ b/squawk-vscode/package.json @@ -88,7 +88,22 @@ "postgres" ] } - ] + ], + "configuration": { + "title": "Squawk", + "properties": { + "squawk.trace.server": { + "type": "string", + "enum": [ + "off", + "messages", + "verbose" + ], + "default": "off", + "description": "Trace the communication between VS Code and the Squawk language server" + } + } + } }, "scripts": { "vscode:prepublish": "pnpm run package",