Skip to content

Commit 4cff2c0

Browse files
authored
lsp: add code action for case else clause (#709)
1 parent 634b6fc commit 4cff2c0

File tree

10 files changed

+234
-31
lines changed

10 files changed

+234
-31
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_ide/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ rust-version.workspace = true
1313

1414
[dependencies]
1515
squawk-syntax.workspace = true
16+
squawk-linter.workspace = true
1617
rowan.workspace = true
1718
line-index.workspace = true
1819
annotate-snippets.workspace = true
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
use rowan::TextSize;
2+
use squawk_linter::Edit;
3+
use squawk_syntax::{
4+
SyntaxKind,
5+
ast::{self, AstNode},
6+
};
7+
8+
#[derive(Debug, Clone)]
9+
pub struct CodeAction {
10+
pub title: String,
11+
pub edits: Vec<Edit>,
12+
}
13+
14+
pub fn code_actions(file: ast::SourceFile, offset: TextSize) -> Option<Vec<CodeAction>> {
15+
let mut actions = vec![];
16+
remove_else_clause(&mut actions, &file, offset);
17+
Some(actions)
18+
}
19+
20+
fn remove_else_clause(
21+
actions: &mut Vec<CodeAction>,
22+
file: &ast::SourceFile,
23+
offset: TextSize,
24+
) -> Option<()> {
25+
let else_token = file
26+
.syntax()
27+
.token_at_offset(offset)
28+
.find(|x| x.kind() == SyntaxKind::ELSE_KW)?;
29+
let parent = else_token.parent()?;
30+
let else_clause = ast::ElseClause::cast(parent)?;
31+
32+
let mut edits = vec![];
33+
edits.push(Edit::delete(else_clause.syntax().text_range()));
34+
if let Some(token) = else_token.prev_token() {
35+
if token.kind() == SyntaxKind::WHITESPACE {
36+
edits.push(Edit::delete(token.text_range()));
37+
}
38+
}
39+
40+
actions.push(CodeAction {
41+
title: "Remove `else` clause".to_owned(),
42+
edits,
43+
});
44+
Some(())
45+
}
46+
47+
#[cfg(test)]
48+
mod test {
49+
use super::*;
50+
use crate::test_utils::fixture;
51+
use insta::assert_snapshot;
52+
use rowan::TextSize;
53+
use squawk_syntax::ast;
54+
55+
fn apply_code_action(
56+
f: impl Fn(&mut Vec<CodeAction>, &ast::SourceFile, TextSize) -> Option<()>,
57+
sql: &str,
58+
) -> String {
59+
let (offset, sql) = fixture(sql);
60+
let parse = ast::SourceFile::parse(&sql);
61+
assert_eq!(parse.errors(), vec![]);
62+
let file: ast::SourceFile = parse.tree();
63+
64+
let mut actions = vec![];
65+
f(&mut actions, &file, offset);
66+
67+
assert!(
68+
!actions.is_empty(),
69+
"We should always have actions for `apply_code_action`. If you want to ensure there are no actions, use `code_action_not_applicable` instead."
70+
);
71+
72+
let action = &actions[0];
73+
let mut result = sql.clone();
74+
75+
let mut edits = action.edits.clone();
76+
edits.sort_by_key(|e| e.text_range.start());
77+
check_overlap(&edits);
78+
edits.reverse();
79+
80+
for edit in edits {
81+
let start: usize = edit.text_range.start().into();
82+
let end: usize = edit.text_range.end().into();
83+
let replacement = edit.text.as_deref().unwrap_or("");
84+
result.replace_range(start..end, replacement);
85+
}
86+
87+
let reparse = ast::SourceFile::parse(&result);
88+
assert_eq!(
89+
reparse.errors(),
90+
vec![],
91+
"Code actions shouldn't cause syntax errors"
92+
);
93+
94+
result
95+
}
96+
97+
// There's an invariant where the edits can't overlap.
98+
// For example, if we have an edit that deletes the full `else clause` and
99+
// another edit that deletes the `else` keyword and they overlap, then
100+
// vscode doesn't surface the code action.
101+
fn check_overlap(edits: &[Edit]) {
102+
for (edit_i, edit_j) in edits.iter().zip(edits.iter().skip(1)) {
103+
if let Some(intersection) = edit_i.text_range.intersect(edit_j.text_range) {
104+
assert!(
105+
intersection.is_empty(),
106+
"Edit ranges must not overlap: {:?} and {:?} intersect at {:?}",
107+
edit_i.text_range,
108+
edit_j.text_range,
109+
intersection
110+
);
111+
}
112+
}
113+
}
114+
115+
fn code_action_not_applicable(
116+
f: impl Fn(&mut Vec<CodeAction>, &ast::SourceFile, TextSize) -> Option<()>,
117+
sql: &str,
118+
) -> bool {
119+
let (offset, sql) = fixture(sql);
120+
let parse = ast::SourceFile::parse(&sql);
121+
assert_eq!(parse.errors(), vec![]);
122+
let file: ast::SourceFile = parse.tree();
123+
124+
let mut actions = vec![];
125+
f(&mut actions, &file, offset);
126+
actions.is_empty()
127+
}
128+
129+
#[test]
130+
fn remove_else_clause_() {
131+
assert_snapshot!(apply_code_action(
132+
remove_else_clause,
133+
"select case x when true then 1 else$0 2 end;"),
134+
@"select case x when true then 1 end;"
135+
);
136+
}
137+
138+
#[test]
139+
fn remove_else_clause_before_token() {
140+
assert_snapshot!(apply_code_action(
141+
remove_else_clause,
142+
"select case x when true then 1 $0else 2 end;"),
143+
@"select case x when true then 1 end;"
144+
);
145+
}
146+
147+
#[test]
148+
fn remove_else_clause_not_applicable() {
149+
assert!(code_action_not_applicable(
150+
remove_else_clause,
151+
"select case x when true then 1 else 2 end$0;"
152+
));
153+
}
154+
}

crates/squawk_ide/src/expand_selection.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,8 @@ fn extend_list_item(node: &SyntaxNode) -> Option<TextRange> {
281281
#[cfg(test)]
282282
mod tests {
283283
use super::*;
284+
use crate::test_utils::fixture;
284285
use insta::assert_debug_snapshot;
285-
use rowan::TextSize;
286286
use squawk_syntax::{SourceFile, ast::AstNode};
287287

288288
fn expand(sql: &str) -> Vec<String> {
@@ -306,14 +306,6 @@ mod tests {
306306
results
307307
}
308308

309-
fn fixture(sql: &str) -> (TextSize, String) {
310-
const MARKER: &str = "$0";
311-
if let Some(pos) = sql.find(MARKER) {
312-
return (TextSize::new(pos as u32), sql.replace(MARKER, ""));
313-
}
314-
panic!("No marker found in test SQL");
315-
}
316-
317309
#[test]
318310
fn simple() {
319311
assert_debug_snapshot!(expand(r#"select $01 + 1"#), @r#"

crates/squawk_ide/src/goto_definition.rs

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -38,25 +38,12 @@ fn token_from_offset(file: &ast::SourceFile, offset: TextSize) -> Option<SyntaxT
3838
#[cfg(test)]
3939
mod test {
4040
use crate::goto_definition::goto_definition;
41+
use crate::test_utils::fixture;
4142
use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle};
4243
use insta::assert_snapshot;
4344
use log::info;
44-
use rowan::TextSize;
4545
use squawk_syntax::ast;
4646

47-
// TODO: we should probably use something else since `$0` is valid syntax, maybe `%0`?
48-
const MARKER: &str = "$0";
49-
50-
fn fixture(sql: &str) -> (Option<TextSize>, String) {
51-
if let Some(pos) = sql.find(MARKER) {
52-
return (
53-
Some(TextSize::new((pos - 1) as u32)),
54-
sql.replace(MARKER, ""),
55-
);
56-
}
57-
(None, sql.to_string())
58-
}
59-
6047
#[track_caller]
6148
fn goto(sql: &str) -> String {
6249
goto_(sql).expect("should always find a definition")
@@ -65,14 +52,13 @@ mod test {
6552
#[track_caller]
6653
fn goto_(sql: &str) -> Option<String> {
6754
info!("starting");
68-
let (offset, sql) = fixture(sql);
55+
let (mut offset, sql) = fixture(sql);
56+
// For go to def we want the previous character since we usually put the
57+
// marker after the item we're trying to go to def on.
58+
offset = offset.checked_sub(1.into()).unwrap_or_default();
6959
let parse = ast::SourceFile::parse(&sql);
7060
assert_eq!(parse.errors(), vec![]);
7161
let file: ast::SourceFile = parse.tree();
72-
let Some(offset) = offset else {
73-
info!("offset not found, did you put a marker `$0` in the sql?");
74-
return None;
75-
};
7662
if let Some(result) = goto_definition(file, offset) {
7763
let offset: usize = offset.into();
7864
let group = Level::INFO.primary_title("definition").element(

crates/squawk_ide/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
1+
pub mod code_actions;
12
pub mod expand_selection;
23
pub mod goto_definition;
4+
#[cfg(test)]
5+
pub mod test_utils;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
use rowan::TextSize;
2+
3+
// TODO: we should probably use something else since `$0` is valid syntax, maybe `%0`?
4+
const MARKER: &str = "$0";
5+
6+
#[track_caller]
7+
pub(crate) fn fixture(sql: &str) -> (TextSize, String) {
8+
if let Some(pos) = sql.find(MARKER) {
9+
return (TextSize::new(pos as u32), sql.replace(MARKER, ""));
10+
}
11+
panic!("No marker found in SQL. Did you forget to add a marker `$0`?");
12+
}

crates/squawk_linter/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ impl Fix {
228228
#[derive(Debug, Clone, PartialEq, Eq)]
229229
pub struct Edit {
230230
pub text_range: TextRange,
231+
// TODO: does this need to be an Option?
231232
pub text: Option<String>,
232233
}
233234
impl Edit {
@@ -243,6 +244,12 @@ impl Edit {
243244
text: Some(text.into()),
244245
}
245246
}
247+
pub fn delete(text_range: TextRange) -> Self {
248+
Self {
249+
text_range,
250+
text: None,
251+
}
252+
}
246253
}
247254

248255
#[derive(Debug, Clone, PartialEq, Eq)]

crates/squawk_server/src/lib.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use lsp_types::{
1717
request::{CodeActionRequest, GotoDefinition, Request, SelectionRangeRequest},
1818
};
1919
use rowan::TextRange;
20+
use squawk_ide::code_actions::code_actions;
2021
use squawk_ide::goto_definition::goto_definition;
2122
use squawk_syntax::{Parse, SourceFile};
2223
use std::collections::HashMap;
@@ -235,12 +236,25 @@ fn handle_selection_range(
235236
fn handle_code_action(
236237
connection: &Connection,
237238
req: lsp_server::Request,
238-
_documents: &HashMap<Url, DocumentState>,
239+
documents: &HashMap<Url, DocumentState>,
239240
) -> Result<()> {
240241
let params: CodeActionParams = serde_json::from_value(req.params)?;
241242
let uri = params.text_document.uri;
242243

243-
let mut actions = Vec::new();
244+
let mut actions: CodeActionResponse = Vec::new();
245+
246+
let content = documents.get(&uri).map_or("", |doc| &doc.content);
247+
let parse: Parse<SourceFile> = SourceFile::parse(content);
248+
let file = parse.tree();
249+
let line_index = LineIndex::new(content);
250+
let offset = lsp_utils::offset(&line_index, params.range.start).unwrap();
251+
252+
let ide_actions = code_actions(file, offset).unwrap_or_default();
253+
254+
for action in ide_actions {
255+
let lsp_action = lsp_utils::code_action(&line_index, uri.clone(), action);
256+
actions.push(CodeActionOrCommand::CodeAction(lsp_action));
257+
}
244258

245259
for mut diagnostic in params
246260
.context

crates/squawk_server/src/lsp_utils.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
use std::ops::Range;
1+
use std::{collections::HashMap, ops::Range};
22

33
use line_index::{LineIndex, TextRange, TextSize};
44
use log::warn;
5+
use lsp_types::{CodeAction, CodeActionKind, Url, WorkspaceEdit};
56

67
fn text_range(index: &LineIndex, range: lsp_types::Range) -> Option<TextRange> {
78
let start = offset(index, range.start)?;
@@ -36,6 +37,38 @@ pub(crate) fn offset(index: &LineIndex, position: lsp_types::Position) -> Option
3637
Some(line_range.start() + clamped_len)
3738
}
3839

40+
pub(crate) fn code_action(
41+
line_index: &LineIndex,
42+
uri: Url,
43+
action: squawk_ide::code_actions::CodeAction,
44+
) -> lsp_types::CodeAction {
45+
CodeAction {
46+
title: action.title,
47+
kind: Some(CodeActionKind::QUICKFIX),
48+
diagnostics: None,
49+
edit: Some(WorkspaceEdit {
50+
changes: Some({
51+
let mut changes = HashMap::new();
52+
let edits = action
53+
.edits
54+
.into_iter()
55+
.map(|edit| lsp_types::TextEdit {
56+
range: range(line_index, edit.text_range),
57+
new_text: edit.text.unwrap_or_default(),
58+
})
59+
.collect();
60+
changes.insert(uri, edits);
61+
changes
62+
}),
63+
..Default::default()
64+
}),
65+
command: None,
66+
is_preferred: Some(true),
67+
disabled: None,
68+
data: None,
69+
}
70+
}
71+
3972
pub(crate) fn range(line_index: &LineIndex, range: TextRange) -> lsp_types::Range {
4073
let start = line_index.line_col(range.start());
4174
let end = line_index.line_col(range.end());

0 commit comments

Comments
 (0)