Add SyntaxErrors in GlobalDeclarationInstantiation (#2908)

* Add SyntaxErrors in GlobalDeclarationInstantiation

* Fix tests

* Apply suggestions

boa_engine/src/builtins/string/tests.rs

@@ -569,25 +569,25 @@ fn split_with_symbol_split_method() {
         TestAction::run_harness(),
         TestAction::assert_eq(
             indoc! {r#"
-                let sep = {};
-                sep[Symbol.split] = function(s, limit) { return s + limit.toString(); };
-                'hello'.split(sep, 10)
+                let sep_a = {};
+                sep_a[Symbol.split] = function(s, limit) { return s + limit.toString(); };
+                'hello'.split(sep_a, 10)
             "#},
             "hello10",
         ),
         TestAction::assert(indoc! {r#"
-                let sep = {};
-                sep[Symbol.split] = undefined;
+                let sep_b = {};
+                sep_b[Symbol.split] = undefined;
                 arrayEquals(
-                    'hello'.split(sep),
+                    'hello'.split(sep_b),
                     ['hello']
                 )
             "#}),
         TestAction::assert_native_error(
             indoc! {r#"
-                let sep = {};
-                sep[Symbol.split] = 10;
-                'hello'.split(sep, 10);
+                let sep_c = {};
+                sep_c[Symbol.split] = 10;
+                'hello'.split(sep_c, 10);
             "#},
             JsNativeErrorKind::Type,
             "value returned for property of object is not a function",

boa_engine/src/bytecompiler/declarations.rs

@@ -26,16 +26,44 @@ impl ByteCompiler<'_, '_> {
         &mut self,
         script: &StatementList,
     ) -> JsResult<()> {
-        // Note: Step 1-4 Are currently handled while parsing.
         // 1. Let lexNames be the LexicallyDeclaredNames of script.
+        let lex_names = top_level_lexically_declared_names(script);
+
         // 2. Let varNames be the VarDeclaredNames of script.
+        let var_names = top_level_var_declared_names(script);
+
         // 3. For each element name of lexNames, do
-        //     a. If env.HasVarDeclaration(name) is true, throw a SyntaxError exception.
-        //     b. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
-        //     c. Let hasRestrictedGlobal be ? env.HasRestrictedGlobalProperty(name).
-        //     d. If hasRestrictedGlobal is true, throw a SyntaxError exception.
+        for name in lex_names {
+            // Note: Our implementation differs from the spec here.
+            // a. If env.HasVarDeclaration(name) is true, throw a SyntaxError exception.
+
+            // b. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
+            if self.has_binding(name) {
+                return Err(JsNativeError::syntax()
+                    .with_message("duplicate lexical declaration")
+                    .into());
+            }
+
+            // c. Let hasRestrictedGlobal be ? env.HasRestrictedGlobalProperty(name).
+            let has_restricted_global = self.context.has_restricted_global_property(name)?;
+
+            // d. If hasRestrictedGlobal is true, throw a SyntaxError exception.
+            if has_restricted_global {
+                return Err(JsNativeError::syntax()
+                    .with_message("cannot redefine non-configurable global property")
+                    .into());
+            }
+        }
+
         // 4. For each element name of varNames, do
-        //     a. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
+        for name in var_names {
+            // a. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
+            if self.has_binding(name) {
+                return Err(JsNativeError::syntax()
+                    .with_message("duplicate lexical declaration")
+                    .into());
+            }
+        }
 
         // 5. Let varDeclarations be the VarScopedDeclarations of script.
         // Note: VarScopedDeclarations for a Script node is TopLevelVarScopedDeclarations.

boa_engine/src/context/mod.rs

@@ -812,6 +812,35 @@ impl Context<'_> {
         // 9. Return unused.
         Ok(())
     }
+
+    /// `HasRestrictedGlobalProperty ( N )`
+    ///
+    /// More information:
+    ///  - [ECMAScript reference][spec]
+    ///
+    /// [spec]: https://tc39.es/ecma262/#sec-hasrestrictedglobalproperty
+    pub(crate) fn has_restricted_global_property(&mut self, name: Identifier) -> JsResult<bool> {
+        // 1. Let ObjRec be envRec.[[ObjectRecord]].
+        // 2. Let globalObject be ObjRec.[[BindingObject]].
+        let global_object = self.realm().global_object().clone();
+
+        // 3. Let existingProp be ? globalObject.[[GetOwnProperty]](N).
+        let name = PropertyKey::from(self.interner().resolve_expect(name.sym()).utf16());
+        let existing_prop = global_object.__get_own_property__(&name, self)?;
+
+        // 4. If existingProp is undefined, return false.
+        let Some(existing_prop) = existing_prop else {
+            return Ok(false);
+        };
+
+        // 5. If existingProp.[[Configurable]] is true, return false.
+        if existing_prop.configurable() == Some(true) {
+            return Ok(false);
+        }
+
+        // 6. Return true.
+        Ok(true)
+    }
 }
 
 impl<'host> Context<'host> {

boa_engine/src/tests/operators.rs

@@ -25,25 +25,25 @@ fn short_circuit_evaluation() {
         // short circuiting OR.
         TestAction::assert_eq(
             indoc! {r#"
-                function add_one(counter) {
+                function add_one_a(counter) {
                     counter.value += 1;
                     return true;
                 }
-                let counter = { value: 0 };
-                let _ = add_one(counter) || add_one(counter);
-                counter.value
+                let counter_a = { value: 0 };
+                add_one_a(counter_a) || add_one_a(counter_a);
+                counter_a.value
             "#},
             1,
         ),
         TestAction::assert_eq(
             indoc! {r#"
-                function add_one(counter) {
+                function add_one_b(counter) {
                     counter.value += 1;
                     return false;
                 }
-                let counter = { value: 0 };
-                let _ = add_one(counter) || add_one(counter);
-                counter.value
+                let counter_b = { value: 0 };
+                add_one_b(counter_b) || add_one_b(counter_b);
+                counter_b.value
             "#},
             2,
         ),
@@ -55,25 +55,25 @@ fn short_circuit_evaluation() {
         // short circuiting AND
         TestAction::assert_eq(
             indoc! {r#"
-                function add_one(counter) {
+                function add_one_c(counter) {
                     counter.value += 1;
                     return true;
                 }
-                let counter = { value: 0 };
-                let _ = add_one(counter) && add_one(counter);
-                counter.value
+                let counter_c = { value: 0 };
+                add_one_c(counter_c) && add_one_c(counter_c);
+                counter_c.value
             "#},
             2,
         ),
         TestAction::assert_eq(
             indoc! {r#"
-                function add_one(counter) {
+                function add_one_d(counter) {
                     counter.value += 1;
                     return false;
                 }
-                let counter = { value: 0 };
-                let _ = add_one(counter) && add_one(counter);
-                counter.value
+                let counter_d = { value: 0 };
+                add_one_d(counter_d) && add_one_d(counter_d);
+                counter_d.value
             "#},
             1,
         ),
@@ -114,24 +114,24 @@ fn assign_operator_precedence() {
 fn unary_pre() {
     run_test_actions([
         TestAction::assert_eq("{ let a = 5; ++a; a }", 6),
-        TestAction::assert_eq("{ let a = 5; --a; a }", 4),
-        TestAction::assert_eq("{ const a = { b: 5 }; ++a.b; a['b'] }", 6),
-        TestAction::assert_eq("{ const a = { b: 5 }; --a['b']; a.b }", 4),
-        TestAction::assert_eq("{ let a = 5; ++a }", 6),
-        TestAction::assert_eq("{ let a = 5; --a }", 4),
-        TestAction::assert_eq("{ let a = 2147483647; ++a }", 2_147_483_648_i64),
-        TestAction::assert_eq("{ let a = -2147483648; --a }", -2_147_483_649_i64),
+        TestAction::assert_eq("{ let b = 5; --b; b }", 4),
+        TestAction::assert_eq("{ const c = { a: 5 }; ++c.a; c['a'] }", 6),
+        TestAction::assert_eq("{ const d = { a: 5 }; --d['a']; d.a }", 4),
+        TestAction::assert_eq("{ let e = 5; ++e }", 6),
+        TestAction::assert_eq("{ let f = 5; --f }", 4),
+        TestAction::assert_eq("{ let g = 2147483647; ++g }", 2_147_483_648_i64),
+        TestAction::assert_eq("{ let h = -2147483648; --h }", -2_147_483_649_i64),
         TestAction::assert_eq(
             indoc! {r#"
-                let a = {[Symbol.toPrimitive]() { return 123; }};
-                ++a
+                let i = {[Symbol.toPrimitive]() { return 123; }};
+                ++i
             "#},
             124,
         ),
         TestAction::assert_eq(
             indoc! {r#"
-                let a = {[Symbol.toPrimitive]() { return 123; }};
-                ++a
+                let j = {[Symbol.toPrimitive]() { return 123; }};
+                ++j
             "#},
             124,
         ),
@@ -210,24 +210,24 @@ fn typeofs() {
 fn unary_post() {
     run_test_actions([
         TestAction::assert_eq("{ let a = 5; a++; a }", 6),
-        TestAction::assert_eq("{ let a = 5; a--; a }", 4),
-        TestAction::assert_eq("{ const a = { b: 5 }; a.b++; a['b'] }", 6),
-        TestAction::assert_eq("{ const a = { b: 5 }; a['b']--; a.b }", 4),
-        TestAction::assert_eq("{ let a = 5; a++ }", 5),
-        TestAction::assert_eq("{ let a = 5; a-- }", 5),
-        TestAction::assert_eq("{ let a = 2147483647; a++; a }", 2_147_483_648_i64),
-        TestAction::assert_eq("{ let a = -2147483648; a--; a }", -2_147_483_649_i64),
+        TestAction::assert_eq("{ let b = 5; b--; b }", 4),
+        TestAction::assert_eq("{ const c = { a: 5 }; c.a++; c['a'] }", 6),
+        TestAction::assert_eq("{ const d = { a: 5 }; d['a']--; d.a }", 4),
+        TestAction::assert_eq("{ let e = 5; e++ }", 5),
+        TestAction::assert_eq("{ let f = 5; f-- }", 5),
+        TestAction::assert_eq("{ let g = 2147483647; g++; g }", 2_147_483_648_i64),
+        TestAction::assert_eq("{ let h = -2147483648; h--; h }", -2_147_483_649_i64),
         TestAction::assert_eq(
             indoc! {r#"
-                let a = {[Symbol.toPrimitive]() { return 123; }};
-                a++
+                let i = {[Symbol.toPrimitive]() { return 123; }};
+                i++
             "#},
             123,
         ),
         TestAction::assert_eq(
             indoc! {r#"
-                let a = {[Symbol.toPrimitive]() { return 123; }};
-                a--
+                let j = {[Symbol.toPrimitive]() { return 123; }};
+                j--
             "#},
             123,
         ),

boa_tester/src/exec/mod.rs

@@ -243,7 +243,7 @@ impl Test {
                 (true, value.display().to_string())
             }
             Outcome::Negative {
-                phase: Phase::Parse | Phase::Early,
+                phase: Phase::Parse,
                 error_type,
             } => {
                 assert_eq!(
@@ -259,18 +259,12 @@ impl Test {
 
                 if self.is_module() {
                     match context.parse_module(source) {
-                        Ok(module_item_list) => match context.compile_module(&module_item_list) {
-                            Ok(_) => (false, "ModuleItemList compilation should fail".to_owned()),
-                            Err(e) => (true, format!("Uncaught {e:?}")),
-                        },
+                        Ok(_) => (false, "ModuleItemList parsing should fail".to_owned()),
                         Err(e) => (true, format!("Uncaught {e}")),
                     }
                 } else {
                     match context.parse_script(source) {
-                        Ok(statement_list) => match context.compile_script(&statement_list) {
-                            Ok(_) => (false, "StatementList compilation should fail".to_owned()),
-                            Err(e) => (true, format!("Uncaught {e:?}")),
-                        },
+                        Ok(_) => (false, "StatementList parsing should fail".to_owned()),
                         Err(e) => (true, format!("Uncaught {e}")),
                     }
                 }
@@ -290,30 +284,33 @@ impl Test {
                 if let Err(e) = self.set_up_env(harness, context, AsyncResult::default()) {
                     return (false, e);
                 }
-                let code = if self.is_module() {
-                    match context
-                        .parse_module(source)
-                        .map_err(Into::into)
-                        .and_then(|stmts| context.compile_module(&stmts))
-                    {
+
+                let e = if self.is_module() {
+                    let module = match context.parse_module(source) {
                         Ok(code) => code,
                         Err(e) => return (false, format!("Uncaught {e}")),
-                    }
-                } else {
+                    };
                     match context
-                        .parse_script(source)
-                        .map_err(Into::into)
-                        .and_then(|stmts| context.compile_script(&stmts))
+                        .compile_module(&module)
+                        .and_then(|code| context.execute(code))
                     {
+                        Ok(_) => return (false, "Module execution should fail".to_owned()),
+                        Err(e) => e,
+                    }
+                } else {
+                    let script = match context.parse_script(source) {
                         Ok(code) => code,
                         Err(e) => return (false, format!("Uncaught {e}")),
+                    };
+                    match context
+                        .compile_script(&script)
+                        .and_then(|code| context.execute(code))
+                    {
+                        Ok(_) => return (false, "Script execution should fail".to_owned()),
+                        Err(e) => e,
                     }
                 };
 
-                let e = match context.execute(code) {
-                    Ok(res) => return (false, res.display().to_string()),
-                    Err(e) => e,
-                };
                 if let Ok(e) = e.try_native(context) {
                     match &e.kind {
                         JsNativeErrorKind::Syntax if error_type == ErrorType::SyntaxError => {}

boa_tester/src/main.rs

@@ -773,7 +773,6 @@ impl<'de> Deserialize<'de> for TestFlags {
 #[serde(rename_all = "lowercase")]
 enum Phase {
     Parse,
-    Early,
     Resolution,
     Runtime,
 }