Document the AST (#2377)

As promised in #2319 (comment). There are still some style inconsistencies (which require a bit more time and effort), but having the whole module documented is a lot better for clarity.

boa_engine/src/bytecompiler/mod.rs

@@ -8,20 +8,20 @@ use crate::{
             access::{PrivatePropertyAccess, PropertyAccess, PropertyAccessField},
             literal::{self, TemplateElement},
             operator::{
-                assign::{op::AssignOp, AssignTarget},
-                binary::op::{ArithmeticOp, BinaryOp, BitwiseOp, LogicalOp, RelationalOp},
-                unary::op::UnaryOp,
+                assign::{AssignOp, AssignTarget},
+                binary::{ArithmeticOp, BinaryOp, BitwiseOp, LogicalOp, RelationalOp},
+                unary::UnaryOp,
             },
             Call, Identifier, New,
         },
         function::{
             ArrowFunction, AsyncFunction, AsyncGenerator, Class, ClassElement, FormalParameterList,
             Function, Generator,
         },
-        pattern::{Pattern, PatternArrayElement, PatternObjectElement},
+        pattern::{ArrayPatternElement, ObjectPatternElement, Pattern},
         property::{MethodDefinition, PropertyDefinition, PropertyName},
         statement::{
-            iteration::{for_loop::ForLoopInitializer, IterableLoopInitializer},
+            iteration::{ForLoopInitializer, IterableLoopInitializer},
             Block, DoWhileLoop, ForInLoop, ForLoop, ForOfLoop, LabelledItem, WhileLoop,
         },
         Declaration, Expression, Statement, StatementList, StatementListItem,
@@ -1221,7 +1221,7 @@ impl<'b> ByteCompiler<'b> {
                 use_expr,
             )?,
             Expression::Conditional(op) => {
-                self.compile_expr(op.cond(), true)?;
+                self.compile_expr(op.condition(), true)?;
                 let jelse = self.jump_if_false();
                 self.compile_expr(op.if_true(), true)?;
                 let exit = self.jump();
@@ -1259,7 +1259,7 @@ impl<'b> ByteCompiler<'b> {
             Expression::This => {
                 self.access_get(Access::This, use_expr)?;
             }
-            Expression::Spread(spread) => self.compile_expr(spread.val(), true)?,
+            Expression::Spread(spread) => self.compile_expr(spread.target(), true)?,
             Expression::Function(function) => {
                 self.function(function.into(), NodeKind::Expression, use_expr)?;
             }
@@ -1299,15 +1299,15 @@ impl<'b> ByteCompiler<'b> {
                 }
             }
             Expression::Await(expr) => {
-                self.compile_expr(expr.expr(), true)?;
+                self.compile_expr(expr.target(), true)?;
                 self.emit_opcode(Opcode::Await);
                 self.emit_opcode(Opcode::GeneratorNext);
                 if !use_expr {
                     self.emit_opcode(Opcode::Pop);
                 }
             }
             Expression::Yield(r#yield) => {
-                if let Some(expr) = r#yield.expr() {
+                if let Some(expr) = r#yield.target() {
                     self.compile_expr(expr, true)?;
                 } else {
                     self.emit_opcode(Opcode::PushUndefined);
@@ -1616,17 +1616,17 @@ impl<'b> ByteCompiler<'b> {
         for_in_loop: &ForInLoop,
         label: Option<Sym>,
     ) -> JsResult<()> {
-        let init_bound_names = for_in_loop.init().bound_names();
+        let init_bound_names = for_in_loop.initializer().bound_names();
         if init_bound_names.is_empty() {
-            self.compile_expr(for_in_loop.expr(), true)?;
+            self.compile_expr(for_in_loop.target(), true)?;
         } else {
             self.context.push_compile_time_environment(false);
             let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);
 
             for name in init_bound_names {
                 self.context.create_mutable_binding(name, false);
             }
-            self.compile_expr(for_in_loop.expr(), true)?;
+            self.compile_expr(for_in_loop.target(), true)?;
 
             let (num_bindings, compile_environment) = self.context.pop_compile_time_environment();
             let index_compile_environment = self.push_compile_environment(compile_environment);
@@ -1646,7 +1646,7 @@ impl<'b> ByteCompiler<'b> {
         let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);
         let exit = self.emit_opcode_with_operand(Opcode::ForInLoopNext);
 
-        match for_in_loop.init() {
+        match for_in_loop.initializer() {
             IterableLoopInitializer::Identifier(ident) => {
                 self.context.create_mutable_binding(*ident, true);
                 let binding = self.context.set_mutable_binding(*ident);
@@ -2106,7 +2106,7 @@ impl<'b> ByteCompiler<'b> {
                 }
             }
             Statement::Throw(throw) => {
-                self.compile_expr(throw.expr(), true)?;
+                self.compile_expr(throw.target(), true)?;
                 self.emit(Opcode::Throw, &[]);
             }
             Statement::Switch(switch) => {
@@ -2153,7 +2153,7 @@ impl<'b> ByteCompiler<'b> {
                 self.emit_opcode(Opcode::PopEnvironment);
             }
             Statement::Return(ret) => {
-                if let Some(expr) = ret.expr() {
+                if let Some(expr) = ret.target() {
                     self.compile_expr(expr, true)?;
                 } else {
                     self.emit(Opcode::PushUndefined, &[]);
@@ -2336,14 +2336,14 @@ impl<'b> ByteCompiler<'b> {
         }
 
         let (call, kind) = match callable {
-            Callable::Call(call) => match call.expr() {
+            Callable::Call(call) => match call.function() {
                 Expression::Identifier(ident) if *ident == Sym::EVAL => (call, CallKind::CallEval),
                 _ => (call, CallKind::Call),
             },
             Callable::New(new) => (new.call(), CallKind::New),
         };
 
-        match call.expr() {
+        match call.function() {
             Expression::PropertyAccess(access) => {
                 self.compile_expr(access.target(), true)?;
                 if kind == CallKind::Call {
@@ -2453,14 +2453,12 @@ impl<'b> ByteCompiler<'b> {
                 let rest_exits = pattern.has_rest();
 
                 for binding in pattern.bindings() {
-                    use PatternObjectElement::{
-                        AssignmentPropertyAccess, AssignmentRestPropertyAccess, Empty, Pattern,
+                    use ObjectPatternElement::{
+                        AssignmentPropertyAccess, AssignmentRestPropertyAccess, Pattern,
                         RestProperty, SingleName,
                     };
 
                     match binding {
-                        // ObjectBindingPattern : { }
-                        Empty => {}
                         //  SingleNameBinding : BindingIdentifier Initializer[opt]
                         SingleName {
                             ident,
@@ -2605,14 +2603,12 @@ impl<'b> ByteCompiler<'b> {
                 self.emit_opcode(Opcode::InitIterator);
 
                 for binding in pattern.bindings().iter() {
-                    use PatternArrayElement::{
-                        Elision, Empty, Pattern, PatternRest, PropertyAccess, PropertyAccessRest,
+                    use ArrayPatternElement::{
+                        Elision, Pattern, PatternRest, PropertyAccess, PropertyAccessRest,
                         SingleName, SingleNameRest,
                     };
 
                     match binding {
-                        // ArrayBindingPattern : [ ]
-                        Empty => {}
                         // ArrayBindingPattern : [ Elision ]
                         Elision => {
                             self.emit_opcode(Opcode::IteratorNext);

boa_engine/src/lib.rs

@@ -40,7 +40,7 @@
     unused_lifetimes,
     unreachable_pub,
     trivial_numeric_casts,
-    // rustdoc,
+    rustdoc::broken_intra_doc_links,
     missing_debug_implementations,
     missing_copy_implementations,
     deprecated_in_future,
@@ -49,7 +49,7 @@
     rust_2018_compatibility,
     rust_2018_idioms,
     future_incompatible,
-    nonstandard_style,
+    nonstandard_style
 )]
 #![allow(
     clippy::missing_inline_in_public_items,

boa_engine/src/syntax/ast/declaration/mod.rs

@@ -1,3 +1,19 @@
+//! The [`Declaration`] Parse Node, as defined by the [spec].
+//!
+//! Javascript declarations include:
+//! - [Lexical][lex] declarations (`let`, `const`).
+//! - [Function][fun] declarations (`function`, `async function`).
+//! - [Class][class] declarations.
+//!
+//! See [*Difference between statements and declarations*][diff] for an explanation on why `Declaration`s
+//! and `Statement`s are distinct nodes.
+//!
+//! [spec]: https://tc39.es/ecma262/#prod-Declaration
+//! [lex]: https://tc39.es/ecma262/#prod-LexicalDeclaration
+//! [fun]: https://tc39.es/ecma262/#prod-HoistableDeclaration
+//! [class]: https://tc39.es/ecma262/#prod-ClassDeclaration
+//! [diff]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements#difference_between_statements_and_declarations
+
 use super::{
     expression::Identifier,
     function::{AsyncFunction, AsyncGenerator, Class, Function, Generator},
@@ -10,6 +26,9 @@ mod variable;
 
 pub use variable::*;
 
+/// The `Declaration` Parse Node.
+///
+/// See the [module level documentation][self] for more information.
 #[cfg_attr(feature = "deser", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Debug, PartialEq)]
 pub enum Declaration {

boa_engine/src/syntax/ast/declaration/variable.rs

@@ -12,18 +12,34 @@ use boa_interner::{Interner, ToInternedString};
 
 use super::Declaration;
 
-/// A [`var`][var] declaration list, also called [`VariableDeclarationList`][vardecl]
-/// in the spec.
+/// A [`var`][var] statement, also called [`VariableStatement`][varstmt] in the spec.
 ///
 /// The scope of a variable declared with `var` is its current execution context, which is either
 /// the enclosing function or, for variables declared outside any function, global. If you
 /// re-declare a JavaScript variable, it will not lose its value.
 ///
+/// Although a bit confusing, `VarDeclaration`s are not considered [`Declaration`]s by the spec.
+/// This is partly because it has very different semantics from `let` and `const` declarations, but
+/// also because a `var` statement can be labelled just like any other [`Statement`]:
+///
+/// ```javascript
+/// label: var a = 5;
+/// a;
+/// ```
+///
+/// returns `5` as the value of the statement list, while:
+///
+/// ```javascript
+/// label: let a = 5;
+/// a;
+/// ```
+/// throws a `SyntaxError`.
+///
 /// `var` declarations, wherever they occur, are processed before any code is executed. This is
 /// called <code>[hoisting]</code>.
 ///
 /// [var]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/var
-/// [vardecl]: https://tc39.es/ecma262/#prod-VariableStatement
+/// [varstmt]: https://tc39.es/ecma262/#prod-VariableStatement
 /// [hoisting]: https://developer.mozilla.org/en-US/docs/Glossary/Hoisting
 #[cfg_attr(feature = "deser", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Debug, PartialEq)]
@@ -155,6 +171,7 @@ impl ToInternedString for VariableList {
     }
 }
 
+/// The error returned by the [`VariableList::try_from`] function.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct TryFromVariableListError(());
 
@@ -280,7 +297,9 @@ impl Variable {
 #[cfg_attr(feature = "deser", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Debug, PartialEq)]
 pub enum Binding {
+    /// A single identifier binding.
     Identifier(Identifier),
+    /// A pattern binding.
     Pattern(Pattern),
 }
 

boa_engine/src/syntax/ast/expression/access.rs

@@ -1,10 +1,27 @@
+//! Property access expressions, as defined by the [spec].
+//!
+//! [Property access expressions][access] provide two ways to access properties of an object: *dot notation*
+//! and *bracket notation*.
+//! - *Dot notation* is mostly used when the name of the property is static, and a valid Javascript
+//! identifier e.g. `obj.prop`, `arr.$val`.
+//! - *Bracket notation* is used when the name of the property is either variable, not a valid
+//! identifier or a symbol e.g. `arr[var]`, `arr[5]`, `arr[Symbol.iterator]`.
+//!
+//! [spec]: https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-property-accessors
+//! [access]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Property_Accessors
+
 use crate::syntax::ast::{expression::Expression, ContainsSymbol};
 use boa_interner::{Interner, Sym, ToInternedString};
 
+/// A property access field.
+///
+/// See the [module level documentation][self] for more information.
 #[cfg_attr(feature = "deser", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Debug, PartialEq)]
 pub enum PropertyAccessField {
+    /// A constant property field, such as `x.prop`.
     Const(Sym),
+    /// An expression property field, such as `x["val"]`.
     Expr(Box<Expression>),
 }
 
@@ -38,28 +55,9 @@ impl From<Expression> for PropertyAccessField {
     }
 }
 
-/// This property accessor provides access to an object's properties by using the
-/// [bracket notation][mdn].
-///
-/// In the `object[property_name]` syntax, the `property_name` is just a string or
-/// [Symbol][symbol]. So, it can be any string, including '1foo', '!bar!', or even ' ' (a
-/// space).
-///
-/// One can think of an object as an associative array (a.k.a. map, dictionary, hash, lookup
-/// table). The keys in this array are the names of the object's properties.
-///
-/// It's typical when speaking of an object's properties to make a distinction between
-/// properties and methods. However, the property/method distinction is little more than a
-/// convention. A method is simply a property that can be called (for example, if it has a
-/// reference to a Function instance as its value).
-///
-/// More information:
-///  - [ECMAScript reference][spec]
-///  - [MDN documentation][mdn]
+/// A property access expression.
 ///
-/// [spec]: https://tc39.es/ecma262/#sec-property-accessors
-/// [symbol]: https://developer.mozilla.org/en-US/docs/Glossary/Symbol
-/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Property_accessors#Bracket_notation
+/// See the [module level documentation][self] for more information.
 #[cfg_attr(feature = "deser", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Debug, PartialEq)]
 pub struct PropertyAccess {
@@ -68,11 +66,13 @@ pub struct PropertyAccess {
 }
 
 impl PropertyAccess {
+    /// Gets the target object of the property access.
     #[inline]
     pub fn target(&self) -> &Expression {
         &self.target
     }
 
+    /// Gets the accessed field of the target object.
     #[inline]
     pub fn field(&self) -> &PropertyAccessField {
         &self.field
@@ -121,14 +121,12 @@ impl From<PropertyAccess> for Expression {
     }
 }
 
-/// This property accessor provides access to an class object's private fields.
+/// An access expression to a class object's [private fields][mdn].
 ///
-/// This expression can be described as ` MemberExpression.PrivateIdentifier`
-/// Example: `this.#a`
+/// Private property accesses differ slightly from plain property accesses, since the accessed
+/// property must be prefixed by `#`, and the bracket notation is not allowed e.g. `this.#a`.
 ///
-/// More information:
-///  - [ECMAScript reference][spec]
-///  - [MDN documentation][mdn]
+/// This expression corresponds to the [`MemberExpression.PrivateIdentifier`][spec] production.
 ///
 /// [spec]: https://tc39.es/ecma262/#prod-MemberExpression
 /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields
@@ -190,11 +188,10 @@ impl From<PrivatePropertyAccess> for Expression {
     }
 }
 
-/// The `super` keyword is used to access fields on an object's parent.
+/// A property access of an object's parent, as defined by the [spec].
 ///
-/// More information:
-///  - [ECMAScript reference][spec]
-///  - [MDN documentation][mdn]
+/// A `SuperPropertyAccess` is much like a regular [`PropertyAccess`], but where its `target` object
+/// is not a regular object, but a reference to the parent object of the current object ([`super`][mdn]).
 ///
 /// [spec]: https://tc39.es/ecma262/#prod-SuperProperty
 /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/super