Implement binary `in` operation with private names (#2582)

This Pull Request changes the following:

- Implement binary `in` operation with private names.
- Adding a separate `BinaryInPrivate` expression in addition to the existing `Binary` expression seems like the best way to implement this in a typesafe manner. Other methods like adding an enum for the `Binary` lhs result in having to make assertions.

boa_ast/src/expression/mod.rs

@@ -9,20 +9,18 @@
 //! [primary]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#primary_expressions
 //! [lhs]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#left-hand-side_expressions
 
-use boa_interner::{Interner, ToIndentedString, ToInternedString};
-use core::ops::ControlFlow;
-
 use self::{
     access::PropertyAccess,
     literal::{ArrayLiteral, Literal, ObjectLiteral, TemplateLiteral},
-    operator::{Assign, Binary, Conditional, Unary, Update},
+    operator::{Assign, Binary, BinaryInPrivate, Conditional, Unary, Update},
 };
-
 use super::{
     function::{ArrowFunction, AsyncFunction, AsyncGenerator, Class, Function, Generator},
     function::{AsyncArrowFunction, FormalParameterList},
     Statement,
 };
+use boa_interner::{Interner, ToIndentedString, ToInternedString};
+use core::ops::ControlFlow;
 
 mod r#await;
 mod call;
@@ -144,6 +142,9 @@ pub enum Expression {
     /// See [`Binary`].
     Binary(Binary),
 
+    /// See [`BinaryInPrivate`].
+    BinaryInPrivate(BinaryInPrivate),
+
     /// See [`Conditional`].
     Conditional(Conditional),
 
@@ -193,6 +194,7 @@ impl Expression {
             Self::Unary(unary) => unary.to_interned_string(interner),
             Self::Update(update) => update.to_interned_string(interner),
             Self::Binary(bin) => bin.to_interned_string(interner),
+            Self::BinaryInPrivate(bin) => bin.to_interned_string(interner),
             Self::Conditional(cond) => cond.to_interned_string(interner),
             Self::Await(aw) => aw.to_interned_string(interner),
             Self::Yield(yi) => yi.to_interned_string(interner),
@@ -286,6 +288,7 @@ impl VisitWith for Expression {
             Self::Unary(u) => visitor.visit_unary(u),
             Self::Update(u) => visitor.visit_update(u),
             Self::Binary(b) => visitor.visit_binary(b),
+            Self::BinaryInPrivate(b) => visitor.visit_binary_in_private(b),
             Self::Conditional(c) => visitor.visit_conditional(c),
             Self::Await(a) => visitor.visit_await(a),
             Self::Yield(y) => visitor.visit_yield(y),
@@ -325,6 +328,7 @@ impl VisitWith for Expression {
             Self::Unary(u) => visitor.visit_unary_mut(u),
             Self::Update(u) => visitor.visit_update_mut(u),
             Self::Binary(b) => visitor.visit_binary_mut(b),
+            Self::BinaryInPrivate(b) => visitor.visit_binary_in_private_mut(b),
             Self::Conditional(c) => visitor.visit_conditional_mut(c),
             Self::Await(a) => visitor.visit_await_mut(a),
             Self::Yield(y) => visitor.visit_yield_mut(y),

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

@@ -16,16 +16,16 @@
 
 mod op;
 
-use core::ops::ControlFlow;
-pub use op::*;
-
-use boa_interner::{Interner, ToInternedString};
-
 use crate::{
     expression::Expression,
+    function::PrivateName,
     try_break,
     visitor::{VisitWith, Visitor, VisitorMut},
 };
+use boa_interner::{Interner, ToInternedString};
+use core::ops::ControlFlow;
+
+pub use op::*;
 
 /// Binary operations require two operands, one before the operator and one after the operator.
 ///
@@ -109,3 +109,78 @@ impl VisitWith for Binary {
         visitor.visit_expression_mut(&mut self.rhs)
     }
 }
+
+/// Binary [relational][relat] `In` expression with a private name on the left hand side.
+///
+/// Because the left hand side must be a private name, this is a separate type from [`Binary`].
+///
+/// [relat]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#relational_operators
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
+#[derive(Clone, Debug, PartialEq)]
+pub struct BinaryInPrivate {
+    lhs: PrivateName,
+    rhs: Box<Expression>,
+}
+
+impl BinaryInPrivate {
+    /// Creates a `BinaryInPrivate` AST Expression.
+    #[inline]
+    #[must_use]
+    pub fn new(lhs: PrivateName, rhs: Expression) -> Self {
+        Self {
+            lhs,
+            rhs: Box::new(rhs),
+        }
+    }
+
+    /// Gets the left hand side of the binary operation.
+    #[inline]
+    #[must_use]
+    pub const fn lhs(&self) -> &PrivateName {
+        &self.lhs
+    }
+
+    /// Gets the right hand side of the binary operation.
+    #[inline]
+    #[must_use]
+    pub const fn rhs(&self) -> &Expression {
+        &self.rhs
+    }
+}
+
+impl ToInternedString for BinaryInPrivate {
+    #[inline]
+    fn to_interned_string(&self, interner: &Interner) -> String {
+        format!(
+            "#{} in {}",
+            interner.resolve_expect(self.lhs.description()),
+            self.rhs.to_interned_string(interner)
+        )
+    }
+}
+
+impl From<BinaryInPrivate> for Expression {
+    #[inline]
+    fn from(op: BinaryInPrivate) -> Self {
+        Self::BinaryInPrivate(op)
+    }
+}
+
+impl VisitWith for BinaryInPrivate {
+    fn visit_with<'a, V>(&'a self, visitor: &mut V) -> ControlFlow<V::BreakTy>
+    where
+        V: Visitor<'a>,
+    {
+        try_break!(visitor.visit_private_name(&self.lhs));
+        visitor.visit_expression(&self.rhs)
+    }
+
+    fn visit_with_mut<'a, V>(&'a mut self, visitor: &mut V) -> ControlFlow<V::BreakTy>
+    where
+        V: VisitorMut<'a>,
+    {
+        try_break!(visitor.visit_private_name_mut(&mut self.lhs));
+        visitor.visit_expression_mut(&mut self.rhs)
+    }
+}

boa_ast/src/expression/operator/mod.rs

@@ -20,5 +20,9 @@ pub mod unary;
 pub mod update;
 
 pub use self::{
-    assign::Assign, binary::Binary, conditional::Conditional, unary::Unary, update::Update,
+    assign::Assign,
+    binary::{Binary, BinaryInPrivate},
+    conditional::Conditional,
+    unary::Unary,
+    update::Update,
 };

boa_ast/src/visitor.rs

@@ -17,7 +17,7 @@ use crate::{
         literal::{ArrayLiteral, Literal, ObjectLiteral, TemplateElement, TemplateLiteral},
         operator::{
             assign::{Assign, AssignTarget},
-            Binary, Conditional, Unary, Update,
+            Binary, BinaryInPrivate, Conditional, Unary, Update,
         },
         Await, Call, Expression, Identifier, New, Optional, OptionalOperation,
         OptionalOperationKind, Spread, SuperCall, TaggedTemplate, Yield,
@@ -167,6 +167,7 @@ node_ref! {
     Unary,
     Update,
     Binary,
+    BinaryInPrivate,
     Conditional,
     Await,
     Yield,
@@ -254,6 +255,7 @@ pub trait Visitor<'ast>: Sized {
     define_visit!(visit_unary, Unary);
     define_visit!(visit_update, Update);
     define_visit!(visit_binary, Binary);
+    define_visit!(visit_binary_in_private, BinaryInPrivate);
     define_visit!(visit_conditional, Conditional);
     define_visit!(visit_await, Await);
     define_visit!(visit_yield, Yield);
@@ -338,6 +340,7 @@ pub trait Visitor<'ast>: Sized {
             NodeRef::Unary(n) => self.visit_unary(n),
             NodeRef::Update(n) => self.visit_update(n),
             NodeRef::Binary(n) => self.visit_binary(n),
+            NodeRef::BinaryInPrivate(n) => self.visit_binary_in_private(n),
             NodeRef::Conditional(n) => self.visit_conditional(n),
             NodeRef::Await(n) => self.visit_await(n),
             NodeRef::Yield(n) => self.visit_yield(n),
@@ -427,6 +430,7 @@ pub trait VisitorMut<'ast>: Sized {
     define_visit_mut!(visit_unary_mut, Unary);
     define_visit_mut!(visit_update_mut, Update);
     define_visit_mut!(visit_binary_mut, Binary);
+    define_visit_mut!(visit_binary_in_private_mut, BinaryInPrivate);
     define_visit_mut!(visit_conditional_mut, Conditional);
     define_visit_mut!(visit_await_mut, Await);
     define_visit_mut!(visit_yield_mut, Yield);
@@ -511,6 +515,7 @@ pub trait VisitorMut<'ast>: Sized {
             NodeRefMut::Unary(n) => self.visit_unary_mut(n),
             NodeRefMut::Update(n) => self.visit_update_mut(n),
             NodeRefMut::Binary(n) => self.visit_binary_mut(n),
+            NodeRefMut::BinaryInPrivate(n) => self.visit_binary_in_private_mut(n),
             NodeRefMut::Conditional(n) => self.visit_conditional_mut(n),
             NodeRefMut::Await(n) => self.visit_await_mut(n),
             NodeRefMut::Yield(n) => self.visit_yield_mut(n),

boa_engine/src/bytecompiler/expression/binary.rs

@@ -1,6 +1,6 @@
 use boa_ast::expression::operator::{
     binary::{ArithmeticOp, BinaryOp, BitwiseOp, LogicalOp, RelationalOp},
-    Binary,
+    Binary, BinaryInPrivate,
 };
 
 use crate::{bytecompiler::ByteCompiler, vm::Opcode, JsResult};
@@ -95,4 +95,20 @@ impl ByteCompiler<'_, '_> {
 
         Ok(())
     }
+
+    pub(crate) fn compile_binary_in_private(
+        &mut self,
+        binary: &BinaryInPrivate,
+        use_expr: bool,
+    ) -> JsResult<()> {
+        let index = self.get_or_insert_private_name(*binary.lhs());
+        self.compile_expr(binary.rhs(), true)?;
+        self.emit(Opcode::InPrivate, &[index]);
+
+        if !use_expr {
+            self.emit_opcode(Opcode::Pop);
+        }
+
+        Ok(())
+    }
 }

boa_engine/src/bytecompiler/expression/mod.rs

@@ -91,6 +91,9 @@ impl ByteCompiler<'_, '_> {
             Expression::Unary(unary) => self.compile_unary(unary, use_expr)?,
             Expression::Update(update) => self.compile_update(update, use_expr)?,
             Expression::Binary(binary) => self.compile_binary(binary, use_expr)?,
+            Expression::BinaryInPrivate(binary) => {
+                self.compile_binary_in_private(binary, use_expr)?;
+            }
             Expression::Assign(assign) => self.compile_assign(assign, use_expr)?,
             Expression::ObjectLiteral(object) => {
                 self.compile_object_literal(object, use_expr)?;

boa_engine/src/vm/code_block.rs

@@ -323,7 +323,8 @@ impl CodeBlock {
             | Opcode::PushClassFieldPrivate
             | Opcode::PushClassPrivateGetter
             | Opcode::PushClassPrivateSetter
-            | Opcode::PushClassPrivateMethod => {
+            | Opcode::PushClassPrivateMethod
+            | Opcode::InPrivate => {
                 let operand = self.read::<u32>(*pc);
                 *pc += size_of::<u32>();
                 format!(

boa_engine/src/vm/flowgraph/mod.rs

@@ -358,7 +358,8 @@ impl CodeBlock {
                 | Opcode::PushClassFieldPrivate
                 | Opcode::PushClassPrivateGetter
                 | Opcode::PushClassPrivateSetter
-                | Opcode::PushClassPrivateMethod => {
+                | Opcode::PushClassPrivateMethod
+                | Opcode::InPrivate => {
                     let operand = self.read::<u32>(pc);
                     pc += size_of::<u32>();
                     let label = format!(

boa_engine/src/vm/opcode/binary_ops/mod.rs

@@ -98,6 +98,40 @@ impl Operation for In {
     }
 }
 
+/// `InPrivate` implements the Opcode Operation for `Opcode::InPrivate`
+///
+/// Operation:
+///  - Binary `in` operation for private names.
+#[derive(Debug, Clone, Copy)]
+pub(crate) struct InPrivate;
+
+impl Operation for InPrivate {
+    const NAME: &'static str = "InPrivate";
+    const INSTRUCTION: &'static str = "INST - InPrivate";
+
+    fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
+        let index = context.vm.read::<u32>();
+        let name = context.vm.frame().code_block.private_names[index as usize];
+        let rhs = context.vm.pop();
+
+        let Some(rhs) = rhs.as_object() else {
+            return Err(JsNativeError::typ()
+                .with_message(format!(
+                    "right-hand side of 'in' should be an object, got `{}`",
+                    rhs.type_of()
+                ))
+                .into());
+        };
+        if rhs.private_element_find(&name, true, true).is_some() {
+            context.vm.push(true);
+        } else {
+            context.vm.push(false);
+        }
+
+        Ok(ShouldExit::False)
+    }
+}
+
 /// `InstanceOf` implements the Opcode Operation for `Opcode::InstanceOf`
 ///
 /// Operation:

boa_engine/src/vm/opcode/mod.rs

@@ -486,6 +486,13 @@ generate_impl! {
         /// Stack: lhs, rhs **=>** (lhs `in` rhs)
         In,
 
+        /// Binary `in` operator for private names.
+        ///
+        /// Operands: private_name_index: `u32`
+        ///
+        /// Stack: rhs **=>** (private_name `in` rhs)
+        InPrivate,
+
         /// Binary `==` operator.
         ///
         /// Operands:

boa_parser/src/parser/expression/mod.rs

@@ -32,10 +32,11 @@ use boa_ast::{
     expression::{
         operator::{
             binary::{BinaryOp, LogicalOp},
-            Binary,
+            Binary, BinaryInPrivate,
         },
         Identifier,
     },
+    function::PrivateName,
     Keyword, Position, Punctuator,
 };
 use boa_interner::{Interner, Sym};
@@ -564,8 +565,44 @@ where
     fn parse(self, cursor: &mut Cursor<R>, interner: &mut Interner) -> ParseResult<Self::Output> {
         let _timer = Profiler::global().start_event("Relation Expression", "Parsing");
 
+        if self.allow_in.0 {
+            let token = cursor.peek(0, interner).or_abrupt()?;
+            let span = token.span();
+            if let TokenKind::PrivateIdentifier(identifier) = token.kind() {
+                let identifier = *identifier;
+                let token = cursor.peek(1, interner).or_abrupt()?;
+                match token.kind() {
+                    TokenKind::Keyword((Keyword::In, true)) => {
+                        return Err(Error::general(
+                            "Keyword must not contain escaped characters",
+                            token.span().start(),
+                        ));
+                    }
+                    TokenKind::Keyword((Keyword::In, false)) => {
+                        cursor.advance(interner);
+                        cursor.advance(interner);
+
+                        if !cursor.in_class() {
+                            return Err(Error::general(
+                                "Private identifier outside of class",
+                                span.start(),
+                            ));
+                        }
+
+                        let rhs =
+                            ShiftExpression::new(self.name, self.allow_yield, self.allow_await)
+                                .parse(cursor, interner)?;
+
+                        return Ok(BinaryInPrivate::new(PrivateName::new(identifier), rhs).into());
+                    }
+                    _ => {}
+                }
+            }
+        }
+
         let mut lhs = ShiftExpression::new(self.name, self.allow_yield, self.allow_await)
             .parse(cursor, interner)?;
+
         while let Some(tok) = cursor.peek(0, interner)? {
             match *tok.kind() {
                 TokenKind::Punctuator(op)