Add AST node for parenthesized expressions (#2738)

Currently we have no explicit representation for parenthesized expressions which makes some behaviours impossible to detect. A bonus is that we can now turn AST that contains parenthesized expressions back to code.

This Pull Request changes the following:

- Add an AST node for parenthesized expressions.
- Adjust some conversions and checks to "ignore"/"expand" parenthesized expressions.
- Fix some tests that had parenthesized expressions.

boa_ast/src/expression/mod.rs

@@ -27,6 +27,7 @@ mod call;
 mod identifier;
 mod new;
 mod optional;
+mod parenthesized;
 mod spread;
 mod tagged_template;
 mod r#yield;
@@ -36,6 +37,7 @@ pub use call::{Call, SuperCall};
 pub use identifier::{Identifier, RESERVED_IDENTIFIERS_STRICT};
 pub use new::New;
 pub use optional::{Optional, OptionalOperation, OptionalOperationKind};
+pub use parenthesized::Parenthesized;
 pub use r#await::Await;
 pub use r#yield::Yield;
 pub use spread::Spread;
@@ -154,6 +156,9 @@ pub enum Expression {
     /// See [`Yield`].
     Yield(Yield),
 
+    /// See [`Parenthesized`].
+    Parenthesized(Parenthesized),
+
     /// A FormalParameterList.
     ///
     /// This is only used in the parser itself.
@@ -198,6 +203,7 @@ impl Expression {
             Self::Conditional(cond) => cond.to_interned_string(interner),
             Self::Await(aw) => aw.to_interned_string(interner),
             Self::Yield(yi) => yi.to_interned_string(interner),
+            Self::Parenthesized(expr) => expr.to_interned_string(interner),
             Self::FormalParameterList(_) => unreachable!(),
         }
     }
@@ -240,9 +246,21 @@ impl Expression {
             Self::AsyncGenerator(f) => f.name().is_none(),
             Self::AsyncFunction(f) => f.name().is_none(),
             Self::Class(f) => f.name().is_none(),
+            Self::Parenthesized(p) => p.expression().is_anonymous_function_definition(),
             _ => false,
         }
     }
