Skip to content

Commit

Permalink
Allow using a different source range for suggestions than the error m…
Browse files Browse the repository at this point in the history
…essage (#5442)

Signed-off-by: Nick Cameron <[email protected]>
  • Loading branch information
nrc authored Feb 22, 2025
1 parent cbc1255 commit 57f7d02
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 17 deletions.
6 changes: 5 additions & 1 deletion src/lang/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,11 @@ export function complilationErrorsToDiagnostics(
name: suggestion.title,
apply: (view: EditorView, from: number, to: number) => {
view.dispatch({
changes: { from, to, insert: suggestion.insert },
changes: {
from: suggestion.source_range[0],
to: suggestion.source_range[1],
insert: suggestion.insert,
},
})
},
},
Expand Down
8 changes: 6 additions & 2 deletions src/wasm-lib/kcl/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,12 +402,15 @@ impl CompilationError {
self,
suggestion_title: impl ToString,
suggestion_insert: impl ToString,
// Will use the error source range if none is supplied
source_range: Option<SourceRange>,
tag: Tag,
) -> CompilationError {
CompilationError {
suggestion: Some(Suggestion {
title: suggestion_title.to_string(),
insert: suggestion_insert.to_string(),
source_range: source_range.unwrap_or(self.source_range),
}),
tag,
..self
Expand All @@ -419,9 +422,9 @@ impl CompilationError {
let suggestion = self.suggestion.as_ref()?;
Some(format!(
"{}{}{}",
&src[0..self.source_range.start()],
&src[0..suggestion.source_range.start()],
suggestion.insert,
&src[self.source_range.end()..]
&src[suggestion.source_range.end()..]
))
}
}
Expand Down Expand Up @@ -465,4 +468,5 @@ pub enum Tag {
pub struct Suggestion {
pub title: String,
pub insert: String,
pub source_range: SourceRange,
}
9 changes: 4 additions & 5 deletions src/wasm-lib/kcl/src/lsp/kcl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1373,12 +1373,11 @@ impl LanguageServer for Backend {
.diagnostics
.into_iter()
.filter_map(|diagnostic| {
let suggestion = diagnostic
.data
.as_ref()
.and_then(|data| serde_json::from_value::<Suggestion>(data.clone()).ok())?;
let (suggestion, range) = diagnostic.data.as_ref().and_then(|data| {
serde_json::from_value::<(Suggestion, tower_lsp::lsp_types::Range)>(data.clone()).ok()
})?;
let edit = TextEdit {
range: diagnostic.range,
range,
new_text: suggestion.insert,
};
let changes = HashMap::from([(params.text_document.uri.clone(), vec![edit])]);
Expand Down
5 changes: 4 additions & 1 deletion src/wasm-lib/kcl/src/lsp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ use crate::{

impl IntoDiagnostic for CompilationError {
fn to_lsp_diagnostic(&self, code: &str) -> Diagnostic {
let edit = self.suggestion.as_ref().map(|s| serde_json::to_value(s).unwrap());
let edit = self.suggestion.as_ref().map(|s| {
let range = s.source_range.to_lsp_range(code);
serde_json::to_value((s, range)).unwrap()
});

Diagnostic {
range: self.source_range.to_lsp_range(code),
Expand Down
22 changes: 14 additions & 8 deletions src/wasm-lib/kcl/src/parsing/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ fn object_property(i: &mut TokenSlice) -> PResult<Node<ObjectProperty>> {
sep.into(),
"Using `:` to initialize objects is deprecated, prefer using `=`.",
)
.with_suggestion("Replace `:` with `=`", " =", Tag::Deprecated),
.with_suggestion("Replace `:` with `=`", " =", None, Tag::Deprecated),
);
}

Expand Down Expand Up @@ -1104,7 +1104,7 @@ fn function_expr(i: &mut TokenSlice) -> PResult<Expr> {
result.as_source_range().start_as_range(),
"Missing `fn` in function declaration",
)
.with_suggestion("Add `fn`", "fn", Tag::None),
.with_suggestion("Add `fn`", "fn", None, Tag::None),
);
} else {
let err = CompilationError::fatal(result.as_source_range(), "Anonymous function requires `fn` before `(`");
Expand Down Expand Up @@ -1161,7 +1161,7 @@ fn function_decl(i: &mut TokenSlice) -> PResult<(Node<FunctionExpression>, bool)
if let Some(arrow) = arrow {
ParseContext::warn(
CompilationError::err(arrow.as_source_range(), "Unnecessary `=>` in function declaration")
.with_suggestion("Remove `=>`", "", Tag::Unnecessary),
.with_suggestion("Remove `=>`", "", None, Tag::Unnecessary),
);
true
} else {
Expand Down Expand Up @@ -1997,7 +1997,7 @@ fn declaration(i: &mut TokenSlice) -> PResult<BoxNode<VariableDeclaration>> {
if let Some(t) = eq {
ParseContext::warn(
CompilationError::err(t.as_source_range(), "Unnecessary `=` in function declaration")
.with_suggestion("Remove `=`", "", Tag::Unnecessary),
.with_suggestion("Remove `=`", "", None, Tag::Unnecessary),
);
}

Expand All @@ -2022,6 +2022,7 @@ fn declaration(i: &mut TokenSlice) -> PResult<BoxNode<VariableDeclaration>> {
.parse_next(i);

if let Some((_, tok)) = decl_token {
let range_to_remove = SourceRange::new(tok.start, id.start, id.module_id);
ParseContext::warn(
CompilationError::err(
tok.as_source_range(),
Expand All @@ -2030,7 +2031,12 @@ fn declaration(i: &mut TokenSlice) -> PResult<BoxNode<VariableDeclaration>> {
tok.value
),
)
.with_suggestion(format!("Remove `{}`", tok.value), "", Tag::Deprecated),
.with_suggestion(
format!("Remove `{}`", tok.value),
"",
Some(range_to_remove),
Tag::Deprecated,
),
);
}

Expand Down Expand Up @@ -4610,9 +4616,9 @@ var baz = 2
let replaced = errs[0].apply_suggestion(&replaced).unwrap();
assert_eq!(
replaced,
r#" foo = 0
bar = 1
baz = 2
r#"foo = 0
bar = 1
baz = 2
"#
);
}
Expand Down

0 comments on commit 57f7d02

Please sign in to comment.