refactor(semantic): move Counts code into counter module (#5710)

Pure refactor. Move code for counting nodes etc, and verifying counts, into `counter` module.

Cargo.toml

@@ -147,6 +147,7 @@ memoffset = "0.9.1"
 miette = { version = "7.2.0", features = ["fancy-no-syscall"] }
 mimalloc = "0.1.43"
 mime_guess = "2.0.5"
+more-asserts = "0.3.1"
 nonmax = "0.5.5"
 num-bigint = "0.4.6"
 num-traits = "0.2.19"

crates/oxc_semantic/Cargo.toml

@@ -30,6 +30,7 @@ oxc_syntax = { workspace = true }
 assert-unchecked = { workspace = true }
 indexmap = { workspace = true }
 itertools = { workspace = true }
+more-asserts = { workspace = true }
 phf = { workspace = true, features = ["macros"] }
 rustc-hash = { workspace = true }
 serde = { workspace = true, features = ["derive"], optional = true }

crates/oxc_semantic/src/builder.rs

@@ -231,8 +231,7 @@ impl<'a> SemanticBuilder<'a> {
             // Avoiding this growth produces up to 30% perf boost on our benchmarks.
             // TODO: It would be even more efficient to calculate counts in parser to avoid
             // this extra AST traversal.
-            let mut counts = Counts::default();
-            counts.visit_program(program);
+            let counts = Counts::count(program);
             self.nodes.reserve(counts.nodes);
             self.scope.reserve(counts.scopes);
             self.symbols.reserve(counts.symbols, counts.references);
@@ -241,16 +240,16 @@ impl<'a> SemanticBuilder<'a> {
             self.visit_program(program);
 
             // Check that estimated counts accurately
-            debug_assert_eq!(self.nodes.len(), counts.nodes);
-            debug_assert_eq!(self.scope.len(), counts.scopes);
-            debug_assert_eq!(self.symbols.references.len(), counts.references);
-            // `Counts` may overestimate number of symbols, because multiple `BindingIdentifier`s
-            // can result in only a single symbol.
-            // e.g. `var x; var x;` = 2 x `BindingIdentifier` but 1 x symbol.
-            // This is not a big problem - allocating a `Vec` with excess capacity is cheap.
-            // It's allocating with *not enough* capacity which is costly, as then the `Vec`
-            // will grow and reallocate.
-            debug_assert!(self.symbols.len() <= counts.symbols);
+            #[cfg(debug_assertions)]
+            {
+                let actual_counts = Counts {
+                    nodes: self.nodes.len(),
+                    scopes: self.scope.len(),
+                    symbols: self.symbols.len(),
+                    references: self.symbols.references.len(),
+                };
+                Counts::assert_accurate(&actual_counts, &counts);
+            }
 
             // Checking syntax error on module record requires scope information from the previous AST pass
             if self.check_syntax_error {

crates/oxc_semantic/src/counter.rs

@@ -4,8 +4,12 @@
 
 use std::cell::Cell;
 
+use more_asserts::assert_le;
+
 use oxc_ast::{
-    ast::{BindingIdentifier, IdentifierReference, TSEnumMemberName, TSModuleDeclarationName},
+    ast::{
+        BindingIdentifier, IdentifierReference, Program, TSEnumMemberName, TSModuleDeclarationName,
+    },
     visit::walk::{walk_ts_enum_member_name, walk_ts_module_declaration_name},
     AstKind, Visit,
 };
@@ -19,6 +23,27 @@ pub(crate) struct Counts {
     pub references: usize,
 }
 
+impl Counts {
+    pub fn count(program: &Program) -> Self {
+        let mut counts = Counts::default();
+        counts.visit_program(program);
+        counts
+    }
+
+    pub fn assert_accurate(actual: &Self, estimated: &Self) {
+        assert_eq!(actual.nodes, estimated.nodes, "nodes count mismatch");
+        assert_eq!(actual.scopes, estimated.scopes, "scopes count mismatch");
+        assert_eq!(actual.references, estimated.references, "references count mismatch");
+        // `Counts` may overestimate number of symbols, because multiple `BindingIdentifier`s
+        // can result in only a single symbol.
+        // e.g. `var x; var x;` = 2 x `BindingIdentifier` but 1 x symbol.
+        // This is not a big problem - allocating a `Vec` with excess capacity is cheap.
+        // It's allocating with *not enough* capacity which is costly, as then the `Vec`
+        // will grow and reallocate.
+        assert_le!(actual.symbols, estimated.symbols, "symbols count mismatch");
+    }
+}
+
 impl<'a> Visit<'a> for Counts {
     #[inline]
     fn enter_node(&mut self, _: AstKind<'a>) {