feat(parser)!: assign node IDs during parsing (#27411)

Assign unique node IDs while the parser constructs AST nodes, reserving zero for Program. IDs follow construction order and may have gaps from discarded nodes or reparsing; semantic analysis still replaces them with dense IDs for its node store.

This is a breaking change because consumers can no longer assume node IDs are unset after parsing.

crates/oxc_ast/src/builder/mod.rs

@@ -123,7 +123,7 @@ pub trait GetAstBuilder<'a> {
 /// AST builder which assigns dummy [`NodeId`]s to AST nodes.
 ///
 /// For use where no `NodeId`s are required on AST nodes as they are built
-/// e.g. parser, because `NodeId`s are assigned later when building `Semantic`.
+/// e.g. generated nodes whose IDs will be assigned when rebuilding `Semantic`.
 pub struct AstBuilder<'a> {
     /// The memory allocator used to allocate AST nodes in the arena.
     allocator: &'a Allocator,

crates/oxc_parser/src/ast_builder.rs

@@ -0,0 +1,48 @@
+use std::cell::Cell;
+
+use oxc_allocator::{Allocator, GetAllocator};
+use oxc_ast::builder::{AstBuild, GetAstBuilder};
+use oxc_syntax::node::NodeId;
+
+/// Assign IDs as nodes are constructed, reserving zero for Program.
+///
+/// IDs follow construction order, rather than visitor order, and may have gaps
+/// from discarded nodes. The counter is deliberately not rewound during
+/// speculation or top-level await reparsing: nodes constructed after an earlier
+/// checkpoint can still survive in the returned AST.
+pub struct ParserAstBuilder<'a> {
+    allocator: &'a Allocator,
+    next_node_id: Cell<usize>,
+}
+
+impl<'a> ParserAstBuilder<'a> {
+    pub fn new(allocator: &'a Allocator) -> Self {
+        Self { allocator, next_node_id: Cell::new(1) }
+    }
+}
+
+impl<'a> AstBuild<'a> for ParserAstBuilder<'a> {
+    #[inline]
+    fn node_id(&self) -> NodeId {
+        let index = self.next_node_id.get();
+        assert!(index <= NodeId::MAX_INDEX, "Too many AST nodes");
+        self.next_node_id.set(index + 1);
+        NodeId::new(index)
+    }
+}
+
+impl<'a> GetAstBuilder<'a> for ParserAstBuilder<'a> {
+    type Builder = Self;
+
+    #[inline]
+    fn builder(&self) -> &Self {
+        self
+    }
+}
+
+impl<'a> GetAllocator<'a> for ParserAstBuilder<'a> {
+    #[inline]
+    fn allocator(&self) -> &'a Allocator {
+        self.allocator
+    }
+}

crates/oxc_parser/src/lib.rs

@@ -66,6 +66,7 @@
 
 use std::any::Any;
 
+mod ast_builder;
 pub mod config;
 mod context;
 mod cursor;
