From b078fffc44e67b3e3ebf9734a271a4f33a2986df Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 13 Jan 2022 15:57:53 +0100 Subject: [PATCH] Parser: allow `.` to access member after any expression Before we would only allow `foo.bar` where `foo` was an identifier. Now we also allow more complex expression such as `"foo".bar` or `(123 + foo).bar` (in the parser) In particular, this will allow to get the member of objects returned by functions or, in the future, part of arrays --- sixtyfps_compiler/parser.rs | 5 ++++- sixtyfps_compiler/parser/expressions.rs | 27 +++++++++++++++++++----- sixtyfps_compiler/passes/resolving.rs | 28 ++++++++++++++++++------- tests/cases/types/object.60 | 8 +++++++ tests/cases/types/string_to_float.60 | 2 +- 5 files changed, 56 insertions(+), 14 deletions(-) diff --git a/sixtyfps_compiler/parser.rs b/sixtyfps_compiler/parser.rs index 7bd90b4cc..0339f4443 100644 --- a/sixtyfps_compiler/parser.rs +++ b/sixtyfps_compiler/parser.rs @@ -359,7 +359,8 @@ declare_syntax! { // FIXME: the test should test that as alternative rather than several of them (but it can also be a literal) Expression-> [ ?Expression, ?FunctionCallExpression, ?SelfAssignment, ?ConditionalExpression, ?QualifiedName, ?BinaryExpression, ?Array, ?ObjectLiteral, - ?UnaryOpExpression, ?CodeBlock, ?StringTemplate, ?AtImageUrl, ?AtLinearGradient], + ?UnaryOpExpression, ?CodeBlock, ?StringTemplate, ?AtImageUrl, ?AtLinearGradient, + ?MemberAccess ], /// Concatenate the Expressions to make a string (usually expended from a template string) StringTemplate -> [*Expression], /// `@image-url("foo.png")` @@ -376,6 +377,8 @@ declare_syntax! { BinaryExpression -> [2 Expression], /// `- expr` UnaryOpExpression -> [Expression], + /// `(foo).bar`, where `foo` is the base expression, and `bar` is a Identifier. + MemberAccess -> [Expression], /// `[ ... ]` Array -> [ *Expression ], /// `{ foo: bar }` diff --git a/sixtyfps_compiler/parser/expressions.rs b/sixtyfps_compiler/parser/expressions.rs index d561f1c14..efd3d116c 100644 --- a/sixtyfps_compiler/parser/expressions.rs +++ b/sixtyfps_compiler/parser/expressions.rs @@ -13,6 +13,7 @@ use super::prelude::*; /// 42px /// #aabbcc /// (something) +/// (something).something /// @image-url("something") /// @image_url("something") /// some_id.some_property @@ -27,6 +28,7 @@ use super::prelude::*; /// aa == cc && bb && (xxx || fff) && 3 + aaa == bbb /// [array] /// {object:42} +/// "foo".bar.something().something.xx({a: 1.foo}.a) /// ``` pub fn parse_expression(p: &mut impl Parser) -> bool { parse_expression_helper(p, OperatorPrecedence::Default) @@ -85,12 +87,27 @@ fn parse_expression_helper(p: &mut impl Parser, precedence: OperatorPrecedence) } } - if p.nth(0).kind() == SyntaxKind::LParent { - { - let _ = p.start_node_at(checkpoint.clone(), SyntaxKind::Expression); + loop { + match p.nth(0).kind() { + SyntaxKind::Dot => { + { + let _ = p.start_node_at(checkpoint.clone(), SyntaxKind::Expression); + } + let mut p = p.start_node_at(checkpoint.clone(), SyntaxKind::MemberAccess); + p.consume(); // '.' + if !p.expect(SyntaxKind::Identifier) { + return false; + } + } + SyntaxKind::LParent => { + { + let _ = p.start_node_at(checkpoint.clone(), SyntaxKind::Expression); + } + let mut p = p.start_node_at(checkpoint.clone(), SyntaxKind::FunctionCallExpression); + parse_function_arguments(&mut *p); + } + _ => break, } - let mut p = p.start_node_at(checkpoint.clone(), SyntaxKind::FunctionCallExpression); - parse_function_arguments(&mut *p); } if precedence >= OperatorPrecedence::Mul { diff --git a/sixtyfps_compiler/passes/resolving.rs b/sixtyfps_compiler/passes/resolving.rs index 8005f27ca..6e83506a8 100644 --- a/sixtyfps_compiler/passes/resolving.rs +++ b/sixtyfps_compiler/passes/resolving.rs @@ -287,6 +287,7 @@ impl Expression { .or_else(|| { node.FunctionCallExpression().map(|n| Self::from_function_call_node(n, ctx)) }) + .or_else(|| node.MemberAccess().map(|n| Self::from_member_access_node(n, ctx))) .or_else(|| node.SelfAssignment().map(|n| Self::from_self_assignment_node(n, ctx))) .or_else(|| node.BinaryExpression().map(|n| Self::from_binary_expression_node(n, ctx))) .or_else(|| { @@ -591,22 +592,27 @@ impl Expression { let mut sub_expr = node.Expression(); - let function = sub_expr - .next() - .and_then(|n| { - n.QualifiedName().or_else(|| { + let function = sub_expr.next().map_or(Self::Invalid, |n| { + // Treat the QualifiedName separately so we can catch the uses of uncalled signal + n.QualifiedName() + .or_else(|| { n.Expression().and_then(|mut e| { while let Some(e2) = e.Expression() { e = e2; } e.QualifiedName().map(|q| { - ctx.diag.push_warning("Parentheses around callable are deprecated. Remove the parentheses".into(), &n); + ctx.diag.push_warning( + "Parentheses around callable are deprecated. Remove the parentheses" + .into(), + &n, + ); q }) }) }) - }) - .map_or(Expression::Invalid, |n| Self::from_qualified_name_node(n, ctx)); + .map(|qn| Self::from_qualified_name_node(qn, ctx)) + .unwrap_or_else(|| Self::from_expression_node(n, ctx)) + }); let sub_expr = sub_expr.map(|n| { (Self::from_expression_node(n.clone(), ctx), Some(NodeOrToken::from((*n).clone()))) @@ -658,6 +664,14 @@ impl Expression { } } + fn from_member_access_node( + node: syntax_nodes::MemberAccess, + ctx: &mut LookupCtx, + ) -> Expression { + let base = Self::from_expression_node(node.Expression(), ctx); + maybe_lookup_object(base, node.child_token(SyntaxKind::Identifier).into_iter(), ctx) + } + fn from_self_assignment_node( node: syntax_nodes::SelfAssignment, ctx: &mut LookupCtx, diff --git a/tests/cases/types/object.60 b/tests/cases/types/object.60 index 807d797e8..83d77ad25 100644 --- a/tests/cases/types/object.60 +++ b/tests/cases/types/object.60 @@ -17,6 +17,11 @@ TestCase := Rectangle { foo.a = obj_conversion2.a; foo.b += 8 + obj_conversion2.b; } + + callback return_object() -> { aa: { bb: int } }; + return_object => { return { aa: { bb: { cc: 42 }.cc } }; } + property test: return_object().aa.bb == 42 && obj_binop_merge; + } @@ -30,6 +35,7 @@ assert_eq!(instance.get_foo_a(), sixtyfps::SharedString::from("hello")); assert_eq!(instance.get_foo_b(), 20); assert_eq!(instance.get_obj_cond_merge_b(), 0); assert!(instance.get_obj_binop_merge()); +assert!(instance.get_test()); // This API to set with a tuple should maybe not be accessible? instance.set_foo(("yo".into(), 33)); @@ -48,6 +54,7 @@ assert_eq(instance.get_foo_a(), sixtyfps::SharedString("hello")); assert_eq(instance.get_foo_b(), 20); assert_eq(instance.get_obj_cond_merge_b(), 0); assert_eq(instance.get_obj_binop_merge(), true); +assert(instance.get_test()); // This API to set with a tuple should maybe not be accessible? instance.set_foo(std::make_tuple(sixtyfps::SharedString("yo"), 33)); @@ -65,6 +72,7 @@ assert.equal(instance.foo_a, "hello"); assert.equal(instance.foo_b, 20); assert.equal(instance.obj_cond_merge_b, 0); assert(instance.obj_binop_merge); +assert(instance.test); instance.foo = { a: "yo", b: 33 }; assert.equal(instance.foo_a, "yo"); diff --git a/tests/cases/types/string_to_float.60 b/tests/cases/types/string_to_float.60 index 1238e27c8..d147f2ea0 100644 --- a/tests/cases/types/string_to_float.60 +++ b/tests/cases/types/string_to_float.60 @@ -12,7 +12,7 @@ TestCase := Rectangle { property test_is_float: !hello.is_float() && number.is_float() && !invalid.is_float() && negative.is_float(); - property test: test_is_float && 42.56001 - number_as_float < 0.001; + property test: test_is_float && 42.56001 - number_as_float < 0.001 && "123".to-float() == 123; }