LibJS: Correct behaviour of direct vs. indirect eval

eval only has direct access to the local scope when accessed through
the name eval. This includes locals named eval, because of course it
does.

9 files changed, 119 insertions, 20 deletions

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index b0014cca06..f85376baa3 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -220,6 +220,11 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj
         }
     }
 
+    if (!is<NewExpression>(*this) && is<Identifier>(*m_callee) && static_cast<Identifier const&>(*m_callee).string() == vm.names.eval.as_string() && &callee.as_function() == global_object.eval_function()) {
+        auto script_value = arguments.size() == 0 ? js_undefined() : arguments[0];
+        return perform_eval(script_value, global_object, vm.in_strict_mode() ? CallerMode::Strict : CallerMode::NonStrict, EvalMode::Direct);
+    }
+
     vm.call_frame().current_node = interpreter.current_node();
     Object* new_object = nullptr;
     Value result;
diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp
index 81acec4917..5f13e32c16 100644
--- a/Userland/Libraries/LibJS/Parser.cpp
+++ b/Userland/Libraries/LibJS/Parser.cpp
@@ -226,11 +226,15 @@ Associativity Parser::operator_associativity(TokenType type) const
     }
 }
 
-NonnullRefPtr<Program> Parser::parse_program()
+NonnullRefPtr<Program> Parser::parse_program(bool starts_in_strict_mode)
 {
     auto rule_start = push_start();
     ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Let | ScopePusher::Function);
     auto program = adopt_ref(*new Program({ m_filename, rule_start.position(), position() }));
+    if (starts_in_strict_mode) {
+        program->set_strict_mode();
+        m_state.strict_mode = true;
+    }
 
     bool first = true;
     while (!done()) {
diff --git a/Userland/Libraries/LibJS/Parser.h b/Userland/Libraries/LibJS/Parser.h
index 245bf59ba1..a53352c940 100644
--- a/Userland/Libraries/LibJS/Parser.h
+++ b/Userland/Libraries/LibJS/Parser.h
@@ -37,7 +37,7 @@ class Parser {
 public:
     explicit Parser(Lexer lexer);
 
-    NonnullRefPtr<Program> parse_program();
+    NonnullRefPtr<Program> parse_program(bool starts_in_strict_mode = false);
 
     template<typename FunctionNodeType>
     NonnullRefPtr<FunctionNodeType> parse_function_node(u8 parse_options = FunctionNodeParseOptions::CheckForFunctionAndName);
diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
index 21e23f295e..1865212e0a 100644
--- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
+++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
@@ -7,12 +7,16 @@
 
 #include <AK/Function.h>
 #include <AK/Result.h>
+#include <AK/TemporaryChange.h>
+#include <LibJS/Interpreter.h>
+#include <LibJS/Parser.h>
 #include <LibJS/Runtime/AbstractOperations.h>
 #include <LibJS/Runtime/BoundFunction.h>
 #include <LibJS/Runtime/DeclarativeEnvironmentRecord.h>
 #include <LibJS/Runtime/ErrorTypes.h>
 #include <LibJS/Runtime/Function.h>
 #include <LibJS/Runtime/FunctionEnvironmentRecord.h>
+#include <LibJS/Runtime/GlobalEnvironmentRecord.h>
 #include <LibJS/Runtime/GlobalObject.h>
 #include <LibJS/Runtime/Object.h>
 #include <LibJS/Runtime/ObjectEnvironmentRecord.h>
@@ -198,4 +202,30 @@ Object* get_super_constructor(VM& vm)
     return super_constructor;
 }
 
+// 19.2.1.1 PerformEval ( x, callerRealm, strictCaller, direct ), https://tc39.es/ecma262/#sec-performeval
+Value perform_eval(Value x, GlobalObject& caller_realm, CallerMode strict_caller, EvalMode direct)
+{
+    VERIFY(direct == EvalMode::Direct || strict_caller == CallerMode::NonStrict);
+    if (!x.is_string())
+        return x;
+
+    auto& vm = caller_realm.vm();
+    auto& code_string = x.as_string();
+    Parser parser { Lexer { code_string.string() } };
+    auto program = parser.parse_program(strict_caller == CallerMode::Strict);
+
+    if (parser.has_errors()) {
+        auto& error = parser.errors()[0];
+        vm.throw_exception<SyntaxError>(caller_realm, error.to_string());
+        return {};
+    }
+
+    auto& interpreter = vm.interpreter();
+    if (direct == EvalMode::Direct)
+        return interpreter.execute_statement(caller_realm, program).value_or(js_undefined());
+
+    TemporaryChange scope_change(vm.call_frame().lexical_environment, static_cast<EnvironmentRecord*>(&caller_realm.environment_record()));
+    return interpreter.execute_statement(caller_realm, program).value_or(js_undefined());
+}
+
 }
diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.h b/Userland/Libraries/LibJS/Runtime/AbstractOperations.h
index 737ebdf368..1e00d90cb7 100644
--- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.h
+++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.h
@@ -25,6 +25,16 @@ Function* species_constructor(GlobalObject&, Object const&, Function& default_co
 GlobalObject* get_function_realm(GlobalObject&, Function const&);
 Object* get_prototype_from_constructor(GlobalObject&, Function const& constructor, Object* (GlobalObject::*intrinsic_default_prototype)());
 
+enum class CallerMode {
+    Strict,
+    NonStrict
+};
+enum class EvalMode {
+    Direct,
+    Indirect
+};
+Value perform_eval(Value, GlobalObject&, CallerMode, EvalMode);
+
 // 10.1.13 OrdinaryCreateFromConstructor ( constructor, intrinsicDefaultProto [ , internalSlotsList ] ), https://tc39.es/ecma262/#sec-ordinarycreatefromconstructor
 template<typename T, typename... Args>
 T* ordinary_create_from_constructor(GlobalObject& global_object, Function const& constructor, Object* (GlobalObject::*intrinsic_default_prototype)(), Args&&... args)
diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp
index cdd4dddff3..5217340603 100644
--- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp
@@ -8,13 +8,13 @@
 #include <AK/CharacterTypes.h>
 #include <AK/Hex.h>
 #include <AK/Platform.h>
-#include <AK/TemporaryChange.h>
 #include <AK/Utf8View.h>
 #include <LibJS/Console.h>
 #include <LibJS/Heap/DeferGC.h>
 #include <LibJS/Interpreter.h>
 #include <LibJS/Lexer.h>
 #include <LibJS/Parser.h>
+#include <LibJS/Runtime/AbstractOperations.h>
 #include <LibJS/Runtime/AggregateErrorConstructor.h>
 #include <LibJS/Runtime/AggregateErrorPrototype.h>
 #include <LibJS/Runtime/ArrayBufferConstructor.h>
@@ -140,6 +140,8 @@ void GlobalObject::initialize_global_object()
     define_native_function(vm.names.parseFloat, parse_float, 1, attr);
     define_native_function(vm.names.parseInt, parse_int, 2, attr);
     define_native_function(vm.names.eval, eval, 1, attr);
+    m_eval_function = &get_without_side_effects(vm.names.eval).as_function();
+
     define_native_function(vm.names.encodeURI, encode_uri, 1, attr);
     define_native_function(vm.names.decodeURI, decode_uri, 1, attr);
     define_native_function(vm.names.encodeURIComponent, encode_uri_component, 1, attr);
@@ -223,6 +225,8 @@ void GlobalObject::visit_edges(Visitor& visitor)
     visitor.visit(m_##snake_name##_prototype);
     JS_ENUMERATE_ITERATOR_PROTOTYPES
 #undef __JS_ENUMERATE
+
+    visitor.visit(m_eval_function);
 }
 
 JS_DEFINE_NATIVE_FUNCTION(GlobalObject::gc)
@@ -335,23 +339,7 @@ JS_DEFINE_NATIVE_FUNCTION(GlobalObject::parse_int)
 // 19.2.1 eval ( x ), https://tc39.es/ecma262/#sec-eval-x
 JS_DEFINE_NATIVE_FUNCTION(GlobalObject::eval)
 {
-    if (!vm.argument(0).is_string())
-        return vm.argument(0);
-    auto& code_string = vm.argument(0).as_string();
-    JS::Parser parser { JS::Lexer { code_string.string() } };
-    auto program = parser.parse_program();
-
-    if (parser.has_errors()) {
-        auto& error = parser.errors()[0];
-        vm.throw_exception<SyntaxError>(global_object, error.to_string());
-        return {};
-    }
-
-    auto& caller_frame = vm.call_stack().at(vm.call_stack().size() - 2);
-    TemporaryChange scope_change(vm.call_frame().lexical_environment, caller_frame->lexical_environment);
-
-    auto& interpreter = vm.interpreter();
-    return interpreter.execute_statement(global_object, program).value_or(js_undefined());
+    return perform_eval(vm.argument(0), global_object, CallerMode::NonStrict, EvalMode::Indirect);
 }
 
 // 19.2.6.1.1 Encode ( string, unescapedSet ), https://tc39.es/ecma262/#sec-encode
diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.h b/Userland/Libraries/LibJS/Runtime/GlobalObject.h
index 8451169460..669a99ef9b 100644
--- a/Userland/Libraries/LibJS/Runtime/GlobalObject.h
+++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.h
@@ -36,6 +36,8 @@ public:
     // Not included in JS_ENUMERATE_NATIVE_OBJECTS due to missing distinct constructor
     GeneratorObjectPrototype* generator_object_prototype() { return m_generator_object_prototype; }
 
+    Function* eval_function() const { return m_eval_function; }
+
 #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \
     ConstructorName* snake_name##_constructor() { return m_##snake_name##_constructor; } \
     Object* snake_name##_prototype() { return m_##snake_name##_prototype; }
@@ -95,6 +97,8 @@ private:
     Object* m_##snake_name##_prototype { nullptr };
     JS_ENUMERATE_ITERATOR_PROTOTYPES
 #undef __JS_ENUMERATE
+
+    Function* m_eval_function;
 };
 
 template<typename ConstructorType>