@@ -98,6 +99,7 @@ use oxc_syntax::module_record::ModuleRecord;
 
 pub use crate::lexer::{Kind, Token};
 use crate::{
+    ast_builder::ParserAstBuilder,
     config::{
         LexerConfig, NoTokensParserConfig, ParserConfig, RuntimeParserConfig, TokensParserConfig,
     },
@@ -165,6 +167,10 @@ pub struct ParserReturn<'a> {
     ///
     /// Will be empty (e.g. no statements, directives, etc) if the parser encountered a fatal error.
     ///
+    /// Every node has a unique node ID within this AST. Program has ID zero;
+    /// other IDs follow construction order and may have gaps from discarded nodes.
+    /// Semantic analysis replaces these with its own dense node IDs.
+    ///
     /// ## Validity
     /// It is possible for the AST to be present and semantically invalid. This will happen if
     /// 1. The [`Parser`] encounters a recoverable syntax error
@@ -415,6 +421,9 @@ mod parser_parse {
 
         /// Parse a single [`Expression`].
         ///
+        /// Every node receives a unique, nonzero node ID. IDs follow construction
+        /// order and may have gaps from discarded nodes.
+        ///
         /// # Example
         ///
         /// ```rust
@@ -654,7 +663,7 @@ struct ParserImpl<'a, C: ParserConfig> {
     ctx: Context,
 
     /// Ast builder for creating AST nodes
-    ast: AstBuilder<'a>,
+    ast: ParserAstBuilder<'a>,
 
     /// Module Record Builder
     module_record_builder: ModuleRecordBuilder<'a>,
@@ -690,7 +699,7 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
             prev_token_end: 0,
             state: ParserState::new(),
             ctx: Self::default_context(source_type, options),
-            ast: AstBuilder::new(allocator),
+            ast: ParserAstBuilder::new(allocator),
             module_record_builder: ModuleRecordBuilder::new(allocator, source_type),
             is_ts: source_type.is_typescript(),
         }
@@ -847,7 +856,9 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
             hashbang,
             directives,
             statements,
-            self,
+            // The default builder assigns ID zero (NodeId::ROOT) to Program.
+            // ParserAstBuilder starts at one, keeping every child ID distinct.
+            &AstBuilder::new(self.allocator()),
         )
     }
 
@@ -1017,10 +1028,10 @@ impl<'a, C: ParserConfig> GetAllocator<'a> for ParserImpl<'a, C> {
 }
 
 impl<'a, C: ParserConfig> GetAstBuilder<'a> for ParserImpl<'a, C> {
-    type Builder = AstBuilder<'a>;
+    type Builder = ParserAstBuilder<'a>;
 
     #[inline]
-    fn builder(&self) -> &AstBuilder<'a> {
+    fn builder(&self) -> &ParserAstBuilder<'a> {
         &self.ast
     }
 }

crates/oxc_parser/tests/node_ids.rs

