diff --git a/src/wasm-lib/kcl/src/errors.rs b/src/wasm-lib/kcl/src/errors.rs index 32f7cbd760..911e2ccc8f 100644 --- a/src/wasm-lib/kcl/src/errors.rs +++ b/src/wasm-lib/kcl/src/errors.rs @@ -362,8 +362,6 @@ impl From for pyo3::PyErr { pub struct CompilationError { #[serde(rename = "sourceRange")] pub source_range: SourceRange, - #[serde(rename = "contextRange")] - pub context_range: Option, pub message: String, pub suggestion: Option, pub severity: Severity, @@ -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, @@ -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, @@ -394,22 +390,18 @@ impl CompilationError { } pub(crate) fn with_suggestion( - source_range: SourceRange, - context_range: Option, - 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 } } diff --git a/src/wasm-lib/kcl/src/parsing/parser.rs b/src/wasm-lib/kcl/src/parsing/parser.rs index b858289adf..f6604002ab 100644 --- a/src/wasm-lib/kcl/src/parsing/parser.rs +++ b/src/wasm-lib/kcl/src/parsing/parser.rs @@ -844,13 +844,13 @@ fn object_property(i: &mut TokenSlice) -> PResult> { }; 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 { 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, 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> { 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 { @@ -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] diff --git a/src/wasm-lib/kcl/src/source_range.rs b/src/wasm-lib/kcl/src/source_range.rs index 7b618b3cd7..a8964afb60 100644 --- a/src/wasm-lib/kcl/src/source_range.rs +++ b/src/wasm-lib/kcl/src/source_range.rs @@ -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]