diff --git a/Userland/Libraries/LibJS/Tests/eval-aliasing.js b/Userland/Libraries/LibJS/Tests/eval-aliasing.js
new file mode 100644
index 0000000000..2779783535
--- /dev/null
+++ b/Userland/Libraries/LibJS/Tests/eval-aliasing.js
@@ -0,0 +1,15 @@
+test("variable named 'eval' pointing to another function calls that function", function () {
+    var testValue = "inner";
+    // This breaks prettier as it considers this to be a parse error
+    // before even trying to do any linting
+    var eval = () => {
+        return "wat";
+    };
+    expect(eval("testValue")).toEqual("wat");
+});
+
+test("variable named 'eval' pointing to real eval works as a direct eval", function () {
+    var testValue = "inner";
+    var eval = globalThis.eval;
+    expect(eval("testValue")).toEqual("inner");
+});
diff --git a/Userland/Libraries/LibJS/Tests/eval-basic.js b/Userland/Libraries/LibJS/Tests/eval-basic.js
index 43b75028c6..0443d39662 100644
--- a/Userland/Libraries/LibJS/Tests/eval-basic.js
+++ b/Userland/Libraries/LibJS/Tests/eval-basic.js
@@ -30,3 +30,46 @@ test("returns 1st argument unless 1st argument is a string", () => {
     var stringObject = new String("1 + 2");
     expect(eval(stringObject)).toBe(stringObject);
 });
+
+// These eval scope tests use function expressions due to bug #8198
+var testValue = "outer";
+test("eval only touches locals if direct use", function () {
+    var testValue = "inner";
+    expect(globalThis.eval("testValue")).toEqual("outer");
+});
+
+test("alias to eval works as a global eval", function () {
+    var testValue = "inner";
+    var eval1 = globalThis.eval;
+    expect(eval1("testValue")).toEqual("outer");
+});
+
+test("eval evaluates all args", function () {
+    var i = 0;
+    expect(eval("testValue", i++, i++, i++)).toEqual("outer");
+    expect(i).toEqual(3);
+});
+
+test("eval tests for exceptions", function () {
+    var i = 0;
+    expect(function () {
+        eval("testValue", i++, i++, j, i++);
+    }).toThrowWithMessage(ReferenceError, "'j' is not defined");
+    expect(i).toEqual(2);
+});
+
+test("direct eval inherits non-strict evaluation", function () {
+    expect(eval("01")).toEqual(1);
+});
+
+test("direct eval inherits strict evaluation", function () {
+    "use strict";
+    expect(() => {
+        eval("01");
+    }).toThrowWithMessage(SyntaxError, "Unprefixed octal number not allowed in strict mode");
+});
+
+test("global eval evaluates as non-strict", function () {
+    "use strict";
+    expect(globalThis.eval("01"));
+});