Fix panic in Function constructor with nested lexical bindings (#4531)

core/ast/src/function/ordinary_function.rs

@@ -5,7 +5,9 @@ use crate::{
     join_nodes,
     operations::{ContainsSymbol, contains},
     scope::{FunctionScopes, Scope},
-    scope_analyzer::{analyze_binding_escapes, collect_bindings},
+    scope_analyzer::{
+        analyze_binding_escapes, collect_bindings, optimize_scope_indices_function_constructor,
+    },
     visitor::{VisitWith, Visitor, VisitorMut},
 };
 use boa_interner::{Interner, ToIndentedString};
@@ -281,7 +283,9 @@ impl FunctionExpression {
         interner: &Interner,
     ) -> Result<(), &'static str> {
         collect_bindings(self, strict, false, scope, interner)?;
-        analyze_binding_escapes(self, false, scope.clone(), interner)
+        analyze_binding_escapes(self, false, scope.clone(), interner)?;
+        optimize_scope_indices_function_constructor(self, scope);
+        Ok(())
     }
 }
 

core/ast/src/scope_analyzer.rs

@@ -1230,6 +1230,30 @@ where
     let _ = visitor.visit(node.into());
 }
 
+/// Like [`optimize_scope_indices`] but for a [`FunctionExpression`] compiled
+/// by the `Function` constructor with `force_function_scope = true`.
+///
+/// The `Function` constructor always pushes a function scope at runtime, so
+/// the optimizer must account for that even when the function scope would
+/// otherwise be elided.
+pub(crate) fn optimize_scope_indices_function_constructor(
+    node: &mut FunctionExpression,
+    scope: &Scope,
+) {
+    let mut visitor = ScopeIndexVisitor {
+        index: scope.scope_index(),
+    };
+    let _ = visitor.visit_function_like(
+        &mut node.body,
+        &mut node.parameters,
+        &mut node.scopes,
+        &mut node.name_scope,
+        false,
+        // Always force the function scope for the Function constructor.
+        true,
+    );
+}
+
 struct ScopeIndexVisitor {
     index: u32,
 }

core/engine/src/tests/function.rs

@@ -178,3 +178,23 @@ fn eval_out_of_scope() {
         TestAction::assert_eq("f()", JsValue::null()),
     ]);
 }
+
+/// Regression test for issue #4531.
+/// `Function` constructor with nested function containing lexical bindings
+/// captured by a closure should not panic with "must be declarative environment".
+#[test]
+fn function_constructor_nested_lexical_binding() {
+    run_test_actions([TestAction::assert_eq(
+        indoc! {r#"
+            var code = "\
+                function f() {\
+                    const a = 42;\
+                    return () => { return a; };\
+                }\
+                return f()();\
+            ";
+            Function(code)();
+        "#},
+        42,
+    )]);
+}