@@ -0,0 +1,151 @@
+use rustc_hash::FxHashSet;
+
+use oxc_allocator::Allocator;
+use oxc_ast::{AstKind, AstType, ast::Program};
+use oxc_ast_visit::Visit;
+use oxc_parser::{
+    ParseOptions, Parser,
+    config::{NoTokensParserConfig, ParserConfig, RuntimeParserConfig, TokensParserConfig},
+};
+use oxc_span::{GetSpan, SourceType, Span};
+use oxc_syntax::node::NodeId;
+
+#[derive(Default)]
+struct NodeIds {
+    seen: FxHashSet<NodeId>,
+    nodes: Vec<(AstType, Span, NodeId)>,
+}
+
+impl<'a> Visit<'a> for NodeIds {
+    fn enter_node(&mut self, kind: AstKind<'a>) {
+        let id = kind.node_id();
+        if matches!(kind, AstKind::Program(_)) {
+            assert_eq!(id, NodeId::ROOT);
+        } else {
+            assert_ne!(id, NodeId::DUMMY, "Unassigned ID for {kind:?}");
+        }
+        assert!(self.seen.insert(id), "Duplicate ID for {kind:?}");
+        self.nodes.push((kind.ty(), kind.span(), id));
+    }
+}
+
+fn collect(program: &Program<'_>) -> Vec<(AstType, Span, NodeId)> {
+    let mut ids = NodeIds::default();
+    ids.visit_program(program);
+    ids.nodes
+}
+
+fn check<C: ParserConfig>(source: &str, source_type: SourceType, config: C, preserve_parens: bool) {
+    let allocator = Allocator::default();
+    let parsed = Parser::new(&allocator, source, source_type)
+        .with_config(config)
+        .with_options(ParseOptions { preserve_parens, ..ParseOptions::default() })
+        .parse();
+    assert!(parsed.diagnostics.is_empty(), "{source}: {:?}", parsed.diagnostics);
+    assert!(!parsed.fatal_error, "{source}");
+    let nodes = collect(&parsed.program);
+    assert_eq!(nodes[0].0, AstType::Program);
+}
+
+#[test]
+fn assigns_ids_in_all_parser_configs() {
+    let cases = [
+        ("", SourceType::mjs()),
+        ("/* comments only */", SourceType::mjs()),
+        ("#!/usr/bin/env node\n'use strict';\nvalue;", SourceType::mjs()),
+        ("function f([first, ...rest]) { return [first, , `${rest.length}`]; }", SourceType::mjs()),
+        (
+            "type Pair<T> = [left: T, right: T]; const pair: Pair<number> = [1, 2];",
+            SourceType::ts(),
+        ),
+        (
+            "const element = <Component prop={value}><>{/* comment */}{value}</></Component>;",
+            SourceType::jsx(),
+        ),
+        ("const element: JSX.Element = <Component<string> prop={value} />;", SourceType::tsx()),
+    ];
+    for (source, source_type) in cases {
+        for preserve_parens in [false, true] {
+            check(source, source_type, NoTokensParserConfig, preserve_parens);
+            check(source, source_type, TokensParserConfig, preserve_parens);
+            for tokens in [false, true] {
+                check(source, source_type, RuntimeParserConfig::new(tokens), preserve_parens);
+            }
+        }
+    }
+}
+
+#[test]
+fn ids_survive_speculation_and_cover_grammar() {
+    for source in [
+        "const f = (a = call()) => a; const value = (a + b);",
+        "const f = async (a = call()) => a; const value = async(a + b);",
+        "const f = <T>(a: T): T => a; const value = (a as number);",
+        "({ a: [b = c, ...rest], d = e } = value); [first, , ...rest] = value;",
+        "const value = ((a ? (b): string => b : c));",
+    ] {
+        for preserve_parens in [false, true] {
+            check(source, SourceType::ts(), NoTokensParserConfig, preserve_parens);
+        }
+    }
+}
+
+#[test]
+fn ids_survive_unambiguous_await_reparsing() {
+    for source in [
+        "before(); await /x/u; between(); await /y/g; export {}; tail();",
+        "before(); await /a/ /b/g; between(); await /x/u; export {}; tail();",
+        "await\nawait /x/u; export {};",
+        "const x = await /x/u; between(); const y = await /y/g; export {};",
+        "#!/usr/bin/env node\n'use strict';\nawait /* regexp */ /x/u; export {};",
+    ] {
+        check(source, SourceType::unambiguous(), NoTokensParserConfig, true);
+        check(source, SourceType::unambiguous(), TokensParserConfig, true);
+    }
+}
+
+#[test]
+fn assigns_ids_to_recovered_nodes() {
+    for (source, source_type) in [
+        ("interface I { [key: boolean]: string; }", SourceType::ts()),
+        ("interface I extends (value) {}", SourceType::ts()),
+        ("const value = <><div /></div>;", SourceType::jsx()),
+    ] {
+        let allocator = Allocator::default();
+        let parsed = Parser::new(&allocator, source, source_type).parse();
+        assert!(!parsed.fatal_error, "{source}: {:?}", parsed.diagnostics);
+        assert!(!parsed.diagnostics.is_empty(), "{source}");
+        assert!(collect(&parsed.program).len() > 1);
+    }
+}
+
+#[test]
+fn fatal_errors_return_only_the_root_node() {
+    let allocator = Allocator::default();
+    let parsed = Parser::new(&allocator, "before(); const value = ;", SourceType::mjs()).parse();
+    assert!(parsed.fatal_error);
+    assert_eq!(collect(&parsed.program), [(AstType::Program, Span::default(), NodeId::ROOT)]);
+}
+
+#[test]
+fn expression_parser_assigns_nonzero_ids() {
+    for source in ["left + right", "(a + b)", "({ a: [b = c, ...rest] } = value)"] {
+        let allocator = Allocator::default();
+        let expression =
+            Parser::new(&allocator, source, SourceType::mjs()).parse_expression().unwrap();
+        let mut ids = NodeIds::default();
+        ids.visit_expression(&expression);
+        assert_ne!(ids.nodes, []);
+    }
+}
+
+#[test]
+fn each_parse_starts_a_new_id_sequence() {
+    let allocator = Allocator::default();
+    let source = "'use strict'; function f(a) { return a + 1; }";
+    let parse = || Parser::new(&allocator, source, SourceType::mjs()).parse().program;
+    let first = parse();
+    Parser::new(&allocator, "unrelated();", SourceType::mjs()).parse();
+    let second = parse();
+    assert_eq!(collect(&first), collect(&second));
+}

