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"),
+    ]);
+}

tests/insta-bytecode/scripts/with-statement-call.js

@@ -0,0 +1,15 @@
+// Snapshot pins the bytecode for a function call whose callee is resolved
+// through an object Environment Record (a `with` statement). PR #5507 makes
+// the compiler emit `GetNameAndLocator` to resolve the binding/locator once,
+// then `ThisForObjectEnvironmentName dst:rNN` (dst-only; the old `index`
+// operand was removed) to derive the call's `this` (WithBaseObject) from that
+// same resolved locator, so HasBinding/[[HasProperty]] runs exactly once.
+with (
+  {
+    fn() {
+      return this;
+    },
+  }
+) {
+  fn();
+}

tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@with-statement-call.js.snap

@@ -0,0 +1,33 @@
+---
+source: tests/insta-bytecode/src/lib.rs
+expression: output
+input_file: tests/insta-bytecode/scripts/with-statement-call.js
+---
+-------------------------- Compiled Output: '<main>' ---------------------------
+Location     Handler      Opcode                            Operands
+  000000                    StoreEmptyObject                  dst:r01
+  000005                    GetFunction                       index:0, dst:r02
+  00000e                    SetHomeObject                     function:r02, home:r01
+  000017                    DefineOwnPropertyByName           object:r01, value:r02, name_index:1
+  000024                    PushObjectEnvironment             src:r01
+  000029                    GetNameAndLocator                 dst:r03, binding_index:0
+  000032                    ThisForObjectEnvironmentName      dst:r02
+  000037                    PushFromRegister                  src:r02
+  00003c                    PushFromRegister                  src:r03
+  000041                    Call                              argument_count:0
+  000046                    PopIntoRegister                   dst:r01
+  00004b                    SetAccumulator                    src:r01
+  000050                    PopEnvironment                    
+  000051                    CheckReturn                       
+  000052                    Return                            
+
+Register Count: 4, Flags: CodeBlockFlags(HAS_PROTOTYPE_PROPERTY)
+Constants:
+    0000: [FUNCTION] name: 'fn' (length: 0)
+    0001: [STRING] "fn"
+    0002: [SCOPE] index: 1, bindings: 0
+Bindings:
+    0000: fn, scope: GlobalObject
+Handlers: <empty>
+Source Map:
+    0000: 65..75: (14, 5)