fix(engine): correct WithBaseObject and HasBinding semantics for with calls

core/engine/src/bytecompiler/mod.rs

@@ -2659,19 +2659,31 @@ impl<'ctx> ByteCompiler<'ctx> {
                                 unreachable!("with binding cannot be local")
                             }
                         };
+                        // Resolve the binding exactly once. `GetNameAndLocator` fetches
+                        // the callee value and records the resolved locator on the
+                        // binding stack; `ThisForObjectEnvironmentName` then derives the
+                        // `this` value (`WithBaseObject`) from that same locator without
+                        // re-scanning the environment chain. This guarantees `HasBinding`
+                        // (and the binding object's `[[HasProperty]]` trap) runs once, as
+                        // required by the specification.
+                        let this = self.register_allocator.alloc();
                         let value = self.register_allocator.alloc();
                         self.bytecode
-                            .emit_this_for_object_environment_name(value.variable(), index.into());
+                            .emit_get_name_and_locator(value.variable(), index.into());
+                        self.bytecode
+                            .emit_this_for_object_environment_name(this.variable());
+                        self.push_from_register(&this);
                         self.push_from_register(&value);
                         self.register_allocator.dealloc(value);
+                        self.register_allocator.dealloc(this);
                     } else {
                         self.push_from_register(&CallFrame::undefined_register());
+                        self.compile_expr_to_stack(expr);
                     }
                 } else {
                     self.push_from_register(&CallFrame::undefined_register());
+                    self.compile_expr_to_stack(expr);
                 }