+
+    /// Returns the expression without any outer parenthesized expressions.
+    #[must_use]
+    #[inline]
+    pub const fn flatten(&self) -> &Self {
+        let mut expression = self;
+        while let Self::Parenthesized(p) = expression {
+            expression = p.expression();
+        }
+        expression
+    }
 }
 
 impl From<Expression> for Statement {
@@ -292,6 +310,7 @@ impl VisitWith for Expression {
             Self::Conditional(c) => visitor.visit_conditional(c),
             Self::Await(a) => visitor.visit_await(a),
             Self::Yield(y) => visitor.visit_yield(y),
+            Self::Parenthesized(e) => visitor.visit_parenthesized(e),
             Self::FormalParameterList(fpl) => visitor.visit_formal_parameter_list(fpl),
             Self::This | Self::NewTarget => {
                 // do nothing; can be handled as special case by visitor
@@ -332,6 +351,7 @@ impl VisitWith for Expression {
             Self::Conditional(c) => visitor.visit_conditional_mut(c),
             Self::Await(a) => visitor.visit_await_mut(a),
             Self::Yield(y) => visitor.visit_yield_mut(y),
+            Self::Parenthesized(e) => visitor.visit_parenthesized_mut(e),
             Self::FormalParameterList(fpl) => visitor.visit_formal_parameter_list_mut(fpl),
             Self::This | Self::NewTarget => {
                 // do nothing; can be handled as special case by visitor

boa_ast/src/expression/operator/assign/mod.rs

@@ -15,7 +15,7 @@ mod op;
 use core::ops::ControlFlow;
 pub use op::*;
 
-use boa_interner::{Interner, ToInternedString};
+use boa_interner::{Interner, Sym, ToInternedString};
 
 use crate::{
     expression::{access::PropertyAccess, identifier::Identifier, Expression},
@@ -127,22 +127,35 @@ impl AssignTarget {
     /// Converts the left-hand-side Expression of an assignment expression into an [`AssignTarget`].
     /// Returns `None` if the given Expression is an invalid left-hand-side for a assignment expression.
     #[must_use]
-    pub fn from_expression(
+    pub fn from_expression(expression: &Expression, strict: bool) -> Option<Self> {
-        expression: &Expression,
-        strict: bool,
-        destructure: bool,
-    ) -> Option<Self> {
         match expression {
-            Expression::Identifier(id) => Some(Self::Identifier(*id)),
+            Expression::ObjectLiteral(object) => {
-            Expression::PropertyAccess(access) => Some(Self::Access(access.clone())),
-            Expression::ObjectLiteral(object) if destructure => {
                 let pattern = object.to_pattern(strict)?;
                 Some(Self::Pattern(pattern.into()))
             }
-            Expression::ArrayLiteral(array) if destructure => {
+            Expression::ArrayLiteral(array) => {
                 let pattern = array.to_pattern(strict)?;
                 Some(Self::Pattern(pattern.into()))
             }
+            e => Self::from_expression_simple(e, strict),
+        }
+    }
+
+    /// Converts the left-hand-side Expression of an assignment expression into an [`AssignTarget`].
+    /// Returns `None` if the given Expression is an invalid left-hand-side for a assignment expression.
+    ///
+    /// The `AssignmentTargetType` of the expression must be `simple`.
+    #[must_use]
+    pub fn from_expression_simple(expression: &Expression, strict: bool) -> Option<Self> {
+        match expression {
+            Expression::Identifier(id)
+                if strict && (id.sym() == Sym::EVAL || id.sym() == Sym::ARGUMENTS) =>
+            {
+                None
+            }
+            Expression::Identifier(id) => Some(Self::Identifier(*id)),
+            Expression::PropertyAccess(access) => Some(Self::Access(access.clone())),
+            Expression::Parenthesized(p) => Self::from_expression_simple(p.expression(), strict),
             _ => None,
         }
     }

boa_ast/src/expression/parenthesized.rs

@@ -0,0 +1,66 @@
+use super::Expression;
+use crate::visitor::{VisitWith, Visitor, VisitorMut};
+use boa_interner::{Interner, ToInternedString};
+use core::ops::ControlFlow;
+
+/// A parenthesized expression.
+///
+/// More information:
+///  - [ECMAScript reference][spec]
+///  - [MDN documentation][mdn]
+///
+/// [spec]: https://tc39.es/ecma262/#sec-grouping-operator
+/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Grouping
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
+#[derive(Clone, Debug, PartialEq)]
+pub struct Parenthesized {
+    expression: Box<Expression>,
+}
+
+impl Parenthesized {
+    /// Creates a parenthesized expression.
+    #[inline]
+    #[must_use]
+    pub fn new(expression: Expression) -> Self {
+        Self {
+            expression: Box::new(expression),
+        }
+    }
+
+    /// Gets the expression of this parenthesized expression.
+    #[inline]
+    #[must_use]
+    pub const fn expression(&self) -> &Expression {
+        &self.expression
+    }
+}
+
+impl From<Parenthesized> for Expression {
+    fn from(p: Parenthesized) -> Self {
+        Self::Parenthesized(p)
+    }
+}
+
+impl ToInternedString for Parenthesized {
+    #[inline]
+    fn to_interned_string(&self, interner: &Interner) -> String {
+        format!("({})", self.expression.to_interned_string(interner))
+    }
+}
+
+impl VisitWith for Parenthesized {
+    fn visit_with<'a, V>(&'a self, visitor: &mut V) -> ControlFlow<V::BreakTy>
+    where
+        V: Visitor<'a>,
+    {
+        visitor.visit_expression(&self.expression)
+    }
+
+    fn visit_with_mut<'a, V>(&'a mut self, visitor: &mut V) -> ControlFlow<V::BreakTy>
+    where
+        V: VisitorMut<'a>,
+    {
+        visitor.visit_expression_mut(&mut self.expression)
+    }
+}

boa_ast/src/visitor.rs

@@ -22,7 +22,7 @@ use crate::{
             Binary, BinaryInPrivate, Conditional, Unary, Update,
         },
         Await, Call, Expression, Identifier, New, Optional, OptionalOperation,
-        OptionalOperationKind, Spread, SuperCall, TaggedTemplate, Yield,
+        OptionalOperationKind, Parenthesized, Spread, SuperCall, TaggedTemplate, Yield,
     },
     function::{
         ArrowFunction, AsyncArrowFunction, AsyncFunction, AsyncGenerator, Class, ClassElement,
@@ -173,6 +173,7 @@ node_ref! {
     Conditional,
     Await,
     Yield,
+    Parenthesized,
     ForLoopInitializer,
     IterableLoopInitializer,
     Case,
@@ -271,6 +272,7 @@ pub trait Visitor<'ast>: Sized {
     define_visit!(visit_conditional, Conditional);
     define_visit!(visit_await, Await);
     define_visit!(visit_yield, Yield);
+    define_visit!(visit_parenthesized, Parenthesized);
     define_visit!(visit_for_loop_initializer, ForLoopInitializer);
     define_visit!(visit_iterable_loop_initializer, IterableLoopInitializer);
     define_visit!(visit_case, Case);
@@ -365,6 +367,7 @@ pub trait Visitor<'ast>: Sized {
             NodeRef::Conditional(n) => self.visit_conditional(n),
             NodeRef::Await(n) => self.visit_await(n),
             NodeRef::Yield(n) => self.visit_yield(n),
+            NodeRef::Parenthesized(n) => self.visit_parenthesized(n),
             NodeRef::ForLoopInitializer(n) => self.visit_for_loop_initializer(n),
             NodeRef::IterableLoopInitializer(n) => self.visit_iterable_loop_initializer(n),
             NodeRef::Case(n) => self.visit_case(n),
@@ -465,6 +468,7 @@ pub trait VisitorMut<'ast>: Sized {
     define_visit_mut!(visit_conditional_mut, Conditional);
     define_visit_mut!(visit_await_mut, Await);
     define_visit_mut!(visit_yield_mut, Yield);
+    define_visit_mut!(visit_parenthesized_mut, Parenthesized);
     define_visit_mut!(visit_for_loop_initializer_mut, ForLoopInitializer);
     define_visit_mut!(visit_iterable_loop_initializer_mut, IterableLoopInitializer);
     define_visit_mut!(visit_case_mut, Case);
@@ -559,6 +563,7 @@ pub trait VisitorMut<'ast>: Sized {
             NodeRefMut::Conditional(n) => self.visit_conditional_mut(n),
             NodeRefMut::Await(n) => self.visit_await_mut(n),
             NodeRefMut::Yield(n) => self.visit_yield_mut(n),
+            NodeRefMut::Parenthesized(n) => self.visit_parenthesized_mut(n),
             NodeRefMut::ForLoopInitializer(n) => self.visit_for_loop_initializer_mut(n),
             NodeRefMut::IterableLoopInitializer(n) => self.visit_iterable_loop_initializer_mut(n),
             NodeRefMut::Case(n) => self.visit_case_mut(n),

boa_engine/src/bytecompiler/expression/mod.rs

@@ -305,6 +305,9 @@ impl ByteCompiler<'_, '_> {
                     self.emit_opcode(Opcode::Pop);
                 }
             }
+            Expression::Parenthesized(parenthesized) => {
+                self.compile_expr(parenthesized.expression(), use_expr);
+            }
             // TODO: try to remove this variant somehow
             Expression::FormalParameterList(_) => unreachable!(),
         }

boa_engine/src/bytecompiler/expression/unary.rs

@@ -25,7 +25,7 @@ impl ByteCompiler<'_, '_> {
             UnaryOp::Not => Some(Opcode::LogicalNot),
             UnaryOp::Tilde => Some(Opcode::BitNot),
             UnaryOp::TypeOf => {
-                match &unary.target() {
+                match unary.target().flatten() {
                     Expression::Identifier(identifier) => {
                         let binding = self.context.get_binding_value(*identifier);
                         let index = self.get_or_insert_binding(binding);

boa_engine/src/bytecompiler/mod.rs

@@ -198,6 +198,7 @@ impl Access<'_> {
             Expression::Identifier(name) => Some(Access::Variable { name: *name }),
             Expression::PropertyAccess(access) => Some(Access::Property { access }),
             Expression::This => Some(Access::This),
+            Expression::Parenthesized(expr) => Self::from_expression(expr.expression()),
             _ => None,
         }
     }
@@ -868,7 +869,7 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
     fn compile_optional_preserve_this(&mut self, optional: &Optional) {
         let mut jumps = Vec::with_capacity(optional.chain().len());
 
-        match optional.target() {
+        match optional.target().flatten() {
             Expression::PropertyAccess(access) => {
                 self.compile_access_preserve_this(access);
             }
@@ -1238,15 +1239,12 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
             New,
         }
 
-        let (call, kind) = match callable {
+        let (call, mut kind) = match callable {
-            Callable::Call(call) => match call.function() {
+            Callable::Call(call) => (call, CallKind::Call),
-                Expression::Identifier(ident) if *ident == Sym::EVAL => (call, CallKind::CallEval),
-                _ => (call, CallKind::Call),
-            },
             Callable::New(new) => (new.call(), CallKind::New),
         };
 
-        match call.function() {
+        match call.function().flatten() {
             Expression::PropertyAccess(access) if kind == CallKind::Call => {
                 self.compile_access_preserve_this(access);
             }
@@ -1254,12 +1252,18 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
             Expression::Optional(opt) if kind == CallKind::Call => {
                 self.compile_optional_preserve_this(opt);
             }
-            expr => {
+            expr if kind == CallKind::Call => {
+                if let Expression::Identifier(ident) = expr {
+                    if *ident == Sym::EVAL {
+                        kind = CallKind::CallEval;
+                    }
+                }
                 self.compile_expr(expr, true);
-                if kind == CallKind::Call || kind == CallKind::CallEval {
                 self.emit_opcode(Opcode::PushUndefined);
                 self.emit_opcode(Opcode::Swap);
             }
+            expr => {
+                self.compile_expr(expr, true);
             }
         }
 

boa_parser/src/parser/expression/assignment/mod.rs

@@ -13,7 +13,6 @@ mod conditional;
 mod exponentiation;
 mod r#yield;
 
-use super::check_strict_arguments_or_eval;
 use crate::{
     lexer::{Error as LexError, InputElement, TokenKind},
     parser::{
@@ -238,17 +237,10 @@ where
         if let Some(tok) = cursor.peek(0, interner)?.cloned() {
             match tok.kind() {
                 TokenKind::Punctuator(Punctuator::Assign) => {
-                    if cursor.strict_mode() {
-                        if let Expression::Identifier(ident) = lhs {
-                            check_strict_arguments_or_eval(ident, position)?;
-                        }
-                    }
-
                     cursor.advance(interner);
                     cursor.set_goal(InputElement::RegExp);
 
-                    if let Some(target) =
+                    if let Some(target) = AssignTarget::from_expression(&lhs, cursor.strict_mode())
-                        AssignTarget::from_expression(&lhs, cursor.strict_mode(), true)
                     {
                         if let AssignTarget::Identifier(ident) = target {
                             self.name = Some(ident);
@@ -263,15 +255,9 @@ where
                     }
                 }
                 TokenKind::Punctuator(p) if p.as_assign_op().is_some() => {
-                    if cursor.strict_mode() {
-                        if let Expression::Identifier(ident) = lhs {
-                            check_strict_arguments_or_eval(ident, position)?;
-                        }
-                    }
-
                     cursor.advance(interner);
                     if let Some(target) =
-                        AssignTarget::from_expression(&lhs, cursor.strict_mode(), false)
+                        AssignTarget::from_expression_simple(&lhs, cursor.strict_mode())
                     {
                         let assignop = p.as_assign_op().expect("assignop disappeared");
                         if assignop == AssignOp::BoolAnd

boa_parser/src/parser/expression/primary/mod.rs

@@ -44,7 +44,7 @@ use boa_ast::{
     expression::{
         literal::Literal,
         operator::{assign::AssignTarget, binary::BinaryOp},
-        Call, Identifier, New,
+        Call, Identifier, New, Parenthesized,
     },
     function::{FormalParameter, FormalParameterList},
     operations::{contains, ContainsSymbol},
@@ -493,7 +493,9 @@ where
                 ));
             }
             if let InnerExpression::Expression(expression) = &expressions[0] {
-                return Ok(expression.clone());
+                return Ok(ast::Expression::Parenthesized(Parenthesized::new(
+                    expression.clone(),
+                )));
             }
             return Err(Error::unexpected(
                 Punctuator::CloseParen,

boa_parser/src/parser/expression/tests.rs

@@ -8,7 +8,7 @@ use boa_ast::{
             binary::{ArithmeticOp, BitwiseOp, LogicalOp, RelationalOp},
             Assign, Binary,
         },
-        Call, Identifier, New,
+        Call, Identifier, New, Parenthesized,
     },
     Declaration, Expression, Statement,
 };
@@ -257,6 +257,7 @@ fn check_complex_numeric_operations() {
                 Binary::new(
                     ArithmeticOp::Mul.into(),
                     Identifier::new(interner.get_or_intern_static("d", utf16!("d"))).into(),
+                    Parenthesized::new(
                         Binary::new(
                             ArithmeticOp::Sub.into(),
                             Identifier::new(interner.get_or_intern_static("b", utf16!("b"))).into(),
@@ -267,6 +268,8 @@ fn check_complex_numeric_operations() {
                     .into(),
                 )
                 .into(),
+            )
+            .into(),
             Literal::from(1).into(),
         )))
         .into()],
@@ -695,9 +698,13 @@ macro_rules! check_non_reserved_identifier {
         let interner = &mut Interner::default();
         check_script_parser(
             format!("({})", $keyword).as_str(),
-            vec![Statement::Expression(Expression::from(Identifier::new(
+            vec![Statement::Expression(
-                interner.get_or_intern_static($keyword, utf16!($keyword)),
+                Parenthesized::new(
-            )))
+                    Identifier::new(interner.get_or_intern_static($keyword, utf16!($keyword)))
+                        .into(),
+                )
+                .into(),
+            )
             .into()],
             interner,
         );

boa_parser/src/parser/expression/unary.rs

@@ -76,9 +76,9 @@ where
             TokenKind::Keyword((Keyword::Delete, false)) => {
                 cursor.advance(interner);
                 let position = cursor.peek(0, interner).or_abrupt()?.span().start();
-                let val = self.parse(cursor, interner)?;
+                let target = self.parse(cursor, interner)?;
 
-                match val {
+                match target.flatten() {
                     Expression::Identifier(_) if cursor.strict_mode() => {
                         return Err(Error::lex(LexError::Syntax(
                             "cannot delete variables in strict mode".into(),
@@ -94,7 +94,7 @@ where
                     _ => {}
                 }
 
-                Ok(Unary::new(UnaryOp::Delete, val).into())
+                Ok(Unary::new(UnaryOp::Delete, target).into())
             }
             TokenKind::Keyword((Keyword::Void, false)) => {
                 cursor.advance(interner);

boa_parser/src/parser/expression/update.rs

@@ -80,6 +80,7 @@ fn as_simple(
         Expression::PropertyAccess(access) => {
             Ok(Some(UpdateTarget::PropertyAccess(access.clone())))
         }
+        Expression::Parenthesized(p) => as_simple(p.expression(), position, strict),
         _ => Ok(None),
     }
 }

boa_parser/src/parser/statement/iteration/for_statement.rs

@@ -290,7 +290,11 @@ fn initializer_to_iterable_loop_initializer(
     strict: bool,
 ) -> ParseResult<IterableLoopInitializer> {
     match initializer {
-        ForLoopInitializer::Expression(expr) => match expr {
+        ForLoopInitializer::Expression(mut expr) => {
+            while let ast::Expression::Parenthesized(p) = expr {
+                expr = p.expression().clone();
+            }
+            match expr {
                 ast::Expression::Identifier(ident)
                     if strict && [Sym::EVAL, Sym::ARGUMENTS].contains(&ident.sym()) =>
                 {
@@ -300,7 +304,9 @@ fn initializer_to_iterable_loop_initializer(
                         position,
                     )))
                 }
-            ast::Expression::Identifier(ident) => Ok(IterableLoopInitializer::Identifier(ident)),
+                ast::Expression::Identifier(ident) => {
+                    Ok(IterableLoopInitializer::Identifier(ident))
+                }
                 ast::Expression::ArrayLiteral(array) => array
                     .to_pattern(strict)
                     .ok_or_else(|| {
@@ -319,12 +325,15 @@ fn initializer_to_iterable_loop_initializer(
                         )
                     })
                     .map(|obj| IterableLoopInitializer::Pattern(obj.into())),
-            ast::Expression::PropertyAccess(access) => Ok(IterableLoopInitializer::Access(access)),
+                ast::Expression::PropertyAccess(access) => {
+                    Ok(IterableLoopInitializer::Access(access))
+                }
                 _ => Err(Error::lex(LexError::Syntax(
                     "invalid variable for iterable loop".into(),
                     position,
                 ))),
-        },
+            }
+        }
         ForLoopInitializer::Lexical(decl) => match decl.variable_list().as_ref() {
             [declaration] => {
                 if declaration.init().is_some() {

boa_parser/src/parser/tests/mod.rs

@@ -16,7 +16,7 @@ use boa_ast::{
             update::{UpdateOp, UpdateTarget},
             Assign, Binary, Update,
         },
-        Call, Identifier, New,
+        Call, Identifier, New, Parenthesized,
     },
     function::{
         ArrowFunction, FormalParameter, FormalParameterList, FormalParameterListFlags, Function,
@@ -411,9 +411,12 @@ fn bracketed_expr() {
     let interner = &mut Interner::default();
     check_script_parser(
         s,
-        vec![Statement::Expression(Expression::from(Identifier::new(
+        vec![Statement::Expression(
-            interner.get_or_intern_static("b", utf16!("b")),
+            Parenthesized::new(
-        )))
+                Identifier::new(interner.get_or_intern_static("b", utf16!("b"))).into(),
+            )
+            .into(),
+        )
         .into()],
         interner,
     );
@@ -427,7 +430,9 @@ fn increment_in_comma_op() {
     let b = interner.get_or_intern_static("b", utf16!("b"));
     check_script_parser(
         s,
-        vec![Statement::Expression(Expression::from(Binary::new(
+        vec![Statement::Expression(
+            Parenthesized::new(
+                Binary::new(
                     BinaryOp::Comma,
                     Update::new(
                         UpdateOp::IncrementPost,
@@ -435,7 +440,11 @@ fn increment_in_comma_op() {
                     )
                     .into(),
                     Identifier::new(b).into(),
-        )))
+                )
+                .into(),
+            )
+            .into(),
+        )
         .into()],
         interner,
     );