fix(semantic): do not declare a Symbol for an ModuleDeclaration where the id is not a BindingIdentifier (#10343)

In previous implementations, a symbol was declared for `ModuleDeclaration` but only inserted when `name` was a `BindingIdentifier`. This is incorrect and also causes a potential performance impact. See https://github.com/oxc-project/oxc/blob/bae53b9606b62fce58b4101497849997b0dc07b3/crates/oxc_semantic/src/stats.rs.

So we should only declare a symbol when its name is a `BindingIdentifier` because only `BindingIdentifier` can be inserted with a symbol ID.

Author: Dunqing

crates/oxc_semantic/src/binder.rs

@@ -4,7 +4,6 @@ use std::ptr;
 
 use oxc_ast::{AstKind, ast::*};
 use oxc_ecmascript::{BoundNames, IsSimpleParameterList};
-use oxc_span::GetSpan;
 use oxc_syntax::{
     scope::{ScopeFlags, ScopeId},
     symbol::SymbolFlags,
@@ -385,23 +384,22 @@ impl<'a> Binder<'a> for TSEnumMember<'a> {
 impl<'a> Binder<'a> for TSModuleDeclaration<'a> {
     fn bind(&self, builder: &mut SemanticBuilder) {
         // do not bind `global` for `declare global { ... }`
-        if self.kind == TSModuleDeclarationKind::Global {
+        if self.kind.is_global() {
             return;
         }
+        let TSModuleDeclarationName::Identifier(id) = &self.id else { return };
 
         // At declaration time a module has no value declaration it is only when a value declaration
         // is made inside a the scope of a module that the symbol is modified
         let ambient = if self.declare { SymbolFlags::Ambient } else { SymbolFlags::None };
         let symbol_id = builder.declare_symbol(
-            self.id.span(),
-            self.id.name().as_str(),
+            id.span,
+            &id.name,
             SymbolFlags::NameSpaceModule | ambient,
             SymbolFlags::None,
         );
 
-        if let TSModuleDeclarationName::Identifier(id) = &self.id {
-            id.symbol_id.set(Some(symbol_id));
-        }
+        id.set_symbol_id(symbol_id);
     }
 }
 

crates/oxc_semantic/tests/fixtures/typescript-eslint/ts-module/import.snap

@@ -13,7 +13,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/ts-module/impor
         "symbols": [
           {
             "flags": "SymbolFlags(Import)",
-            "id": 1,
+            "id": 0,
             "name": "bar",
             "node": "ImportSpecifier(bar)",
             "references": []
@@ -24,14 +24,6 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/ts-module/impor
     "flags": "ScopeFlags(StrictMode | Top)",
     "id": 0,
     "node": "Program",
-    "symbols": [
-      {
-        "flags": "SymbolFlags(NameSpaceModule | Ambient)",
-        "id": 0,
-        "name": "foo",
-        "node": "TSModuleDeclaration(foo)",
-        "references": []
-      }
-    ]
+    "symbols": []
   }
 ]

crates/oxc_semantic/tests/integration/symbols.rs

@@ -423,11 +423,6 @@ fn test_module_like_declarations() {
         .contains_flags(SymbolFlags::NameSpaceModule)
         .test();
 
-    SemanticTester::ts(r#"module "A" { export const x = 1; }"#)
-        .has_root_symbol("A")
-        .contains_flags(SymbolFlags::NameSpaceModule)
-        .test();
-
     let test = SemanticTester::ts("declare global { interface Window { x: number; } }");
     let semantic = test.build();
     let global = semantic.scoping().symbol_names().find(|name| *name == "global");

tasks/coverage/snapshots/semantic_babel.snap

@@ -858,10 +858,7 @@ after transform: SymbolId(0): SymbolFlags(RegularEnum)
 rebuilt        : SymbolId(0): SymbolFlags(FunctionScopedVariable)
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/estree-compat/shorthand-ambient-module/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["hot-new-module"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []
 
@@ -1038,10 +1035,7 @@ after transform: ScopeId(0): ["A", "C", "D", "foo"]
 rebuilt        : ScopeId(0): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/not-top-level/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["m"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []
 
@@ -1314,26 +1308,17 @@ after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/module-namespace/declare-shorthand/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["m"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/module-namespace/global-in-module/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["m"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/module-namespace/global-in-module-babel-7/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["m"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []
 
@@ -1345,15 +1330,15 @@ semantic error: Ambient modules cannot be nested in other modules or namespaces.
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/module-namespace/head-declare/input.ts
 semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["M", "N", "m"]
+after transform: ScopeId(0): ["M", "N"]
 rebuilt        : ScopeId(0): []
 Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1), ScopeId(3), ScopeId(4), ScopeId(5)]
 rebuilt        : ScopeId(0): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/module-namespace/head-declare-babel-7/input.ts
 semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["M", "N", "m"]
+after transform: ScopeId(0): ["M", "N"]
 rebuilt        : ScopeId(0): []
 Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1), ScopeId(3), ScopeId(4), ScopeId(5)]
@@ -1573,21 +1558,15 @@ after transform: SymbolId(0): SymbolFlags(RegularEnum)
 rebuilt        : SymbolId(0): SymbolFlags(FunctionScopedVariable)
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/export-func-in-declare-module/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["m2"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1), ScopeId(6)]
 rebuilt        : ScopeId(0): []
 Unresolved references mismatch:
 after transform: ["X"]
 rebuilt        : []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/export-import-in-declare-module/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["~popsicle/dist/common"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []
 
@@ -1646,7 +1625,7 @@ rebuilt        : SymbolId(0): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/module-declaration-var/input.js
 semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["bar", "foo"]
+after transform: ScopeId(0): ["foo"]
 rebuilt        : ScopeId(0): []
 Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1), ScopeId(2)]
@@ -2125,16 +2104,13 @@ after transform: SymbolId(3): [Span { start: 81, end: 82 }, Span { start: 170, e
 rebuilt        : SymbolId(3): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/redeclaration-in-different-module/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["test", "test/submodule"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1), ScopeId(2), ScopeId(3)]
 rebuilt        : ScopeId(0): []
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/redeclaration-in-different-module-and-top-level/input.ts
 semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["JSONSchema7", "foo", "fooBar", "test/submodule", "test/submodule2"]
+after transform: ScopeId(0): ["JSONSchema7", "foo", "fooBar"]
 rebuilt        : ScopeId(0): ["foo", "fooBar"]
 Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1), ScopeId(2), ScopeId(3)]
@@ -2144,10 +2120,7 @@ after transform: SymbolId(1): [ReferenceId(1), ReferenceId(4)]
 rebuilt        : SymbolId(0): [ReferenceId(0)]
 
 tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/redeclaration-in-nested-module/input.ts
-semantic error: Bindings mismatch:
-after transform: ScopeId(0): ["test/sub1"]
-rebuilt        : ScopeId(0): []
-Scope children mismatch:
+semantic error: Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1)]
 rebuilt        : ScopeId(0): []