-
-                self.compile_expr_to_stack(expr);
             }
             expr => {
                 let value = self.register_allocator.alloc();

core/engine/src/environments/runtime/mod.rs

@@ -1,5 +1,6 @@
 use crate::{
     Context, JsResult, JsString, JsSymbol, JsValue,
+    error::JsNativeError,
     object::{JsObject, PrivateName},
 };
 use boa_ast::scope::{BindingLocator, BindingLocatorScope, Scope};
@@ -543,53 +544,25 @@ impl Context {
         Ok(())
     }
 
-    /// Finds the object environment that contains the binding and returns the `this` value of the object environment.
-    pub(crate) fn this_from_object_environment_binding(
-        &mut self,
+    /// Returns the `this` value (`WithBaseObject`) for an already-resolved binding
+    /// locator, without performing another lookup on the environment chain.
+    ///
+    /// Per the specification, the `this` value of a function call whose callee is a
+    /// reference with an Environment Record base is obtained from that already
+    /// resolved record via `WithBaseObject`. Reusing the resolution avoids
+    /// re-running `HasBinding` (and therefore avoids invoking the binding object's
+    /// `[[HasProperty]]` trap a second time).
+    pub(crate) fn this_from_resolved_object_environment_binding(
+        &self,
         locator: &BindingLocator,
-    ) -> JsResult<Option<JsObject>> {
-        let global = self.vm.frame().realm.environment();
-        if let Some(env) = self.vm.frame().environments.current_declarative_ref(global)
-            && !env.with()
+    ) -> Option<JsObject> {
+        if let BindingLocatorScope::Stack(index) = locator.scope()
+            && let Environment::Object(o) = self.environment_expect(index)
         {
-            return Ok(None);
+            return Some(o.clone());
         }
 
-        let min_index = match locator.scope() {
-            BindingLocatorScope::GlobalObject | BindingLocatorScope::GlobalDeclarative => 0,
-            BindingLocatorScope::Stack(index) => index,
-        };
-        let max_index = self.vm.frame().environments.len() as u32;
-
-        for index in (min_index..max_index).rev() {
-            match self.environment_expect(index) {
-                Environment::Declarative(env) => {
-                    if env.poisoned() {
-                        if let Some(env) = env.kind().as_function()
-                            && env.compile().get_binding(locator.name()).is_some()
-                        {
-                            break;
-                        }
-                    } else if !env.with() {
-                        break;
-                    }
-                }
-                Environment::Object(o) => {
-                    let o = o.clone();
-                    let key = locator.name().clone();
-                    if o.has_property(key.clone(), self)? {
-                        if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object()
-                            && unscopables.get(key.clone(), self)?.to_boolean()
-                        {
-                            continue;
-                        }
-                        return Ok(Some(o));
-                    }
-                }
-            }
-        }
-
-        Ok(None)
+        None
     }
 
     /// Checks if the binding pointed by `locator` is initialized.
@@ -659,7 +632,23 @@ impl Context {
                 Environment::Object(obj) => {
                     let key = locator.name().clone();
                     let obj = obj.clone();
-                    obj.get(key, self).map(Some)
+                    // `GetBindingValue` (9.1.1.2.6): step 2 performs `HasProperty`
+                    // before step 4's `Get`. If the binding is gone (e.g. it was
+                    // deleted by an `@@unscopables` getter during `HasBinding`),
+                    // step 3 returns `undefined` in sloppy mode or throws a
+                    // `ReferenceError` in strict mode.
+                    if obj.has_property(key.clone(), self)? {
+                        obj.get(key, self).map(Some)
+                    } else if self.vm.frame().code_block.strict() {
+                        Err(JsNativeError::reference()
+                            .with_message(format!(
+                                "{} is not defined",
+                                locator.name().to_std_string_escaped()
+                            ))
+                            .into())
+                    } else {
+                        Ok(Some(JsValue::undefined()))
+                    }
                 }
             },
         }

core/engine/src/vm/code_block.rs

@@ -390,6 +390,7 @@ impl CodeBlock {
             | Instruction::StoreUndefined { dst }
             | Instruction::Exception { dst }
             | Instruction::This { dst }
+            | Instruction::ThisForObjectEnvironmentName { dst }
             | Instruction::NewTarget { dst }
             | Instruction::ImportMeta { dst }
             | Instruction::CreateMappedArgumentsObject { dst }
@@ -461,7 +462,6 @@ impl CodeBlock {
                 format!("value:{value}, dst:{dst}")
             }
             Instruction::StoreLiteral { index, dst }
-            | Instruction::ThisForObjectEnvironmentName { index, dst }
             | Instruction::GetFunction { index, dst }
             | Instruction::GetArgument { index, dst } => {
                 format!("index:{index}, dst:{dst}")

core/engine/src/vm/opcode/environment/mod.rs

@@ -94,13 +94,20 @@ pub(crate) struct ThisForObjectEnvironmentName;
 
 impl ThisForObjectEnvironmentName {
     #[inline(always)]
-    pub(super) fn operation(
-        (dst, index): (RegisterOperand, IndexOperand),
-        context: &mut Context,
-    ) -> JsResult<()> {
-        let binding_locator = context.vm.frame().code_block.bindings[usize::from(index)].clone();
+    pub(super) fn operation(dst: RegisterOperand, context: &mut Context) -> JsResult<()> {
+        // The preceding `GetNameAndLocator` opcode already resolved the binding and
+        // pushed the resolved locator onto the binding stack. Reuse that resolution
+        // to derive the `this` value (`WithBaseObject`) instead of scanning the
+        // environment chain again, so `HasBinding` (and the binding object's
+        // `[[HasProperty]]` trap) runs exactly once, as required by the specification.
+        let binding_locator = context
+            .vm
+            .frame_mut()
+            .binding_stack
+            .pop()
+            .js_expect("locator should have been pushed by GetNameAndLocator")?;
         let this = context
-            .this_from_object_environment_binding(&binding_locator)?
+            .this_from_resolved_object_environment_binding(&binding_locator)
             .map_or(JsValue::undefined(), Into::into);
         context.vm.set_register(dst.into(), this);
         Ok(())

core/engine/src/vm/opcode/mod.rs

@@ -1770,11 +1770,9 @@ generate_opcodes! {
 
     /// Pushes `this` value that is related to the object environment of the given binding
     ///
-    /// - Operands:
-    ///   - index: `IndexOperand`
     /// - Registers:
     ///   - Output: dst
-    ThisForObjectEnvironmentName { dst: RegisterOperand, index: IndexOperand },
+    ThisForObjectEnvironmentName { dst: RegisterOperand },
 
     /// Execute the `super()` method.
     ///

core/engine/src/vm/tests.rs

@@ -593,3 +593,90 @@ fn recursion_in_setter_throws_uncatchable_error() {
         ),
     ]);
 }
+
+#[test]
+fn with_object_environment_call_single_lookup_and_this() {
+    run_test_actions([
+        TestAction::run(indoc! {r#"
+            let emptyHasCount = 0;
+            const emptyProxy = new Proxy({}, {
+                has(t, p) {
+                    if (p === "Object") {
+                        emptyHasCount++;
+                    }
+                    return Reflect.has(t, p);
+                }
+            });
+            with (emptyProxy) {
+                Object();
+            }
+
+            let hasCount = 0;
+            let callThis = null;
+            const target = {
+                fn() {
+                    callThis = this;
+                }
+            };
+            const proxy = new Proxy(target, {
+                has(t, p) {
+                    if (p === "fn") {
+                        hasCount++;
+                    }
+                    return Reflect.has(t, p);
+                }
+            });
+            with (proxy) {
+                fn();
+            }
+        "#}),
+        TestAction::assert_eq("emptyHasCount", 1),
+        TestAction::assert_eq("hasCount", 2),
+        TestAction::assert("callThis === proxy"),
+    ]);
+}
+
+#[test]
+fn with_object_environment_binding_deleted_in_unscopables() {
+    run_test_actions([
+        TestAction::run(indoc! {r#"
+            let unscopablesCalled = 0;
+            const env = {
+                binding: 42,
+                get [Symbol.unscopables]() {
+                    unscopablesCalled++;
+                    delete env.binding;
+                    return null;
+                }
+            };
+            let sloppyResult = null;
+            with (env) {
+                sloppyResult = binding;
+            }
+
+            let strictThrew = false;
+            const envStrict = {
+                binding: 42,
+                get [Symbol.unscopables]() {
+                    delete envStrict.binding;
+                    return null;
+                }
+            };
+            with (envStrict) {
+                try {
+                    (function() {
+                        "use strict";
+                        return binding;
+                    })();
+                } catch (e) {
+                    if (e instanceof ReferenceError) {
+                        strictThrew = true;
+                    }
+                }
+            }
+        "#}),
+        TestAction::assert_eq("unscopablesCalled", 1),
+        TestAction::assert("sloppyResult === undefined"),
+        TestAction::assert("strictThrew === true"),
+    ]);
+}