Skip to content

Commit

Permalink
Fix suggestion for updating function decl syntax for anon functions
Browse files Browse the repository at this point in the history 
Signed-off-by: Nick Cameron <[email protected]>
  • Loading branch information
@nrc
nrc committed Jan 16, 2025
1 parent 412e956 commit 9ccd25f
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 95 deletions.
22 changes: 7 additions & 15 deletions src/wasm-lib/kcl/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,6 @@ impl From<KclError> for pyo3::PyErr {
pub struct CompilationError {
#[serde(rename = "sourceRange")]
pub source_range: SourceRange,
#[serde(rename = "contextRange")]
pub context_range: Option<SourceRange>,
pub message: String,
pub suggestion: Option<Suggestion>,
pub severity: Severity,
Expand All @@ -374,7 +372,6 @@ impl CompilationError {
pub(crate) fn err(source_range: SourceRange, message: impl ToString) -> CompilationError {
CompilationError {
source_range,
context_range: None,
message: message.to_string(),
suggestion: None,
severity: Severity::Error,
Expand All @@ -385,7 +382,6 @@ impl CompilationError {
pub(crate) fn fatal(source_range: SourceRange, message: impl ToString) -> CompilationError {
CompilationError {
source_range,
context_range: None,
message: message.to_string(),
suggestion: None,
severity: Severity::Fatal,
Expand All @@ -394,22 +390,18 @@ impl CompilationError {
}

pub(crate) fn with_suggestion(
source_range: SourceRange,
context_range: Option<SourceRange>,
message: impl ToString,
suggestion: Option<(impl ToString, impl ToString)>,
self,
suggestion_title: impl ToString,
suggestion_insert: impl ToString,
tag: Tag,
) -> CompilationError {
CompilationError {
source_range,
context_range,
message: message.to_string(),
suggestion: suggestion.map(|(t, i)| Suggestion {
title: t.to_string(),
insert: i.to_string(),
suggestion: Some(Suggestion {
title: suggestion_title.to_string(),
insert: suggestion_insert.to_string(),
}),
severity: Severity::Error,
tag,
..self
}
}

Expand Down
175 changes: 95 additions & 80 deletions src/wasm-lib/kcl/src/parsing/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -844,13 +844,13 @@ fn object_property(i: &mut TokenSlice) -> PResult<Node<ObjectProperty>> {
};

if sep.token_type == TokenType::Colon {
ParseContext::warn(CompilationError::with_suggestion(
sep.into(),
Some(result.as_source_range()),
"Using `:` to initialize objects is deprecated, prefer using `=`.",
Some(("Replace `:` with `=`", " =")),
Tag::Deprecated,
));
ParseContext::warn(
CompilationError::err(
sep.into(),
"Using `:` to initialize objects is deprecated, prefer using `=`.",
)
.with_suggestion("Replace `:` with `=`", " =", Tag::Deprecated),
);
}

Ok(result)
Expand Down Expand Up @@ -1069,9 +1069,19 @@ fn function_expr(i: &mut TokenSlice) -> PResult<Expr> {
let fn_tok = opt(fun).parse_next(i)?;
ignore_whitespace(i);
let (result, has_arrow) = function_decl.parse_next(i)?;
if fn_tok.is_none() && !has_arrow {
let err = CompilationError::fatal(result.as_source_range(), "Anonymous function requires `fn` before `(`");
return Err(ErrMode::Cut(err.into()));
if fn_tok.is_none() {
if has_arrow {
ParseContext::warn(
CompilationError::err(
result.as_source_range().start_as_range(),
"Missing `fn` in function declaration",
)
.with_suggestion("Add `fn`", "fn", Tag::None),
);
} else {
let err = CompilationError::fatal(result.as_source_range(), "Anonymous function requires `fn` before `(`");
return Err(ErrMode::Cut(err.into()));
}
}
Ok(Expr::FunctionExpression(Box::new(result)))
}
Expand Down Expand Up @@ -1113,18 +1123,16 @@ fn function_decl(i: &mut TokenSlice) -> PResult<(Node<FunctionExpression>, bool)
open.module_id,
);

let has_arrow = if let Some(arrow) = arrow {
ParseContext::warn(CompilationError::with_suggestion(
arrow.as_source_range(),
Some(result.as_source_range()),
"Unnecessary `=>` in function declaration",
Some(("Remove `=>`", "")),
Tag::Unnecessary,
));
true
} else {
false
};
let has_arrow =
if let Some(arrow) = arrow {
ParseContext::warn(
CompilationError::err(arrow.as_source_range(), "Unnecessary `=>` in function declaration")
.with_suggestion("Remove `=>`", "", Tag::Unnecessary),
);
true
} else {
false
};

Ok((result, has_arrow))
}
Expand Down Expand Up @@ -1825,67 +1833,60 @@ fn declaration(i: &mut TokenSlice) -> PResult<BoxNode<VariableDeclaration>> {

ignore_whitespace(i);

let val = if kind == VariableKind::Fn {
let eq = opt(equals).parse_next(i)?;
ignore_whitespace(i);

let val = function_decl
.map(|t| Box::new(t.0))
.map(Expr::FunctionExpression)
.context(expected("a KCL function expression, like () { return 1 }"))
.parse_next(i);

if let Some(t) = eq {
let ctxt_end = val.as_ref().map(|e| e.end()).unwrap_or(t.end);
ParseContext::warn(CompilationError::with_suggestion(
t.as_source_range(),
Some(SourceRange::new(id.start, ctxt_end, id.module_id)),
"Unnecessary `=` in function declaration",
Some(("Remove `=`", "")),
Tag::Unnecessary,
));
}
let val =
if kind == VariableKind::Fn {
let eq = opt(equals).parse_next(i)?;
ignore_whitespace(i);

let val = function_decl
.map(|t| Box::new(t.0))
.map(Expr::FunctionExpression)
.context(expected("a KCL function expression, like () { return 1 }"))
.parse_next(i);

if let Some(t) = eq {
ParseContext::warn(
CompilationError::err(t.as_source_range(), "Unnecessary `=` in function declaration")
.with_suggestion("Remove `=`", "", Tag::Unnecessary),
);
}

val
} else {
equals(i)?;
ignore_whitespace(i);
val
} else {
equals(i)?;
ignore_whitespace(i);

let val = expression
.try_map(|val| {
// Function bodies can be used if and only if declaring a function.
// Check the 'if' direction:
if matches!(val, Expr::FunctionExpression(_)) {
return Err(CompilationError::fatal(
SourceRange::new(start, dec_end, id.module_id),
format!("Expected a `fn` variable kind, found: `{}`", kind),
));
}
Ok(val)
})
.context(expected("a KCL value, which is being bound to a variable"))
.parse_next(i);

if let Some((_, tok)) = decl_token {
ParseContext::warn(
CompilationError::err(
tok.as_source_range(),
format!(
"Using `{}` to declare constants is deprecated; no keyword is required",
tok.value
),
)
.with_suggestion(format!("Remove `{}`", tok.value), "", Tag::Deprecated),
);
}

let val = expression
.try_map(|val| {
// Function bodies can be used if and only if declaring a function.
// Check the 'if' direction:
if matches!(val, Expr::FunctionExpression(_)) {
return Err(CompilationError::fatal(
SourceRange::new(start, dec_end, id.module_id),
format!("Expected a `fn` variable kind, found: `{}`", kind),
));
}
Ok(val)
})
.context(expected("a KCL value, which is being bound to a variable"))
.parse_next(i);

if let Some((_, tok)) = decl_token {
ParseContext::warn(CompilationError::with_suggestion(
tok.as_source_range(),
Some(SourceRange::new(
id.start,
val.as_ref().map(|e| e.end()).unwrap_or(dec_end),
id.module_id,
)),
format!(
"Using `{}` to declare constants is deprecated; no keyword is required",
tok.value
),
Some((format!("Remove `{}`", tok.value), "")),
Tag::Deprecated,
));
val
}

val
}
.map_err(|e| e.cut())?;
.map_err(|e| e.cut())?;

let end = val.end();
Ok(Box::new(Node {
Expand Down Expand Up @@ -4345,6 +4346,20 @@ sketch001 = startSketchOn('XZ') |> startProfileAt([90.45, 119.09, %)"#;
return 0
}"#
);

let some_program_string = r#"myMap = map([0..5], (n) => {
return n * 2
})"#;
let (_, errs) = assert_no_err(some_program_string);
assert_eq!(errs.len(), 2);
let replaced = errs[0].apply_suggestion(some_program_string).unwrap();
let replaced = errs[1].apply_suggestion(&replaced).unwrap();
assert_eq!(
replaced,
r#"myMap = map([0..5], fn(n) {
return n * 2
})"#
);
}

#[test]
Expand Down
6 changes: 6 additions & 0 deletions src/wasm-lib/kcl/src/source_range.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ impl SourceRange {
self.0[0]
}

/// Get the start of the range as a zero-length SourceRange, effectively collapse `self` to it's
/// start.
pub fn start_as_range(&self) -> Self {
Self([self.0[0], self.0[0], self.0[2]])
}

/// Get the end of the range.
pub fn end(&self) -> usize {
self.0[1]
Expand Down

0 comments on commit 9ccd25f

Please sign in to comment.