KCL parser: Allow .prop or [index] to follow any expression (#7371)
Previously in a member expression like `foo.x` or `foo[3]`, `foo` had to be an identifier. You could not do something like `f().x` (and if you tried, you got a cryptic error). Rather than make the error better, we should just accept any expression to be the LHS of a member expression (aka its 'object'). This does knock our "parse lots of function calls" from 58 to 55 calls before it stack overflows. But I think it's fine, we'll address this in https://github.com/KittyCAD/modeling-app/pull/6226 when I get back to it. Closes https://github.com/KittyCAD/modeling-app/issues/7273
This commit is contained in:
@ -26,8 +26,8 @@ use crate::{
|
||||
Annotation, ArrayExpression, ArrayRangeExpression, BinaryExpression, BinaryOperator, BinaryPart, BodyItem,
|
||||
BoxNode, CallExpressionKw, CommentStyle, DefaultParamVal, ElseIf, Expr, ExpressionStatement,
|
||||
FunctionExpression, FunctionType, Identifier, IfExpression, ImportItem, ImportSelector, ImportStatement,
|
||||
ItemVisibility, LabeledArg, Literal, LiteralIdentifier, LiteralValue, MemberExpression, MemberObject, Name,
|
||||
Node, NodeList, NonCodeMeta, NonCodeNode, NonCodeValue, ObjectExpression, ObjectProperty, Parameter,
|
||||
ItemVisibility, LabeledArg, Literal, LiteralIdentifier, LiteralValue, MemberExpression, Name, Node,
|
||||
NodeList, NonCodeMeta, NonCodeNode, NonCodeValue, ObjectExpression, ObjectProperty, Parameter,
|
||||
PipeExpression, PipeSubstitution, PrimitiveType, Program, ReturnStatement, Shebang, TagDeclarator, Type,
|
||||
TypeDeclaration, UnaryExpression, UnaryOperator, VariableDeclaration, VariableDeclarator, VariableKind,
|
||||
},
|
||||
@ -1326,10 +1326,7 @@ fn member_expression_subscript(i: &mut TokenSlice) -> ModalResult<(LiteralIdenti
|
||||
|
||||
/// Get a property of an object, or an index of an array, or a member of a collection.
|
||||
/// Can be arbitrarily nested, e.g. `people[i]['adam'].age`.
|
||||
fn member_expression(i: &mut TokenSlice) -> ModalResult<Node<MemberExpression>> {
|
||||
// This is an identifier, followed by a sequence of members (aka properties)
|
||||
// First, the identifier.
|
||||
let id = nameable_identifier.context(expected("the identifier of the object whose property you're trying to access, e.g. in 'shape.size.width', 'shape' is the identifier")).parse_next(i)?;
|
||||
fn build_member_expression(object: Expr, i: &mut TokenSlice) -> ModalResult<Node<MemberExpression>> {
|
||||
// Now a sequence of members.
|
||||
let member = alt((member_expression_dot, member_expression_subscript)).context(expected("a member/property, e.g. size.x and size['height'] and size[0] are all different ways to access a member/property of 'size'"));
|
||||
let mut members: Vec<_> = repeat(1.., member)
|
||||
@ -1340,11 +1337,11 @@ fn member_expression(i: &mut TokenSlice) -> ModalResult<Node<MemberExpression>>
|
||||
// It's safe to call remove(0), because the vec is created from repeat(1..),
|
||||
// which is guaranteed to have >=1 elements.
|
||||
let (property, end, computed) = members.remove(0);
|
||||
let start = id.start;
|
||||
let module_id = id.module_id;
|
||||
let start = object.start();
|
||||
let module_id = object.module_id();
|
||||
let initial_member_expression = Node::new(
|
||||
MemberExpression {
|
||||
object: MemberObject::Identifier(Box::new(id)),
|
||||
object,
|
||||
computed,
|
||||
property,
|
||||
digest: None,
|
||||
@ -1362,7 +1359,7 @@ fn member_expression(i: &mut TokenSlice) -> ModalResult<Node<MemberExpression>>
|
||||
.fold(initial_member_expression, |accumulated, (property, end, computed)| {
|
||||
Node::new(
|
||||
MemberExpression {
|
||||
object: MemberObject::MemberExpression(Box::new(accumulated)),
|
||||
object: Expr::MemberExpression(Box::new(accumulated)),
|
||||
computed,
|
||||
property,
|
||||
digest: None,
|
||||
@ -2085,8 +2082,7 @@ fn unnecessarily_bracketed(i: &mut TokenSlice) -> ModalResult<Expr> {
|
||||
}
|
||||
|
||||
fn expr_allowed_in_pipe_expr(i: &mut TokenSlice) -> ModalResult<Expr> {
|
||||
alt((
|
||||
member_expression.map(Box::new).map(Expr::MemberExpression),
|
||||
let parsed_expr = alt((
|
||||
bool_value.map(Box::new).map(Expr::Literal),
|
||||
tag.map(Box::new).map(Expr::TagDeclarator),
|
||||
literal.map(Expr::Literal),
|
||||
@ -2100,14 +2096,19 @@ fn expr_allowed_in_pipe_expr(i: &mut TokenSlice) -> ModalResult<Expr> {
|
||||
unnecessarily_bracketed,
|
||||
))
|
||||
.context(expected("a KCL expression (but not a pipe expression)"))
|
||||
.parse_next(i)
|
||||
.parse_next(i)?;
|
||||
|
||||
let maybe_member = build_member_expression(parsed_expr.clone(), i);
|
||||
if let Ok(mem) = maybe_member {
|
||||
return Ok(Expr::MemberExpression(Box::new(mem)));
|
||||
}
|
||||
Ok(parsed_expr)
|
||||
}
|
||||
|
||||
fn possible_operands(i: &mut TokenSlice) -> ModalResult<Expr> {
|
||||
let mut expr = alt((
|
||||
unary_expression.map(Box::new).map(Expr::UnaryExpression),
|
||||
bool_value.map(Box::new).map(Expr::Literal),
|
||||
member_expression.map(Box::new).map(Expr::MemberExpression),
|
||||
literal.map(Expr::Literal),
|
||||
fn_call_kw.map(Box::new).map(Expr::CallExpressionKw),
|
||||
name.map(Box::new).map(Expr::Name),
|
||||
@ -2118,6 +2119,10 @@ fn possible_operands(i: &mut TokenSlice) -> ModalResult<Expr> {
|
||||
"a KCL value which can be used as an argument/operand to an operator",
|
||||
))
|
||||
.parse_next(i)?;
|
||||
let maybe_member = build_member_expression(expr.clone(), i);
|
||||
if let Ok(mem) = maybe_member {
|
||||
expr = Expr::MemberExpression(Box::new(mem));
|
||||
}
|
||||
|
||||
let ty = opt((colon, opt(whitespace), type_)).parse_next(i)?;
|
||||
if let Some((_, _, ty)) = ty {
|
||||
@ -3258,7 +3263,7 @@ fn fn_call_kw(i: &mut TokenSlice) -> ModalResult<Node<CallExpressionKw>> {
|
||||
return Err(ErrMode::Cut(
|
||||
CompilationError::fatal(
|
||||
SourceRange::from(&tok),
|
||||
format!("There was an unexpected {}. Try removing it.", tok.value),
|
||||
format!("There was an unexpected `{}`. Try removing it.", tok.value),
|
||||
)
|
||||
.into(),
|
||||
));
|
||||
@ -4427,7 +4432,7 @@ z(-[["#,
|
||||
assert_err(
|
||||
r#"z
|
||||
(--#"#,
|
||||
"There was an unexpected -. Try removing it.",
|
||||
"There was an unexpected `-`. Try removing it.",
|
||||
[3, 4],
|
||||
);
|
||||
}
|
||||
@ -4997,6 +5002,17 @@ type foo = fn(fn, f: fn(number(_))): [fn([any]): string]
|
||||
assert_no_err(some_program_string);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_fn_call_then_field() {
|
||||
let some_program_string = "myFunction().field";
|
||||
let module_id = ModuleId::default();
|
||||
let tokens = crate::parsing::token::lex(some_program_string, module_id).unwrap(); // Updated import path
|
||||
let actual = expression.parse(tokens.as_slice()).unwrap();
|
||||
let Expr::MemberExpression(_expr) = actual else {
|
||||
panic!("expected member expression")
|
||||
};
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_array_missing_closing_bracket() {
|
||||
let some_program_string = r#"
|
||||
@ -5185,7 +5201,7 @@ bar = 1
|
||||
.cause
|
||||
.as_ref()
|
||||
.expect("Found an error, but there was no cause. Add a cause.");
|
||||
assert_eq!(cause.message, "There was an unexpected }. Try removing it.",);
|
||||
assert_eq!(cause.message, "There was an unexpected `}`. Try removing it.",);
|
||||
assert_eq!(cause.source_range.start(), expected_src_start);
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user