crates/oxc_syntax/src/node.rs

@@ -55,7 +55,9 @@ impl SemanticId for NodeId {}
 
 /// Get the [`NodeId`] of given AST node.
 ///
-/// Only use this method on a post-semantic AST where [`NodeId`]s are always defined.
+/// Parser-assigned IDs are unique within the parsed AST but may have gaps and
+/// do not follow visitor order. Semantic analysis replaces them with dense IDs
+/// that index its node store. Synthetically-created nodes may have [`NodeId::DUMMY`].
 pub trait GetNodeId {
     /// Get the [`NodeId`] for an AST node.
     fn node_id(&self) -> NodeId;

tasks/coverage/src/driver.rs

@@ -6,10 +6,10 @@ use oxc::{
     CompilerInterface,
     allocator::Allocator,
     ast::{
-        Comment,
+        AstKind, Comment,
         ast::{Program, RegExpLiteral},
     },
-    ast_visit::{VisitJs, walk_js},
+    ast_visit::{Visit, walk},
     codegen::{CodegenOptions, CodegenReturn},
     diagnostics::{Diagnostics, OxcDiagnostic},
     minifier::CompressOptions,
@@ -17,6 +17,7 @@ use oxc::{
     regular_expression::{LiteralParser, Options},
     semantic::SemanticBuilderReturn,
     span::{ContentEq, SourceType, Span},
+    syntax::node::NodeId,
     transformer::{TransformOptions, TransformerReturn},
 };
 use oxc_tasks_transform_checker::{check_semantic_after_transform, check_semantic_ids};
@@ -173,11 +174,17 @@ struct CheckASTNodes<'a> {
     driver: &'a mut Driver,
     source_text: &'a str,
     allocator: Allocator,
+    node_ids: FxHashSet<NodeId>,
 }
 
 impl<'a> CheckASTNodes<'a> {
     fn new(driver: &'a mut Driver, source_text: &'a str) -> Self {
-        Self { driver, source_text, allocator: Allocator::default() }
+        Self {
+            driver,
+            source_text,
+            allocator: Allocator::default(),
+            node_ids: FxHashSet::default(),
+        }
     }
 
     fn check(&mut self, program: &Program<'a>) {
@@ -185,7 +192,19 @@ impl<'a> CheckASTNodes<'a> {
     }
 }
 
-impl<'a> VisitJs<'a> for CheckASTNodes<'a> {
+impl<'a> Visit<'a> for CheckASTNodes<'a> {
+    fn enter_node(&mut self, kind: AstKind<'a>) {
+        let id = kind.node_id();
+        // These are AST invariants, including for recovered syntax errors.
+        // Reporting them as diagnostics would let negative fixtures hide a failure.
+        assert_eq!(
+            id == NodeId::ROOT,
+            matches!(kind, AstKind::Program(_)),
+            "Only Program may have parser node ID zero: {kind:?}"
+        );
+        assert!(self.node_ids.insert(id), "Duplicate parser node ID: {kind:?}");
+    }
+
     // TODO: This is too slow
     // fn visit_span(&mut self, span: &Span) {
     // let Span { start, end, .. } = span;
@@ -198,7 +217,7 @@ impl<'a> VisitJs<'a> for CheckASTNodes<'a> {
 
     /// Idempotency test for printing regular expressions.
     fn visit_reg_exp_literal(&mut self, literal: &RegExpLiteral<'a>) {
-        walk_js::walk_reg_exp_literal(self, literal);
+        walk::walk_reg_exp_literal(self, literal);
 
         let Some(pattern) = &literal.regex.pattern.pattern else {
             return;