ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix suggestion for updating function decl syntax for anon functions (#5088)

Browse Source 
Signed-off-by: Nick Cameron <nrc@ncameron.org>
This commit is contained in:
Nick Cameron
2025-01-22 07:10:07 +13:00
committed by GitHub
parent 4d24bf7c94
commit a022b8ef6c
3 changed files with 108 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/wasm-lib/kcl/src/errors.rs
View File

@ -370,8 +370,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,
@ -382,7 +380,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,
@ -393,7 +390,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,
@ -402,22 +398,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
}
}

175
src/wasm-lib/kcl/src/parsing/parser.rs
View File

@ -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)
@ -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)))
}
@ -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))
}
@ -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 =
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);
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,
));
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);
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),
);
}
val
}
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::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
}
.map_err(|e| e.cut())?;
.map_err(|e| e.cut())?;
let end = val.end();
Ok(Box::new(Node {
@ -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]

6
src/wasm-lib/kcl/src/source_range.rs
View File

@ -76,6 +76,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]