From e706208ec3fe523b8f8a27ad48ff35e08ac43633 Mon Sep 17 00:00:00 2001 From: Tom Pridham Date: Wed, 15 Jan 2025 22:59:11 -0700 Subject: [PATCH 1/5] fix: add better errors for missing commas in arrays and objects --- src/wasm-lib/kcl/src/parsing/parser.rs | 140 ++++++++++++++++++---- src/wasm-lib/kcl/src/parsing/token/mod.rs | 19 +++ 2 files changed, 133 insertions(+), 26 deletions(-) diff --git a/src/wasm-lib/kcl/src/parsing/parser.rs b/src/wasm-lib/kcl/src/parsing/parser.rs index 805f02d40c..acd02a38d5 100644 --- a/src/wasm-lib/kcl/src/parsing/parser.rs +++ b/src/wasm-lib/kcl/src/parsing/parser.rs @@ -9,7 +9,7 @@ use winnow::{ error::{ErrMode, StrContext, StrContextValue}, prelude::*, stream::Stream, - token::{any, one_of, take_till}, + token::{any, one_of, take_till, take_until}, }; use super::{ @@ -739,20 +739,38 @@ pub(crate) fn array_elem_by_elem(i: &mut TokenSlice) -> PResult> = peek(take_until(0.., "]")).parse_next(i); + let has_closing_bracket = maybe_closing_bracket.is_ok(); + let maybe_end = close_bracket(i).map_err(|e| { + if let Some(mut err) = e.clone().into_inner() { + err.cause = Some(CompilationError::fatal( + open.as_source_range(), + "Array is missing a closing bracket(`]`)", + )); + ErrMode::Cut(err) + } else { + // ErrMode::Incomplete, not sure if it's actually possible to end up with this here + e + } + }); + + // if there is a closing bracket at some point, but it's not the next token after skipping the + // whitespace and ignoring a trailing comma, it's likely that they forgot a comma between some + // of the elements + if has_closing_bracket && maybe_end.is_err() { + // safe to unwrap here because we checked it was Ok above + let start_range = maybe_closing_bracket.unwrap().as_source_range(); + let e = ContextError { + context: vec![], + cause: Some(CompilationError::fatal( + start_range, + "Array is missing a comma in between elements", + )), + }; + return Err(ErrMode::Cut(e)); + } + let end = maybe_end?.end; // Sort the array's elements (i.e. expression nodes) from the noncode nodes. let (elements, non_code_nodes): (Vec<_>, HashMap) = elements.into_iter().enumerate().fold( @@ -811,7 +829,7 @@ fn array_end_start(i: &mut TokenSlice) -> PResult> { } fn object_property_same_key_and_val(i: &mut TokenSlice) -> PResult> { - let key = nameable_identifier.context(expected("the property's key (the name or identifier of the property), e.g. in 'height: 4', 'height' is the property key")).parse_next(i)?; + let key = nameable_identifier.context(expected("the property's key (the name or identifier of the property), e.g. in 'height = 4', 'height' is the property key")).parse_next(i)?; ignore_whitespace(i); Ok(Node { start: key.start, @@ -837,7 +855,7 @@ fn object_property(i: &mut TokenSlice) -> PResult> { ignore_whitespace(i); let expr = expression_but_not_ascription .context(expected( - "the value which you're setting the property to, e.g. in 'height: 4', the value is 4", + "the value which you're setting the property to, e.g. in 'height = 4', the value is 4", )) .parse_next(i)?; @@ -870,7 +888,7 @@ fn property_separator(i: &mut TokenSlice) -> PResult<()> { alt(( // Normally you need a comma. comma_sep, - // But, if the array is ending, no need for a comma. + // But, if the object is ending, no need for a comma. peek(preceded(opt(whitespace), close_brace)).void(), )) .parse_next(i) @@ -893,10 +911,45 @@ pub(crate) fn object(i: &mut TokenSlice) -> PResult> { )), ) .context(expected( - "a comma-separated list of key-value pairs, e.g. 'height: 4, width: 3'", + "a comma-separated list of key-value pairs, e.g. 'height = 4, width = 3'", )) .parse_next(i)?; + ignore_trailing_comma(i); + ignore_whitespace(i); + let maybe_closing_brace: PResult> = peek(take_until(0.., "}")).parse_next(i); + let has_closing_brace = maybe_closing_brace.is_ok(); + // This will be an error if the next token is not a `}`, this can be because they forgot to + // include a `}` entirely or they forgot a comma somewhere in the object + let maybe_end = close_brace(i).map_err(|e| { + if let Some(mut err) = e.clone().into_inner() { + err.cause = Some(CompilationError::fatal( + open.as_source_range(), + "Object is missing a closing brace(`}`)", + )); + ErrMode::Cut(err) + } else { + // ErrMode::Incomplete, not sure if it's actually possible to end up with this here + e + } + }); + // if there is a closing brace at some point, but it's not the next token after skipping the + // whitespace and ignoring a trailing comma, it's likely that they forgot a comma between some + // of the properties + if has_closing_brace && maybe_end.is_err() { + // okay to unwrap here because we checked it was Ok above + let start_range = maybe_closing_brace.unwrap().as_source_range(); + let e = ContextError { + context: vec![], + cause: Some(CompilationError::fatal( + start_range, + "Object is missing a comma in between properties", + )), + }; + return Err(ErrMode::Cut(e)); + } + + let end = maybe_end?.end; // Sort the object's properties from the noncode nodes. let (properties, non_code_nodes): (Vec<_>, HashMap) = properties.into_iter().enumerate().fold( (Vec::new(), HashMap::new()), @@ -912,9 +965,7 @@ pub(crate) fn object(i: &mut TokenSlice) -> PResult> { (properties, non_code_nodes) }, ); - ignore_trailing_comma(i); - ignore_whitespace(i); - let end = close_brace(i)?.end; + let non_code_meta = NonCodeMeta { non_code_nodes, ..Default::default() @@ -3694,7 +3745,7 @@ mySk1 = startSketchAt([0, 0])"#; assert_eq!( src_expected, src_actual, - "expected error would highlight {} but it actually highlighted {}", + "expected error would highlight `{}` but it actually highlighted `{}`", &p[src_expected[0]..src_expected[1]], &p[src_actual[0]..src_actual[1]], ); @@ -3769,7 +3820,7 @@ height = [obj["a"] - 1, 0]"#; #[test] fn test_parse_member_expression_binary_expression_in_array_number_second_missing_space() { let code = r#"obj = { a: 1, b: 2 } -height = [obj["a"] -1, 0]"#; + height = [obj["a"] -1, 0]"#; crate::parsing::top_level_parse(code).unwrap(); } @@ -3885,7 +3936,11 @@ z(-[["#, #[test] fn test_parse_weird_lots_of_fancy_brackets() { - assert_err(r#"zz({{{{{{{{)iegAng{{{{{{{##"#, "Unexpected token: (", [2, 3]); + assert_err( + r#"zz({{{{{{{{)iegAng{{{{{{{##"#, + "Object is missing a closing brace(`}`)", + [2, 3], + ); } #[test] @@ -4447,11 +4502,44 @@ let myBox = box([0,0], -3, -16, -10) } #[test] - fn test_parse_missing_closing_bracket() { + fn test_parse_array_missing_closing_bracket() { let some_program_string = r#" sketch001 = startSketchOn('XZ') |> startProfileAt([90.45, 119.09, %)"#; assert_err(some_program_string, "Array is missing a closing bracket(`]`)", [51, 52]); } + #[test] + fn test_parse_array_missing_comma() { + let some_program_string = r#" +sketch001 = startSketchOn('XZ') |> startProfileAt([90.45 119.09], %)"#; + assert_err( + some_program_string, + "Array is missing a comma in between elements", + [52, 65], + ); + } + + #[test] + fn test_parse_object_missing_closing_brace() { + let some_program_string = r#"{ + foo = bar, + "#; + + assert_err(some_program_string, "Object is missing a closing brace(`}`)", [0, 1]); + } + #[test] + fn test_parse_object_missing_comma() { + let some_program_string = r#"{ + foo = bar, + bar = foo + bat = man + }"#; + + assert_err( + some_program_string, + "Object is missing a comma in between properties", + [37, 78], + ); + } #[test] fn warn_object_expr() { diff --git a/src/wasm-lib/kcl/src/parsing/token/mod.rs b/src/wasm-lib/kcl/src/parsing/token/mod.rs index bd64109116..f700a45776 100644 --- a/src/wasm-lib/kcl/src/parsing/token/mod.rs +++ b/src/wasm-lib/kcl/src/parsing/token/mod.rs @@ -147,7 +147,9 @@ impl IntoIterator for TokenStream { #[derive(Debug, Clone)] pub(crate) struct TokenSlice<'a> { stream: &'a TokenStream, + /// Current position of the leading Token in the stream start: usize, + /// The number of total Tokens in the stream end: usize, } @@ -175,6 +177,15 @@ impl<'a> TokenSlice<'a> { stream: self.stream, } } + + pub fn as_source_range(&self) -> SourceRange { + let first_token = self.token(0); + SourceRange::new( + first_token.start, + self.stream.tokens[self.end].end, + first_token.module_id, + ) + } } impl<'a> IntoIterator for TokenSlice<'a> { @@ -279,6 +290,14 @@ impl<'a> winnow::stream::StreamIsPartial for TokenSlice<'a> { } } +impl<'a> winnow::stream::FindSlice<&str> for TokenSlice<'a> { + fn find_slice(&self, substr: &str) -> Option> { + self.iter() + .enumerate() + .find_map(|(i, b)| if b.value == substr { Some(i..self.end) } else { None }) + } +} + #[derive(Clone, Debug)] pub struct Checkpoint(usize, usize); From 3d8089a4c28ae1ccfed0b94354ef21c26494fe5b Mon Sep 17 00:00:00 2001 From: Tom Pridham Date: Mon, 20 Jan 2025 13:31:34 -0700 Subject: [PATCH 2/5] chore: add object prop shorthand missing comma test --- src/wasm-lib/kcl/src/parsing/parser.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/wasm-lib/kcl/src/parsing/parser.rs b/src/wasm-lib/kcl/src/parsing/parser.rs index acd02a38d5..b9f95ab3c8 100644 --- a/src/wasm-lib/kcl/src/parsing/parser.rs +++ b/src/wasm-lib/kcl/src/parsing/parser.rs @@ -4541,6 +4541,23 @@ sketch001 = startSketchOn('XZ') |> startProfileAt([90.45 119.09], %)"#; ); } + #[test] + fn test_parse_object_shorthand_missing_comma() { + let some_program_string = r#" +bar = 1 + { + foo = bar, + bar + bat = man + }"#; + + assert_err( + some_program_string, + "Object is missing a comma in between properties", + [54, 89], + ); + } + #[test] fn warn_object_expr() { let some_program_string = "{ foo: bar }"; From 4ddf3da374eed1d1b41f55d3250256adcd22e556 Mon Sep 17 00:00:00 2001 From: Tom Pridham Date: Sat, 1 Feb 2025 16:47:13 -0700 Subject: [PATCH 3/5] fix: wording on unexpected character in arrays and objects --- src/lang/abstractSyntaxTree.test.ts | 6 +++-- src/wasm-lib/kcl/src/parsing/parser.rs | 33 +++++++++++++++++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/lang/abstractSyntaxTree.test.ts b/src/lang/abstractSyntaxTree.test.ts index 8a830a3a66..18bbf618a2 100644 --- a/src/lang/abstractSyntaxTree.test.ts +++ b/src/lang/abstractSyntaxTree.test.ts @@ -53,7 +53,9 @@ describe('parsing errors', () => { const result = parse(code) if (err(result)) throw result const error = result.errors[0] - expect(error.message).toBe('Array is missing a closing bracket(`]`)') - expect(error.sourceRange).toEqual([28, 29, 0]) + expect(error.message).toBe( + 'Unexpected character encountered. You might be missing a comma in between elements.' + ) + expect(error.sourceRange).toEqual([29, 31, 0]) }) }) diff --git a/src/wasm-lib/kcl/src/parsing/parser.rs b/src/wasm-lib/kcl/src/parsing/parser.rs index b9f95ab3c8..dcfe0c2ffc 100644 --- a/src/wasm-lib/kcl/src/parsing/parser.rs +++ b/src/wasm-lib/kcl/src/parsing/parser.rs @@ -765,7 +765,7 @@ pub(crate) fn array_elem_by_elem(i: &mut TokenSlice) -> PResult PResult> { context: vec![], cause: Some(CompilationError::fatal( start_range, - "Object is missing a comma in between properties", + "Unexpected character encountered. You might be missing a comma in between properties.", )), }; return Err(ErrMode::Cut(e)); @@ -3939,7 +3939,7 @@ z(-[["#, assert_err( r#"zz({{{{{{{{)iegAng{{{{{{{##"#, "Object is missing a closing brace(`}`)", - [2, 3], + [3, 4], ); } @@ -4513,10 +4513,20 @@ sketch001 = startSketchOn('XZ') |> startProfileAt([90.45, 119.09, %)"#; sketch001 = startSketchOn('XZ') |> startProfileAt([90.45 119.09], %)"#; assert_err( some_program_string, - "Array is missing a comma in between elements", + "Unexpected character encountered. You might be missing a comma in between elements.", [52, 65], ); } + #[test] + fn test_parse_array_random_brace() { + let some_program_string = r#" +sketch001 = startSketchOn('XZ') |> startProfileAt([}], %)"#; + assert_err( + some_program_string, + "Unexpected character encountered. You might be missing a comma in between elements.", + [52, 54], + ); + } #[test] fn test_parse_object_missing_closing_brace() { @@ -4536,11 +4546,22 @@ sketch001 = startSketchOn('XZ') |> startProfileAt([90.45 119.09], %)"#; assert_err( some_program_string, - "Object is missing a comma in between properties", + "Unexpected character encountered. You might be missing a comma in between properties.", [37, 78], ); } + #[test] + fn test_parse_object_random_bracket() { + let some_program_string = r#"{]}"#; + + assert_err( + some_program_string, + "Unexpected character encountered. You might be missing a comma in between properties.", + [1, 3], + ); + } + #[test] fn test_parse_object_shorthand_missing_comma() { let some_program_string = r#" @@ -4553,7 +4574,7 @@ bar = 1 assert_err( some_program_string, - "Object is missing a comma in between properties", + "Unexpected character encountered. You might be missing a comma in between properties.", [54, 89], ); } From 2b0944fb9b7d0f35383095ea1ae971a4e7e5d483 Mon Sep 17 00:00:00 2001 From: Tom Pridham Date: Mon, 10 Feb 2025 18:27:55 -0700 Subject: [PATCH 4/5] fix: don't eagerly evaluate whether there is a closing brace/bracket --- src/wasm-lib/kcl/src/parsing/parser.rs | 72 +++++++++++++------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/src/wasm-lib/kcl/src/parsing/parser.rs b/src/wasm-lib/kcl/src/parsing/parser.rs index bf1956e63f..f8c1d23353 100644 --- a/src/wasm-lib/kcl/src/parsing/parser.rs +++ b/src/wasm-lib/kcl/src/parsing/parser.rs @@ -748,8 +748,6 @@ pub(crate) fn array_elem_by_elem(i: &mut TokenSlice) -> PResult> = peek(take_until(0.., "]")).parse_next(i); - let has_closing_bracket = maybe_closing_bracket.is_ok(); let maybe_end = close_bracket(i).map_err(|e| { if let Some(mut err) = e.clone().into_inner() { err.cause = Some(CompilationError::fatal( @@ -763,20 +761,24 @@ pub(crate) fn array_elem_by_elem(i: &mut TokenSlice) -> PResult> = peek(take_until(0.., "]")).parse_next(i); + let has_closing_bracket = maybe_closing_bracket.is_ok(); + if has_closing_bracket { + // safe to unwrap here because we checked it was Ok above + let start_range = maybe_closing_bracket.unwrap().as_source_range(); + let e = ContextError { + context: vec![], + cause: Some(CompilationError::fatal( + start_range, + "Unexpected character encountered. You might be missing a comma in between elements.", + )), + }; + return Err(ErrMode::Cut(e)); + } } let end = maybe_end?.end; @@ -927,10 +929,6 @@ pub(crate) fn object(i: &mut TokenSlice) -> PResult> { ignore_trailing_comma(i); ignore_whitespace(i); - let maybe_closing_brace: PResult> = peek(take_until(0.., "}")).parse_next(i); - let has_closing_brace = maybe_closing_brace.is_ok(); - // This will be an error if the next token is not a `}`, this can be because they forgot to - // include a `}` entirely or they forgot a comma somewhere in the object let maybe_end = close_brace(i).map_err(|e| { if let Some(mut err) = e.clone().into_inner() { err.cause = Some(CompilationError::fatal( @@ -943,20 +941,24 @@ pub(crate) fn object(i: &mut TokenSlice) -> PResult> { e } }); - // if there is a closing brace at some point, but it's not the next token after skipping the - // whitespace and ignoring a trailing comma, it's likely that they forgot a comma between some - // of the properties - if has_closing_brace && maybe_end.is_err() { - // okay to unwrap here because we checked it was Ok above - let start_range = maybe_closing_brace.unwrap().as_source_range(); - let e = ContextError { - context: vec![], - cause: Some(CompilationError::fatal( - start_range, - "Unexpected character encountered. You might be missing a comma in between properties.", - )), - }; - return Err(ErrMode::Cut(e)); + if maybe_end.is_err() { + // if there is a closing brace at some point, but it's not the next token after skipping the + // whitespace and ignoring a trailing comma, it's likely that they forgot a comma between some + // of the properties + let maybe_closing_brace: PResult> = peek(take_until(0.., "}")).parse_next(i); + let has_closing_brace = maybe_closing_brace.is_ok(); + if has_closing_brace { + // okay to unwrap here because we checked it was Ok above + let start_range = maybe_closing_brace.unwrap().as_source_range(); + let e = ContextError { + context: vec![], + cause: Some(CompilationError::fatal( + start_range, + "Unexpected character encountered. You might be missing a comma in between properties.", + )), + }; + return Err(ErrMode::Cut(e)); + } } let end = maybe_end?.end; @@ -3876,7 +3878,7 @@ height = [obj["a"] - 1, 0]"#; #[test] fn test_parse_member_expression_binary_expression_in_array_number_second_missing_space() { let code = r#"obj = { a: 1, b: 2 } - height = [obj["a"] -1, 0]"#; +height = [obj["a"] -1, 0]"#; crate::parsing::top_level_parse(code).unwrap(); } From 5fd572eaadcde26d02013e679c046b47cd666e59 Mon Sep 17 00:00:00 2001 From: Tom Pridham Date: Thu, 13 Feb 2025 14:05:13 -0700 Subject: [PATCH 5/5] feat: exit early when detecting missing commas if encountering invalid tokens --- src/wasm-lib/kcl/src/parsing/parser.rs | 128 +++++++++++++++++----- src/wasm-lib/kcl/src/parsing/token/mod.rs | 19 ++-- 2 files changed, 114 insertions(+), 33 deletions(-) diff --git a/src/wasm-lib/kcl/src/parsing/parser.rs b/src/wasm-lib/kcl/src/parsing/parser.rs index f8c1d23353..6d45c2ec40 100644 --- a/src/wasm-lib/kcl/src/parsing/parser.rs +++ b/src/wasm-lib/kcl/src/parsing/parser.rs @@ -4,17 +4,17 @@ use std::{cell::RefCell, collections::BTreeMap}; use winnow::{ - combinator::{alt, delimited, opt, peek, preceded, repeat, separated, separated_pair, terminated}, + combinator::{alt, delimited, opt, peek, preceded, repeat, repeat_till, separated, separated_pair, terminated}, dispatch, error::{ErrMode, StrContext, StrContextValue}, prelude::*, stream::Stream, - token::{any, one_of, take_till, take_until}, + token::{any, none_of, one_of, take_till}, }; use super::{ ast::types::{ImportPath, LabelledExpression}, - token::NumericSuffix, + token::{NumericSuffix, RESERVED_WORDS}, }; use crate::{ docs::StdLibFn, @@ -746,13 +746,16 @@ pub(crate) fn array_elem_by_elem(i: &mut TokenSlice) -> PResult PResult> = peek(take_until(0.., "]")).parse_next(i); + let maybe_closing_bracket: PResult<((), Token)> = peek(repeat_till( + 0.., + none_of(|token: Token| { + // bail out early if we encounter something that is for sure not allowed in an + // array, otherwise we could seek to find a closing bracket until the end of the + // file + RESERVED_WORDS + .keys() + .chain([",,", "{", "}", "["].iter()) + .any(|word| *word == token.value) + }) + .void(), + one_of(|term: Token| term.value == "]"), + )) + .parse_next(i); let has_closing_bracket = maybe_closing_bracket.is_ok(); if has_closing_bracket { + let start_range = i.as_source_range(); // safe to unwrap here because we checked it was Ok above - let start_range = maybe_closing_bracket.unwrap().as_source_range(); + let end_range = maybe_closing_bracket.unwrap().1.as_source_range(); let e = ContextError { context: vec![], cause: Some(CompilationError::fatal( - start_range, + SourceRange::from([start_range.start(), end_range.end(), end_range.module_id().as_usize()]), "Unexpected character encountered. You might be missing a comma in between elements.", )), }; @@ -931,9 +948,11 @@ pub(crate) fn object(i: &mut TokenSlice) -> PResult> { let maybe_end = close_brace(i).map_err(|e| { if let Some(mut err) = e.clone().into_inner() { + let start_range = open.as_source_range(); + let end_range = i.as_source_range(); err.cause = Some(CompilationError::fatal( - open.as_source_range(), - "Object is missing a closing brace(`}`)", + SourceRange::from([start_range.start(), end_range.start(), end_range.module_id().as_usize()]), + "Encountered an unexpected character(s) before finding a closing brace(`}`) for the object", )); ErrMode::Cut(err) } else { @@ -942,18 +961,33 @@ pub(crate) fn object(i: &mut TokenSlice) -> PResult> { } }); if maybe_end.is_err() { - // if there is a closing brace at some point, but it's not the next token after skipping the - // whitespace and ignoring a trailing comma, it's likely that they forgot a comma between some + // if there is a closing brace at some point, but it wasn't the next token, it's likely that they forgot a comma between some // of the properties - let maybe_closing_brace: PResult> = peek(take_until(0.., "}")).parse_next(i); + let maybe_closing_brace: PResult<((), Token)> = peek(repeat_till( + 0.., + none_of(|token: Token| { + // bail out early if we encounter something that is for sure not allowed in an + // object, otherwise we could seek to find a closing brace until the end of the + // file + RESERVED_WORDS + .keys() + .chain([",,", "[", "]", "{"].iter()) + .any(|word| *word == token.value) + }) + .void(), + one_of(|c: Token| c.value == "}"), + )) + .parse_next(i); let has_closing_brace = maybe_closing_brace.is_ok(); if has_closing_brace { + let start_range = i.as_source_range(); // okay to unwrap here because we checked it was Ok above - let start_range = maybe_closing_brace.unwrap().as_source_range(); + let end_range = maybe_closing_brace.unwrap().1.as_source_range(); + let e = ContextError { context: vec![], cause: Some(CompilationError::fatal( - start_range, + SourceRange::from([start_range.start(), end_range.end(), end_range.module_id().as_usize()]), "Unexpected character encountered. You might be missing a comma in between properties.", )), }; @@ -3996,7 +4030,7 @@ z(-[["#, fn test_parse_weird_lots_of_fancy_brackets() { assert_err( r#"zz({{{{{{{{)iegAng{{{{{{{##"#, - "Object is missing a closing brace(`}`)", + "Encountered an unexpected character(s) before finding a closing brace(`}`) for the object", [3, 4], ); } @@ -4562,7 +4596,11 @@ let myBox = box([0,0], -3, -16, -10) fn test_parse_array_missing_closing_bracket() { let some_program_string = r#" sketch001 = startSketchOn('XZ') |> startProfileAt([90.45, 119.09, %)"#; - assert_err(some_program_string, "Array is missing a closing bracket(`]`)", [51, 52]); + assert_err( + some_program_string, + "Encountered an unexpected character(s) before finding a closing bracket(`]`) for the array", + [51, 67], + ); } #[test] fn test_parse_array_missing_comma() { @@ -4575,23 +4613,50 @@ sketch001 = startSketchOn('XZ') |> startProfileAt([90.45 119.09], %)"#; ); } #[test] + fn test_parse_array_reserved_word_early_exit() { + // since there is an early exit if encountering a reserved word, the error should be about + // that and not the missing comma + let some_program_string = r#" +sketch001 = startSketchOn('XZ') |> startProfileAt([90.45 $sketch], %)"#; + assert_err( + some_program_string, + "Encountered an unexpected character(s) before finding a closing bracket(`]`) for the array", + [51, 52], + ); + } + #[test] fn test_parse_array_random_brace() { let some_program_string = r#" sketch001 = startSketchOn('XZ') |> startProfileAt([}], %)"#; assert_err( some_program_string, - "Unexpected character encountered. You might be missing a comma in between elements.", - [52, 54], + "Encountered an unexpected character(s) before finding a closing bracket(`]`) for the array", + [51, 52], ); } #[test] fn test_parse_object_missing_closing_brace() { let some_program_string = r#"{ - foo = bar, - "#; + foo = bar,"#; + + assert_err( + some_program_string, + "Encountered an unexpected character(s) before finding a closing brace(`}`) for the object", + [0, 23], + ); + } + #[test] + fn test_parse_object_reserved_word_early_exit() { + // since there is an early exit if encountering a reserved word, the error should be about + // that and not the missing comma + let some_program_string = r#"{bar = foo sketch = man}"#; - assert_err(some_program_string, "Object is missing a closing brace(`}`)", [0, 1]); + assert_err( + some_program_string, + "Encountered an unexpected character(s) before finding a closing brace(`}`) for the object", + [0, 1], + ); } #[test] fn test_parse_object_missing_comma() { @@ -4608,14 +4673,25 @@ sketch001 = startSketchOn('XZ') |> startProfileAt([}], %)"#; ); } + #[test] + fn test_parse_object_missing_comma_one_line() { + let some_program_string = r#"{bar = foo bat = man}"#; + + assert_err( + some_program_string, + "Unexpected character encountered. You might be missing a comma in between properties.", + [1, 21], + ); + } + #[test] fn test_parse_object_random_bracket() { let some_program_string = r#"{]}"#; assert_err( some_program_string, - "Unexpected character encountered. You might be missing a comma in between properties.", - [1, 3], + "Encountered an unexpected character(s) before finding a closing brace(`}`) for the object", + [0, 1], ); } diff --git a/src/wasm-lib/kcl/src/parsing/token/mod.rs b/src/wasm-lib/kcl/src/parsing/token/mod.rs index f9cad58f6f..3c26bee067 100644 --- a/src/wasm-lib/kcl/src/parsing/token/mod.rs +++ b/src/wasm-lib/kcl/src/parsing/token/mod.rs @@ -24,7 +24,6 @@ use crate::{ mod tokeniser; -#[cfg(test)] pub(crate) use tokeniser::RESERVED_WORDS; // Note the ordering, it's important that `m` comes after `mm` and `cm`. @@ -194,12 +193,18 @@ impl<'a> TokenSlice<'a> { } pub fn as_source_range(&self) -> SourceRange { - let first_token = self.token(0); - SourceRange::new( - first_token.start, - self.stream.tokens[self.end].end, - first_token.module_id, - ) + let stream_len = self.stream.tokens.len(); + let first_token = if stream_len == self.start { + &self.stream.tokens[self.start - 1] + } else { + self.token(0) + }; + let last_token = if stream_len == self.end { + &self.stream.tokens[stream_len - 1] + } else { + self.token(self.end - self.start) + }; + SourceRange::new(first_token.start, last_token.end, last_token.module_id) } }