diff --git a/e2e/playwright/editor-tests.spec.ts b/e2e/playwright/editor-tests.spec.ts index 106b16ad5..6859d116f 100644 --- a/e2e/playwright/editor-tests.spec.ts +++ b/e2e/playwright/editor-tests.spec.ts @@ -458,8 +458,8 @@ test.describe('Editor tests', () => { /* add the following code to the editor ($ error is not a valid line) $ error - const topAng = 30 - const bottomAng = 25 + topAng = 30 + bottomAng = 25 */ await u.codeLocator.click() await page.keyboard.type('$ error') @@ -474,12 +474,14 @@ test.describe('Editor tests', () => { await page.keyboard.type('bottomAng = 25') await page.keyboard.press('Enter') - // error in guter + // error in gutter await expect(page.locator('.cm-lint-marker-error')).toBeVisible() // error text on hover await page.hover('.cm-lint-marker-error') - await expect(page.getByText('Unexpected token: $').first()).toBeVisible() + await expect( + page.getByText('Tag names must not be empty').first() + ).toBeVisible() // select the line that's causing the error and delete it await page.getByText('$ error').click() diff --git a/src/wasm-lib/kcl/src/parsing/parser.rs b/src/wasm-lib/kcl/src/parsing/parser.rs index 1b9a700c0..7aa65642b 100644 --- a/src/wasm-lib/kcl/src/parsing/parser.rs +++ b/src/wasm-lib/kcl/src/parsing/parser.rs @@ -1855,22 +1855,59 @@ impl TryFrom for Node { type Error = CompilationError; fn try_from(token: Token) -> Result { - if token.token_type == TokenType::Word { - Ok(Node::new( - TagDeclarator { - // We subtract 1 from the start because the tag starts with a `$`. - name: token.value, - digest: None, - }, - token.start - 1, - token.end, - token.module_id, - )) - } else { - Err(CompilationError::fatal( + match token.token_type { + TokenType::Word => { + Ok(Node::new( + TagDeclarator { + // We subtract 1 from the start because the tag starts with a `$`. + name: token.value, + digest: None, + }, + token.start - 1, + token.end, + token.module_id, + )) + } + TokenType::Number => Err(CompilationError::fatal( + token.as_source_range(), + format!( + "Tag names must not start with a number. Tag starts with `{}`", + token.value.as_str() + ), + )), + + // e.g. `line(%, $)` or `line(%, $ , 5)` + TokenType::Brace | TokenType::Whitespace | TokenType::Comma => Err(CompilationError::fatal( + token.as_source_range(), + "Tag names must not be empty".to_string(), + )), + + TokenType::Type => Err(CompilationError::fatal( token.as_source_range(), format!("Cannot assign a tag to a reserved keyword: {}", token.value.as_str()), - )) + )), + + TokenType::Bang + | TokenType::At + | TokenType::Hash + | TokenType::Colon + | TokenType::Period + | TokenType::Operator + | TokenType::DoublePeriod + | TokenType::QuestionMark + | TokenType::BlockComment + | TokenType::Function + | TokenType::String + | TokenType::Dollar + | TokenType::Keyword + | TokenType::Unknown + | TokenType::LineComment => Err(CompilationError::fatal( + token.as_source_range(), + // this is `start with` because if most of these cases are in the middle, it ends + // up hitting a different error path(e.g. including a bang) or being valid(e.g. including a comment) since it will get broken up into + // multiple tokens + format!("Tag names must not start with a {}", token.token_type), + )), } } } @@ -1894,7 +1931,8 @@ fn tag(i: TokenSlice) -> PResult> { let tag_declarator = any .try_map(Node::::try_from) .context(expected("a tag, e.g. '$seg01' or '$line01'")) - .parse_next(i)?; + .parse_next(i) + .map_err(|e| e.cut())?; // Now that we've parsed a tag declarator, verify that it's not a stdlib // name. If it is, stop backtracking. tag_declarator @@ -2372,6 +2410,7 @@ fn fn_call(i: TokenSlice) -> PResult> { opt(whitespace).parse_next(i)?; let _ = terminated(open_paren, opt(whitespace)).parse_next(i)?; let args = arguments(i)?; + if let Some(std_fn) = crate::std::get_stdlib_fn(&fn_name.name) { let just_args: Vec<_> = args.iter().collect(); typecheck_all(std_fn, &just_args)?; @@ -4035,12 +4074,108 @@ let myBox = box([0,0], -3, -16, -10) } #[test] - fn test_parse_empty_tag() { + fn test_parse_empty_tag_brace() { let some_program_string = r#"startSketchOn('XY') |> startProfileAt([0, 0], %) - |> line([5, 5], %, $) -"#; - assert_err(some_program_string, "Unexpected token: |>", [57, 59]); + |> line(%, $) + "#; + assert_err(some_program_string, "Tag names must not be empty", [69, 70]); + } + #[test] + fn test_parse_empty_tag_whitespace() { + let some_program_string = r#"startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $ ,01) + "#; + assert_err(some_program_string, "Tag names must not be empty", [69, 70]); + } + + #[test] + fn test_parse_empty_tag_comma() { + let some_program_string = r#"startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $,) + "#; + assert_err(some_program_string, "Tag names must not be empty", [69, 70]); + } + #[test] + fn test_parse_tag_starting_with_digit() { + let some_program_string = r#" + startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $01)"#; + assert_err( + some_program_string, + "Tag names must not start with a number. Tag starts with `01`", + [74, 76], + ); + } + #[test] + fn test_parse_tag_including_digit() { + let some_program_string = r#" + startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $var01)"#; + assert_no_err(some_program_string); + } + #[test] + fn test_parse_tag_starting_with_bang() { + let some_program_string = r#"startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $!var,01) + "#; + assert_err(some_program_string, "Tag names must not start with a bang", [69, 70]); + } + #[test] + fn test_parse_tag_starting_with_dollar() { + let some_program_string = r#"startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $$,01) + "#; + assert_err(some_program_string, "Tag names must not start with a dollar", [69, 70]); + } + #[test] + fn test_parse_tag_starting_with_fn() { + let some_program_string = r#"startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $fn,01) + "#; + assert_err(some_program_string, "Tag names must not start with a keyword", [69, 71]); + } + #[test] + fn test_parse_tag_starting_with_a_comment() { + let some_program_string = r#"startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line(%, $// + ,01) + "#; + assert_err( + some_program_string, + "Tag names must not start with a lineComment", + [69, 71], + ); + } + + #[test] + fn test_parse_tag_starting_with_reserved_type() { + let some_program_string = r#" + startSketchOn('XY') + |> line(%, $sketch) + "#; + assert_err( + some_program_string, + "Cannot assign a tag to a reserved keyword: sketch", + [41, 47], + ); + } + #[test] + fn test_parse_tag_with_reserved_in_middle_works() { + let some_program_string = r#" + startSketchOn('XY') + |> startProfileAt([0, 0], %) + |> line([5, 5], %, $sketching) + "#; + assert_no_err(some_program_string); } #[test]