LibJS: Disallow duplicated variable declarations

Userland/Libraries/LibJS/Parser.cpp

@@ -1955,6 +1955,7 @@ NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool for_l
     for (;;) {
         Variant<NonnullRefPtr<Identifier>, NonnullRefPtr<BindingPattern>, Empty> target { Empty() };
         if (match_identifier()) {
+            auto identifier_start = push_start();
             auto name = consume_identifier().value();
             target = create_ast_node<Identifier>(
                 { m_state.current_token.filename(), rule_start.position(), position() },
@@ -1962,6 +1963,33 @@ NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool for_l
             check_identifier_name_for_assignment_validity(name);
             if ((declaration_kind == DeclarationKind::Let || declaration_kind == DeclarationKind::Const) && name == "let"sv)
                 syntax_error("Lexical binding may not be called 'let'");
+
+            // Check we do not have duplicates
+            auto check_declarations = [&](VariableDeclarator const& declarator) {
+                declarator.target().visit([&](NonnullRefPtr<Identifier> const& identifier) {
+                    if (identifier->string() == name)
+                        syntax_error(String::formatted("Identifier '{}' has already been declared", name), identifier_start.position()); },
+                    [&](auto const&) {});
+            };
+
+            // In any previous let scope
+            if (!m_state.let_scopes.is_empty()) {
+                for (auto& decls : m_state.let_scopes.last()) {
+                    for (auto& decl : decls.declarations()) {
+                        check_declarations(decl);
+                    }
+                }
+            }
+
+            // or this declaration
+            if (declaration_kind == DeclarationKind::Let || declaration_kind == DeclarationKind::Const) {
+                // FIXME: We should check the var_scopes here as well however this has edges cases with for loops.
+                //        See duplicated-variable-declarations.js.
+
+                for (auto& declaration : declarations) {
+                    check_declarations(declaration);
+                }
+            }
         } else if (auto pattern = parse_binding_pattern()) {
             target = pattern.release_nonnull();
 

Userland/Libraries/LibJS/Tests/builtins/Proxy/Proxy.handler-setPrototypeOf.js

@@ -6,9 +6,9 @@ describe("[[SetPrototypeOf]] trap normal behavior", () => {
 
         let p = new Proxy(o, { setPrototypeOf: null });
         expect(Object.setPrototypeOf(p, proto)).toBe(p);
-        let p = new Proxy(o, { setPrototypeOf: undefined });
+        p = new Proxy(o, { setPrototypeOf: undefined });
         expect(Object.setPrototypeOf(p, proto)).toBe(p);
-        let p = new Proxy(o, {});
+        p = new Proxy(o, {});
         expect(Object.setPrototypeOf(p, proto)).toBe(p);
     });
 

Userland/Libraries/LibJS/Tests/duplicated-variable-declarations.js

@@ -0,0 +1,30 @@
+describe("duplicated variable declarations should throw", () => {
+    test("given two declarations in the same statement", () => {
+        expect("let a, a;").not.toEval();
+        expect("const a, a;").not.toEval();
+
+        expect("var a, a;").toEval();
+    });
+
+    test("fail to parse if repeated over multiple statements", () => {
+        expect("let a; var a;").not.toEval();
+        expect("let b, a; var c, a;").not.toEval();
+        expect("const a; var a;").not.toEval();
+        expect("const b, a; var c, a;").not.toEval();
+    });
+
+    test.skip("should fail to parse even if variable first declared with var", () => {
+        expect.skip("var a; let a;").toEval();
+        expect.skip("var a; let b, a;").toEval();
+        expect.skip("var a; const a;").toEval();
+        expect.skip("var a; const b, a;").toEval();
+    });
+
+    test.skip("special cases with for loops", () => {
+        expect("for (var a;;) { let a; }").toEval();
+        expect("for (let a;;) { let a; }").toEval();
+        expect("for (var a;;) { var a; }").toEval();
+
+        expect("for (let a;;) { var a; }").not.toEval();
+    });
+});

Userland/Libraries/LibJS/Tests/iterators/string-iterator.js

@@ -15,12 +15,12 @@ test("basic functionality", () => {
 });
 
 test("casts |this| to string", () => {
-    const it = String.prototype[Symbol.iterator].call(45);
+    let it = String.prototype[Symbol.iterator].call(45);
     expect(it.next()).toEqual({ value: "4", done: false });
     expect(it.next()).toEqual({ value: "5", done: false });
     expect(it.next()).toEqual({ value: undefined, done: true });
 
-    const it = String.prototype[Symbol.iterator].call(false);
+    it = String.prototype[Symbol.iterator].call(false);
     expect(it.next()).toEqual({ value: "f", done: false });
     expect(it.next()).toEqual({ value: "a", done: false });
     expect(it.next()).toEqual({ value: "l", done: false });