Skip to content

Commit 4b9d86f

Browse files
authored
server: add quick fixes to ignore rule violations (#613)
1 parent 07987be commit 4b9d86f

File tree

8 files changed

+407
-145
lines changed

8 files changed

+407
-145
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/squawk_linter/src/ignore.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,10 @@ fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> {
4242
}
4343

4444
// TODO: maybe in a future version we can rename this to squawk-ignore-line
45-
const IGNORE_LINE_TEXT: &str = "squawk-ignore";
46-
const IGNORE_FILE_TEXT: &str = "squawk-ignore-file";
45+
pub const IGNORE_LINE_TEXT: &str = "squawk-ignore";
46+
pub const IGNORE_FILE_TEXT: &str = "squawk-ignore-file";
4747

48-
fn ignore_rule_names(token: &SyntaxToken) -> Option<(&str, TextRange, IgnoreKind)> {
48+
pub fn ignore_rule_info(token: &SyntaxToken) -> Option<(&str, TextRange, IgnoreKind)> {
4949
if let Some((comment_body, range)) = comment_body(token) {
5050
let without_start = comment_body.trim_start();
5151
let trim_start_size = comment_body.len() - without_start.len();
@@ -74,7 +74,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
7474
rowan::WalkEvent::Enter(NodeOrToken::Token(token))
7575
if token.kind() == SyntaxKind::COMMENT =>
7676
{
77-
if let Some((rule_names, range, kind)) = ignore_rule_names(&token) {
77+
if let Some((rule_names, range, kind)) = ignore_rule_info(&token) {
7878
let mut set = HashSet::new();
7979
let mut offset = 0usize;
8080

crates/squawk_linter/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use squawk_syntax::{Parse, SourceFile};
1515

1616
pub use version::Version;
1717

18-
mod ignore;
18+
pub mod ignore;
1919
mod ignore_index;
2020
mod version;
2121
mod visitors;
@@ -250,7 +250,7 @@ pub struct Edit {
250250
pub text: Option<String>,
251251
}
252252
impl Edit {
253-
fn insert<T: Into<String>>(text: T, at: TextSize) -> Self {
253+
pub fn insert<T: Into<String>>(text: T, at: TextSize) -> Self {
254254
Self {
255255
text_range: TextRange::new(at, at),
256256
text: Some(text.into()),

crates/squawk_server/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ squawk_lexer.workspace = true
1919
squawk_linter.workspace = true
2020
squawk_syntax.workspace = true
2121
line-index.workspace = true
22+
insta.workspace = true
2223

2324
[lints]
2425
workspace = true
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
use serde::{Deserialize, Serialize};
2+
3+
pub(crate) const DIAGNOSTIC_NAME: &str = "squawk";
4+
5+
// Based on Ruff's setup for LSP diagnostic edits
6+
// see: https://github.com/astral-sh/ruff/blob/1a368b0bf97c3d0246390679166bbd2d589acf39/crates/ruff_server/src/lint.rs#L31
7+
/// This is serialized on the diagnostic `data` field.
8+
#[derive(Serialize, Deserialize, Debug, Clone)]
9+
pub(crate) struct AssociatedDiagnosticData {
10+
/// The message describing what the fix does, if it exists, or the diagnostic name otherwise.
11+
pub(crate) title: String,
12+
/// Edits to fix the diagnostic. If this is empty, a fix
13+
/// does not exist.
14+
pub(crate) edits: Vec<lsp_types::TextEdit>,
15+
/// Edit to ignore the rule the line
16+
pub(crate) ignore_line_edit: Option<lsp_types::TextEdit>,
17+
/// Edit to ignore the rule for the file
18+
pub(crate) ignore_file_edit: Option<lsp_types::TextEdit>,
19+
}

crates/squawk_server/src/ignore.rs

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
use line_index::{LineIndex, TextSize};
2+
use squawk_linter::{
3+
Edit, Rule, Violation,
4+
ignore::{IGNORE_FILE_TEXT, IGNORE_LINE_TEXT},
5+
};
6+
use squawk_syntax::{Parse, SourceFile, SyntaxKind, SyntaxToken, ast::AstNode};
7+
8+
const UNSUPPORTED_RULES: &[Rule] = &[Rule::UnusedIgnore];
9+
10+
pub(crate) fn ignore_line_edit(
11+
violation: &Violation,
12+
line_index: &LineIndex,
13+
parse: &Parse<SourceFile>,
14+
) -> Option<Edit> {
15+
if UNSUPPORTED_RULES.contains(&violation.code) {
16+
return None;
17+
}
18+
let tree = parse.tree();
19+
let rule_name = violation.code.to_string();
20+
21+
let violation_line = line_index.line_col(violation.text_range.start());
22+
let previous_line = violation_line.line.checked_sub(1)?;
23+
let previous_line_offset = line_index.line(previous_line)?.start();
24+
let previous_line_token = tree
25+
.syntax()
26+
.token_at_offset(previous_line_offset)
27+
.right_biased()?;
28+
29+
match previous_line_token.kind() {
30+
SyntaxKind::COMMENT if is_ignore_comment(&previous_line_token) => {
31+
let (_str, range, _ignore_kind) =
32+
squawk_linter::ignore::ignore_rule_info(&previous_line_token)?;
33+
Some(Edit::insert(format!(" {rule_name},"), range.start()))
34+
}
35+
_ => Some(Edit::insert(
36+
format!("-- {IGNORE_LINE_TEXT} {rule_name}\n"),
37+
violation.text_range.start(),
38+
)),
39+
}
40+
}
41+
42+
pub(crate) fn ignore_file_edit(
43+
violation: &Violation,
44+
_line_index: &LineIndex,
45+
_parse: &Parse<SourceFile>,
46+
) -> Option<Edit> {
47+
if UNSUPPORTED_RULES.contains(&violation.code) {
48+
return None;
49+
}
50+
let rule_name = violation.code.to_string();
51+
Some(Edit::insert(
52+
format!("-- {IGNORE_FILE_TEXT} {rule_name}\n"),
53+
TextSize::new(0),
54+
))
55+
}
56+
57+
fn is_ignore_comment(token: &SyntaxToken) -> bool {
58+
assert_eq!(token.kind(), SyntaxKind::COMMENT);
59+
squawk_linter::ignore::ignore_rule_info(&token).is_some()
60+
}
61+
62+
#[cfg(test)]
63+
mod test {
64+
use crate::{diagnostic::AssociatedDiagnosticData, lint::lint};
65+
66+
#[test]
67+
fn ignore_line() {
68+
let sql = "
69+
create table a (
70+
a int
71+
);
72+
73+
-- an existing comment that shouldn't get in the way of us adding a new ignore
74+
create table b (
75+
b int
76+
);
77+
78+
-- squawk-ignore prefer-text-field
79+
create table c (
80+
b int
81+
);
82+
";
83+
let ignore_line_edits = lint(sql)
84+
.into_iter()
85+
.flat_map(|x| {
86+
let data = x.data?;
87+
let associated_data: AssociatedDiagnosticData =
88+
serde_json::from_value(data).unwrap();
89+
associated_data.ignore_line_edit
90+
})
91+
.collect::<Vec<_>>();
92+
insta::assert_snapshot!(apply_text_edits(sql, ignore_line_edits), @r"
93+
-- squawk-ignore prefer-robust-stmts
94+
create table a (
95+
a int
96+
);
97+
98+
-- an existing comment that shouldn't get in the way of us adding a new ignore
99+
-- squawk-ignore prefer-robust-stmts
100+
create table b (
101+
b int
102+
);
103+
104+
-- squawk-ignore prefer-robust-stmts, prefer-text-field
105+
create table c (
106+
b int
107+
);
108+
");
109+
}
110+
111+
#[test]
112+
fn ignore_file() {
113+
let sql = "
114+
-- some existing comment
115+
create table a (
116+
a int
117+
);
118+
119+
create table b (
120+
b int
121+
);
122+
123+
create table c (
124+
b int
125+
);
126+
";
127+
let ignore_line_edits = lint(sql)
128+
.into_iter()
129+
.flat_map(|x| {
130+
let data = x.data?;
131+
let associated_data: AssociatedDiagnosticData =
132+
serde_json::from_value(data).unwrap();
133+
associated_data.ignore_file_edit
134+
})
135+
.collect::<Vec<_>>();
136+
insta::assert_snapshot!(apply_text_edits(sql, ignore_line_edits), @r"
137+
-- squawk-ignore-file prefer-robust-stmts
138+
-- squawk-ignore-file prefer-robust-stmts
139+
-- squawk-ignore-file prefer-robust-stmts
140+
141+
-- some existing comment
142+
create table a (
143+
a int
144+
);
145+
146+
create table b (
147+
b int
148+
);
149+
150+
create table c (
151+
b int
152+
);
153+
");
154+
}
155+
156+
fn apply_text_edits(sql: &str, mut edits: Vec<lsp_types::TextEdit>) -> String {
157+
use line_index::{LineCol, LineIndex};
158+
159+
// Sort edits by position (reverse order to apply from end to start)
160+
edits.sort_by(|a, b| {
161+
b.range
162+
.start
163+
.line
164+
.cmp(&a.range.start.line)
165+
.then_with(|| b.range.start.character.cmp(&a.range.start.character))
166+
});
167+
168+
let line_index = LineIndex::new(sql);
169+
let mut result = sql.to_string();
170+
171+
for edit in edits {
172+
// Convert LSP positions to byte offsets
173+
let start_offset = line_index.offset(LineCol {
174+
line: edit.range.start.line,
175+
col: edit.range.start.character,
176+
});
177+
let end_offset = line_index.offset(LineCol {
178+
line: edit.range.end.line,
179+
col: edit.range.end.character,
180+
});
181+
182+
let start_byte: usize = start_offset.unwrap_or_default().into();
183+
let end_byte: usize = end_offset.unwrap_or_default().into();
184+
185+
result.replace_range(start_byte..end_byte, &edit.new_text);
186+
}
187+
188+
result
189+
}
190+
}

0 commit comments

Comments
 (0)