Fixed all tests. Now token spans work as expected, and we check that …

…they are correct.

boa/src/exec/declaration.rs

@@ -99,15 +99,20 @@ impl Executable for VarDeclList {
                 Some(v) => v.run(interpreter)?,
                 None => Value::undefined(),
             };
-            interpreter.realm_mut().environment.create_mutable_binding(
-                var.name().to_owned(),
-                false,
-                VariableScope::Function,
-            );
-            interpreter
-                .realm_mut()
-                .environment
-                .initialize_binding(var.name(), val);
+            let environment = &mut interpreter.realm_mut().environment;
+
+            if environment.has_binding(var.name()) {
+                if var.init().is_some() {
+                    environment.set_mutable_binding(var.name(), val, true);
+                }
+            } else {
+                environment.create_mutable_binding(
+                    var.name().to_owned(),
+                    false,
+                    VariableScope::Function,
+                );
+                environment.initialize_binding(var.name(), val);
+            }
         }
         Ok(Value::undefined())
     }

boa/src/exec/mod.rs

@@ -298,7 +298,7 @@ impl Interpreter {
 
     fn set_value(&mut self, node: &Node, value: Value) -> ResultValue {
         match node {
-            Node::Local(ref name) => {
+            Node::Identifier(ref name) => {
                 self.realm
                     .environment
                     .set_mutable_binding(name.as_ref(), value.clone(), true);
@@ -328,7 +328,7 @@ impl Executable for Node {
             Node::Const(Const::String(ref value)) => Ok(Value::string(value.to_string())),
             Node::Const(Const::Bool(value)) => Ok(Value::boolean(value)),
             Node::Block(ref block) => block.run(interpreter),
-            Node::Local(ref name) => {
+            Node::Identifier(ref name) => {
                 let val = interpreter
                     .realm()
                     .environment
@@ -540,7 +540,7 @@ impl Executable for Node {
                                 .exec(obj)?
                                 .remove_property(&interpreter.exec(field)?.to_string()),
                         ),
-                        Node::Local(_) => Value::boolean(false),
+                        Node::Identifier(_) => Value::boolean(false),
                         Node::ArrayDecl(_)
                         | Node::Block(_)
                         | Node::Const(_)

boa/src/exec/operator.rs

@@ -14,29 +14,24 @@ impl Executable for Assign {
     fn run(&self, interpreter: &mut Interpreter) -> ResultValue {
         let val = self.rhs().run(interpreter)?;
         match self.lhs() {
-            Node::Local(ref name) => {
-                if interpreter.realm().environment.has_binding(name.as_ref()) {
+            Node::Identifier(ref name) => {
+                let environment = &mut interpreter.realm_mut().environment;
+
+                if environment.has_binding(name.as_ref()) {
                     // Binding already exists
-                    interpreter.realm_mut().environment.set_mutable_binding(
-                        name.as_ref(),
-                        val.clone(),
-                        true,
-                    );
+                    environment.set_mutable_binding(name.as_ref(), val.clone(), true);
                 } else {
-                    interpreter.realm_mut().environment.create_mutable_binding(
+                    environment.create_mutable_binding(
                         name.as_ref().to_owned(),
                         true,
                         VariableScope::Function,
                     );
-                    interpreter
-                        .realm_mut()
-                        .environment
-                        .initialize_binding(name.as_ref(), val.clone());
+                    environment.initialize_binding(name.as_ref(), val.clone());
                 }
             }
             Node::GetConstField(ref obj, ref field) => {
                 let val_obj = obj.run(interpreter)?;
-                val_obj.set_field_slice(&field.clone(), val.clone());
+                val_obj.set_field_slice(field, val.clone());
             }
             Node::GetField(ref obj, ref field) => {
                 let val_obj = obj.run(interpreter)?;
@@ -117,7 +112,7 @@ impl Executable for BinOp {
                 })
             }
             op::BinOp::Assign(op) => match self.lhs() {
-                Node::Local(ref name) => {
+                Node::Identifier(ref name) => {
                     let v_a = interpreter
                         .realm()
                         .environment

boa/src/syntax/ast/constant.rs

@@ -39,7 +39,7 @@ pub enum Const {
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-terms-and-definitions-string-value
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#String_literals
-    String(String),
+    String(Box<str>),
 
     /// A floating-point number literal.
     ///
@@ -103,19 +103,19 @@ pub enum Const {
 
 impl From<&str> for Const {
     fn from(s: &str) -> Self {
-        Self::String(s.into())
+        Self::String(s.to_owned().into_boxed_str())
     }
 }
 
 impl From<&String> for Const {
     fn from(s: &String) -> Self {
-        Self::String(s.clone())
+        Self::String(s.clone().into_boxed_str())
     }
 }
 
 impl From<String> for Const {
     fn from(s: String) -> Self {
-        Self::String(s)
+        Self::String(s.into_boxed_str())
     }
 }
 

boa/src/syntax/ast/node/declaration.rs

@@ -1,6 +1,6 @@
 //! Declaration nodes.
 
-use super::{join_nodes, FormalParameter, Node, StatementList};
+use super::{join_nodes, FormalParameter, Identifier, Node, StatementList};
 use gc::{Finalize, Trace};
 use std::fmt;
 
@@ -78,7 +78,7 @@ where
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[derive(Clone, Debug, Trace, Finalize, PartialEq)]
 pub struct VarDecl {
-    name: Box<str>,
+    name: Identifier,
     init: Option<Node>,
 }
 
@@ -96,7 +96,7 @@ impl VarDecl {
     /// Creates a new variable declaration.
     pub(crate) fn new<N, I>(name: N, init: I) -> Self
     where
-        N: Into<Box<str>>,
+        N: Into<Identifier>,
         I: Into<Option<Node>>,
     {
         Self {
@@ -107,7 +107,7 @@ impl VarDecl {
 
     /// Gets the name of the variable.
     pub fn name(&self) -> &str {
-        &self.name
+        self.name.as_ref()
     }
 
     /// Gets the initialization node for the variable, if any.

boa/src/syntax/ast/node/local.rs → boa/src/syntax/ast/node/identifier.rs

@@ -26,23 +26,23 @@ use serde::{Deserialize, Serialize};
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(transparent))]
 #[derive(Clone, Debug, Trace, Finalize, PartialEq)]
-pub struct Local {
+pub struct Identifier {
     ident: Box<str>,
 }
 
-impl fmt::Display for Local {
+impl fmt::Display for Identifier {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(&self.ident, f)
     }
 }
 
-impl AsRef<str> for Local {
+impl AsRef<str> for Identifier {
     fn as_ref(&self) -> &str {
         &self.ident
     }
 }
 
-impl<T> From<T> for Local
+impl<T> From<T> for Identifier
 where
     T: Into<Box<str>>,
 {
@@ -51,8 +51,8 @@ where
     }
 }
 
-impl From<Local> for Node {
-    fn from(local: Local) -> Self {
-        Self::Local(local)
+impl From<Identifier> for Node {
+    fn from(local: Identifier) -> Self {
+        Self::Identifier(local)
     }
 }

boa/src/syntax/ast/node/mod.rs

@@ -3,15 +3,15 @@
 pub mod array;
 pub mod block;
 pub mod declaration;
-pub mod local;
+pub mod identifier;
 pub mod operator;
 pub mod statement_list;
 
 pub use self::{
     array::ArrayDecl,
     block::Block,
     declaration::{ArrowFunctionDecl, FunctionDecl, FunctionExpr, VarDecl, VarDeclList},
-    local::Local,
+    identifier::Identifier,
     operator::{Assign, BinOp},
     statement_list::StatementList,
 };
@@ -264,7 +264,7 @@ pub enum Node {
     LetDecl(Box<[(Box<str>, Option<Node>)]>),
 
     /// A local identifier node. [More information](./local/struct.Local.html).
-    Local(Local),
+    Identifier(Identifier),
 
     /// The `new` operator lets developers create an instance of a user-defined object type or of
     /// one of the built-in object types that has a constructor function.
@@ -403,7 +403,7 @@ pub enum Node {
     ///
     /// [spec]: https://tc39.es/ecma262/#prod-TryStatement
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch
-    Try(Block, Option<(Option<Local>, Block)>, Option<Block>),
+    Try(Block, Option<(Option<Identifier>, Block)>, Option<Block>),
 
     /// The JavaScript `this` keyword refers to the object it belongs to.
     ///
@@ -495,10 +495,6 @@ impl Node {
             (_, Node::FunctionDecl(_)) => Ordering::Greater,
             (Node::FunctionDecl(_), _) => Ordering::Less,
 
-            (Node::VarDeclList(_), Node::VarDeclList(_)) => Ordering::Equal,
-            (_, Node::VarDeclList(_)) => Ordering::Greater,
-            (Node::VarDeclList(_), _) => Ordering::Less,
-
             (_, _) => Ordering::Equal,
         }
     }
@@ -684,7 +680,7 @@ impl Node {
     /// Creates a `Try` AST node.
     pub fn try_node<OC, OF>(try_node: Block, catch: OC, finally: OF) -> Self
     where
-        OC: Into<Option<(Option<Local>, Block)>>,
+        OC: Into<Option<(Option<Identifier>, Block)>>,
         OF: Into<Option<Block>>,
     {
         let catch = catch.into();
@@ -766,7 +762,7 @@ impl Node {
             ),
             Self::Spread(ref node) => write!(f, "...{}", node),
             Self::Block(ref block) => block.display(f, indentation),
-            Self::Local(ref s) => Display::fmt(s, f),
+            Self::Identifier(ref s) => Display::fmt(s, f),
             Self::GetConstField(ref ex, ref field) => write!(f, "{}.{}", ex, field),
             Self::GetField(ref ex, ref field) => write!(f, "{}[{}]", ex, field),
             Self::Call(ref ex, ref args) => {

boa/src/syntax/ast/position.rs

@@ -1,6 +1,6 @@
 //! This module implements the `Pos` structure, which represents a position in the source code.
 
-use std::{cmp::Ordering, fmt};
+use std::{cmp::Ordering, fmt, num::NonZeroU32};
 
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
@@ -15,31 +15,31 @@ use serde::{Deserialize, Serialize};
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Position {
     /// Line number.
-    line_number: u64,
+    line_number: NonZeroU32,
     /// Column number.
-    column_number: u64,
+    column_number: NonZeroU32,
 }
 
 impl Position {
     /// Creates a new `Position`.
     #[inline]
-    pub fn new(line_number: u64, column_number: u64) -> Self {
+    pub fn new(line_number: u32, column_number: u32) -> Self {
         Self {
-            line_number,
-            column_number,
+            line_number: NonZeroU32::new(line_number).expect("line number cannot be 0"),
+            column_number: NonZeroU32::new(column_number).expect("column number cannot be 0"),
         }
     }
 
     /// Gets the line number of the position.
     #[inline]
-    pub fn line_number(self) -> u64 {
-        self.line_number
+    pub fn line_number(self) -> u32 {
+        self.line_number.get()
     }
 
     /// Gets the column number of the position.
     #[inline]
-    pub fn column_number(self) -> u64 {
-        self.column_number
+    pub fn column_number(self) -> u32 {
+        self.column_number.get()
     }
 }
 
@@ -124,6 +124,20 @@ impl fmt::Display for Span {
 mod tests {
     use super::{Position, Span};
 
+    /// Checks that we cannot create a position with 0 as the column.
+    #[test]
+    #[should_panic]
+    fn invalid_position_column() {
+        Position::new(10, 0);
+    }
+
+    /// Checks that we cannot create a position with 0 as the line.
+    #[test]
+    #[should_panic]
+    fn invalid_position_line() {
+        Position::new(0, 10);
+    }
+
     /// Checks that the `PartialEq` implementation of `Position` is consistent.
     #[test]
     fn position_equality() {