Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 168 additions & 9 deletions crates/squawk_linter/src/ignore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Rule>,
pub kind: IgnoreKind,
}

fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> {
Expand All @@ -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
Expand All @@ -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;

Expand Down Expand Up @@ -97,6 +110,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
ctx.ignore(Ignore {
range,
violation_names: set,
kind,
});
}
}
Expand All @@ -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]
Expand Down Expand Up @@ -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,
]
");
}
}
39 changes: 30 additions & 9 deletions crates/squawk_linter/src/ignore_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u32, HashSet<Rule>>,
line_to_ignored: HashMap<u32, HashSet<Rule>>,
file_ignored: HashSet<Rule>,
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::<Vec<_>>();
let mut keys = self.line_to_ignored.keys().collect::<Vec<_>>();
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:?}")?;
}
}
Expand All @@ -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<u32, HashSet<Rule>> = HashMap::new();
let mut line_to_ignored: HashMap<u32, HashSet<Rule>> = HashMap::new();
let mut file_ignored: HashSet<Rule> = 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;
}
Expand Down
17 changes: 16 additions & 1 deletion squawk-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading