Move redeclaration errors to parser (#2027)

This Pull Request changes the following:

- Implement redeclaration errors in the parser
- Remove redeclaration errors from the compiler (this is a big step towards #1907)
- Fix some failing tests on the way

This requires a slight change in our public api. The Parser new requires a full `Context` instead of just the `Interner` for parsing new code. This is required, because if multiple scripts are parsed (e.g. every input in the REPL) global variables must be checked for redeclarations.

boa_cli/src/main.rs

@@ -60,7 +60,6 @@
 )]
 
 use boa_engine::{syntax::ast::node::StatementList, Context};
-use boa_interner::Interner;
 use clap::{ArgEnum, Parser};
 use colored::{Color, Colorize};
 use rustyline::{config::Config, error::ReadlineError, EditMode, Editor};
@@ -135,29 +134,28 @@ enum DumpFormat {
 ///
 /// Returns a error of type String with a message,
 /// if the token stream has a parsing error.
-fn parse_tokens<S>(src: S, interner: &mut Interner) -> Result<StatementList, String>
+fn parse_tokens<S>(src: S, context: &mut Context) -> Result<StatementList, String>
 where
     S: AsRef<[u8]>,
 {
     use boa_engine::syntax::parser::Parser;
 
     let src_bytes = src.as_ref();
     Parser::new(src_bytes, false)
-        .parse_all(interner)
+        .parse_all(context)
         .map_err(|e| format!("ParsingError: {e}"))
 }
 
 /// Dumps the AST to stdout with format controlled by the given arguments.
 ///
 /// Returns a error of type String with a error message,
 /// if the source has a syntax or parsing error.
-fn dump<S>(src: S, args: &Opt) -> Result<(), String>
+fn dump<S>(src: S, args: &Opt, context: &mut Context) -> Result<(), String>
 where
     S: AsRef<[u8]>,
 {
     if let Some(ref arg) = args.dump_ast {
-        let mut interner = Interner::default();
-        let ast = parse_tokens(src, &mut interner)?;
+        let ast = parse_tokens(src, context)?;
 
         match arg {
             Some(format) => match format {
@@ -194,7 +192,7 @@ pub fn main() -> Result<(), std::io::Error> {
         let buffer = read(file)?;
 
         if args.has_dump_flag() {
-            if let Err(e) = dump(&buffer, &args) {
+            if let Err(e) = dump(&buffer, &args, &mut context) {
                 eprintln!("{e}");
             }
         } else {
@@ -233,7 +231,7 @@ pub fn main() -> Result<(), std::io::Error> {
                     editor.add_history_entry(&line);
 
                     if args.has_dump_flag() {
-                        if let Err(e) = dump(&line, &args) {
+                        if let Err(e) = dump(&line, &args, &mut context) {
                             eprintln!("{e}");
                         }
                     } else {

boa_engine/src/context/mod.rs

@@ -182,7 +182,7 @@ impl Context {
     where
         S: AsRef<[u8]>,
     {
-        Parser::new(src.as_ref(), self.strict).parse_all(&mut self.interner)
+        Parser::new(src.as_ref(), self.strict).parse_all(self)
     }
 
     /// <https://tc39.es/ecma262/#sec-call>
@@ -204,12 +204,6 @@ impl Context {
         self.realm.global_object()
     }
 
-    /// Return a reference to the global object string bindings.
-    #[inline]
-    pub(crate) fn global_bindings(&self) -> &GlobalPropertyMap {
-        self.realm.global_bindings()
-    }
-
     /// Return a mutable reference to the global object string bindings.
     #[inline]
     pub(crate) fn global_bindings_mut(&mut self) -> &mut GlobalPropertyMap {
@@ -648,7 +642,7 @@ impl Context {
         let main_timer = Profiler::global().start_event("Evaluation", "Main");
 
         let parsing_result = Parser::new(src.as_ref(), false)
-            .parse_all(&mut self.interner)
+            .parse_all(self)
             .map_err(|e| e.to_string());
 
         let statement_list = match parsing_result {

boa_engine/src/environments/compile.rs

@@ -1,6 +1,5 @@
 use crate::{
-    environments::runtime::BindingLocator, property::PropertyDescriptor, Context, JsResult,
-    JsString, JsValue,
+    environments::runtime::BindingLocator, property::PropertyDescriptor, Context, JsString, JsValue,
 };
 use boa_interner::Sym;
 use rustc_hash::FxHashMap;
@@ -154,22 +153,13 @@ impl Context {
     ///
     /// Panics if the global environment is not function scoped.
     #[inline]
-    pub(crate) fn create_mutable_binding(
-        &mut self,
-        name: Sym,
-        function_scope: bool,
-        allow_name_reuse: bool,
-    ) -> JsResult<()> {
+    pub(crate) fn create_mutable_binding(&mut self, name: Sym, function_scope: bool) {
         let name_str = JsString::from(self.interner().resolve_expect(name));
 
         for (i, env) in self.realm.compile_env.stack.iter_mut().enumerate().rev() {
             if !function_scope || env.function_scope {
                 if env.bindings.contains_key(&name) {
-                    if allow_name_reuse {
-                        return Ok(());
-                    }
-                    return self
-                        .throw_syntax_error(format!("Redeclaration of variable {}", name_str));
+                    return;
                 }
 
                 if i == 0 {
@@ -178,10 +168,6 @@ impl Context {
                         .global_property_map
                         .string_property_map()
                         .get(&name_str);
-                    let non_configurable_binding_exists = match desc {
-                        Some(desc) => !matches!(desc.configurable(), Some(true)),
-                        None => false,
-                    };
                     if function_scope && desc.is_none() {
                         self.global_bindings_mut().insert(
                             name_str,
@@ -192,15 +178,9 @@ impl Context {
                                 .configurable(true)
                                 .build(),
                         );
-                        return Ok(());
+                        return;
                     } else if function_scope {
-                        return Ok(());
-                    } else if !function_scope
-                        && !allow_name_reuse
-                        && non_configurable_binding_exists
-                    {
-                        return self
-                            .throw_syntax_error(format!("Redeclaration of variable {}", name_str));
+                        return;
                     }
                 }
 
@@ -212,7 +192,7 @@ impl Context {
                         mutable: true,
                     },
                 );
-                return Ok(());
+                return;
             }
             continue;
         }
@@ -249,31 +229,22 @@ impl Context {
     ///
     /// Panics if the global environment does not exist.
     #[inline]
-    pub(crate) fn create_immutable_binding(&mut self, name: Sym) -> JsResult<()> {
-        let name_str = JsString::from(self.interner().resolve_expect(name));
-        let exists_global = self.realm.compile_env.stack.len() == 1
-            && self.global_bindings().contains_key(&name_str);
-
+    pub(crate) fn create_immutable_binding(&mut self, name: Sym) {
         let env = self
             .realm
             .compile_env
             .stack
             .last_mut()
             .expect("global environment must always exist");
 
-        if env.bindings.contains_key(&name) || exists_global {
-            self.throw_syntax_error(format!("Redeclaration of variable {}", name_str))
-        } else {
-            let binding_index = env.bindings.len();
-            env.bindings.insert(
-                name,
-                CompileTimeBinding {
-                    index: binding_index,
-                    mutable: false,
-                },
-            );
-            Ok(())
-        }
+        let binding_index = env.bindings.len();
+        env.bindings.insert(
+            name,
+            CompileTimeBinding {
+                index: binding_index,
+                mutable: false,
+            },
+        );
     }
 
     /// Initialize an immutable binding at bytecode compile time and return it's binding locator.

boa_engine/src/realm.rs

@@ -45,11 +45,6 @@ impl Realm {
         &self.global_object
     }
 
-    #[inline]
-    pub(crate) fn global_bindings(&self) -> &GlobalPropertyMap {
-        self.global_property_map.string_property_map()
-    }
-
     #[inline]
     pub(crate) fn global_bindings_mut(&mut self) -> &mut GlobalPropertyMap {
         self.global_property_map.string_property_map_mut()

boa_engine/src/syntax/ast/node/block/mod.rs

@@ -4,7 +4,6 @@ use super::{Node, StatementList};
 use boa_gc::{Finalize, Trace};
 use boa_interner::{Interner, Sym, ToInternedString};
 
-use rustc_hash::FxHashSet;
 #[cfg(feature = "deser")]
 use serde::{Deserialize, Serialize};
 
@@ -40,12 +39,9 @@ impl Block {
         self.statements.items()
     }
 
-    pub(crate) fn lexically_declared_names(&self, interner: &Interner) -> FxHashSet<Sym> {
-        self.statements.lexically_declared_names(interner)
-    }
-
-    pub(crate) fn var_declared_named(&self) -> FxHashSet<Sym> {
-        self.statements.var_declared_names()
+    /// Get the lexically declared names of the block.
+    pub(crate) fn lexically_declared_names(&self) -> Vec<(Sym, bool)> {
+        self.statements.lexically_declared_names()
     }
 
     /// Implements the display formatting with indentation.

boa_engine/src/syntax/ast/node/iteration/mod.rs

@@ -8,7 +8,7 @@ use crate::syntax::ast::node::{
     declaration::Declaration, identifier::Identifier, DeclarationPattern,
 };
 use boa_gc::{Finalize, Trace};
-use boa_interner::{Interner, ToInternedString};
+use boa_interner::{Interner, Sym, ToInternedString};
 
 #[cfg(feature = "deser")]
 use serde::{Deserialize, Serialize};
@@ -26,6 +26,27 @@ pub enum IterableLoopInitializer {
     DeclarationPattern(DeclarationPattern),
 }
 
+impl IterableLoopInitializer {
+    /// Return the bound names of a for loop initializer.
+    ///
+    /// The returned list may contain duplicates.
+    ///
+    /// More information:
+    ///  - [ECMAScript specification][spec]
+    ///
+    /// [spec]: https://tc39.es/ecma262/#sec-static-semantics-boundnames
+    pub(crate) fn bound_names(&self) -> Vec<Sym> {
+        match self {
+            IterableLoopInitializer::Let(decl) | IterableLoopInitializer::Const(decl) => match decl
+            {
+                Declaration::Identifier { ident, .. } => vec![ident.sym()],
+                Declaration::Pattern(pattern) => pattern.idents(),
+            },
+            _ => Vec::new(),
+        }
+    }
+}
+
 impl ToInternedString for IterableLoopInitializer {
     fn to_interned_string(&self, interner: &Interner) -> String {
         match self {