fix(semantic): track ambient contexts in SemanticBuilder (#24327)

## Summary

- track TypeScript ambient contexts using a nesting depth in `SemanticBuilder`
- enter ambient context for declaration files and `declare` declarations, including declarations without their own scope
- replace namespace-based and ancestry-based ambient detection
- use the ambient context for reserved keywords, global augmentations, class heritage, computed class members, and bodyless accessors
- extend conformance coverage for ambient variables, properties, functions, and classes

This approach was discussed with overlookmotel yesterday after some brainstorming!

crates/oxc_semantic/src/builder.rs

@@ -84,6 +84,8 @@ pub struct SemanticBuilder<'a> {
     pub(crate) current_function_node_id: NodeId,
     pub(crate) module_instance_state_cache: FxHashMap<Address, ModuleInstanceState>,
     current_reference_flags: ReferenceFlags,
+    /// Nesting depth of TypeScript ambient contexts.
+    ambient_depth: u32,
     /// Symbols that have been hoisted out of a scope (e.g. `var` declarations hoisted to
     /// the enclosing function scope, or Annex B function declarations hoisted to the var scope).
     /// Keyed by the `(original scope, name)` the symbol was declared in, so that future
@@ -153,6 +155,7 @@ impl<'a> SemanticBuilder<'a> {
             source_type: SourceType::default(),
             errors: RefCell::new(Diagnostics::new()),
             current_reference_flags: ReferenceFlags::empty(),
+            ambient_depth: 0,
             current_scope_id,
             current_function_node_id: NodeId::ROOT,
             module_instance_state_cache: FxHashMap::default(),
@@ -412,20 +415,23 @@ impl<'a> SemanticBuilder<'a> {
         self.node_store.ancestry()
     }
 
-    fn is_declare_class_element(&self) -> bool {
-        // Class elements are children of `ClassBody`, whose parent is `Class`.
-        matches!(
-            self.ancestry().ancestor_kinds().nth(1),
-            Some(AstKind::Class(class)) if class.declare
-        )
+    pub(crate) fn in_ambient_context(&self) -> bool {
+        self.ambient_depth > 0
     }
 
-    pub(crate) fn in_declare_scope(&self) -> bool {
-        self.source_type.is_typescript_definition()
-            || self
-                .scoping
-                .scope_ancestors(self.current_scope_id)
-                .any(|scope_id| self.scoping.scope_flags(scope_id).is_ts_module_block())
+    #[inline]
+    fn enter_ambient_context(&mut self, is_ambient: bool) {
+        if is_ambient {
+            self.ambient_depth += 1;
+        }
+    }
+
+    #[inline]
+    fn leave_ambient_context(&mut self, is_ambient: bool) {
+        if is_ambient {
+            debug_assert!(self.ambient_depth > 0);
+            self.ambient_depth -= 1;
+        }
     }
 
     fn create_ast_node(&mut self, kind: AstKind<'a>) {
@@ -880,6 +886,9 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
             AstNodeStoreKind::Ancestry(stack) => stack.push(kind),
         }
 
+        let is_ambient = self.source_type.is_typescript_definition();
+        self.enter_ambient_context(is_ambient);
+
         // Don't call `enter_scope` here as `Program` is a special case - scope has no `parent_id`.
         // Inline the specific logic for `Program` here instead.
         // This simplifies logic in `enter_scope`, as it doesn't have to handle the special case.
@@ -912,9 +921,11 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         self.resolve_all_references();
 
         self.leave_node(kind);
+        self.leave_ambient_context(is_ambient);
 
         // Check `current_function_node_id` has been reset to as it was at start
         debug_assert_eq!(self.current_function_node_id, NodeId::ROOT);
+        debug_assert_eq!(self.ambient_depth, 0);
     }
 
     fn visit_break_statement(&mut self, stmt: &BreakStatement<'a>) {
@@ -949,6 +960,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         }
 
         self.visit_decorators(&class.decorators);
+        self.enter_ambient_context(class.declare);
         self.enter_scope(ScopeFlags::StrictMode, &class.scope_id);
 
         if class.is_expression() {
@@ -964,7 +976,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
             self.visit_ts_type_parameter_declaration(type_parameters);
         }
         if let Some(super_class) = &class.super_class {
-            if class.declare {
+            if self.in_ambient_context() {
                 self.current_reference_flags = ReferenceFlags::ValueAsType;
             }
             self.visit_expression(super_class);
@@ -978,6 +990,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         self.leave_scope();
         self.leave_node(kind);
+        self.leave_ambient_context(class.declare);
         self.class_table_builder.pop_class();
     }
 
@@ -2015,6 +2028,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         let kind = AstKind::Function(self.alloc(func));
         self.enter_node(kind);
+        self.enter_ambient_context(func.declare);
 
         let parent_function_node_id = self.current_function_node_id;
         self.current_function_node_id = self.node_store.current_node_id;
@@ -2104,6 +2118,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         self.leave_scope();
         self.leave_node(kind);
+        self.leave_ambient_context(func.declare);
 
         self.current_function_node_id = parent_function_node_id;
     }
@@ -2461,7 +2476,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         self.enter_node(kind);
         self.visit_span(&method.span);
         self.visit_decorators(&method.decorators);
-        if method.computed && (method.r#type.is_abstract() || self.is_declare_class_element()) {
+        if method.computed && (method.r#type.is_abstract() || self.in_ambient_context()) {
             // declare class A { [prop](): string }
             //                    ^^^^  The property can reference value or [`SymbolFlags::TypeImport`] symbol
             self.current_reference_flags = ReferenceFlags::ValueAsType;
@@ -2483,9 +2498,8 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         self.enter_node(kind);
         self.visit_span(&prop.span);
         self.visit_decorators(&prop.decorators);
-        if prop.computed
-            && (prop.declare || prop.r#type.is_abstract() || self.is_declare_class_element())
-        {
+        self.enter_ambient_context(prop.declare);
+        if prop.computed && (prop.r#type.is_abstract() || self.in_ambient_context()) {
             // class A { declare [prop]: string }
             //                   ^^^^^ The property can reference value or [`SymbolFlags::TypeImport`] symbol
             self.current_reference_flags = ReferenceFlags::ValueAsType;
@@ -2499,6 +2513,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
             self.visit_expression(value);
         }
         self.leave_node(kind);
+        self.leave_ambient_context(prop.declare);
     }
 
     fn visit_import_specifier(&mut self, specifier: &ImportSpecifier<'a>) {
@@ -2618,6 +2633,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
     fn visit_ts_module_declaration(&mut self, decl: &TSModuleDeclaration<'a>) {
         let kind = AstKind::TSModuleDeclaration(self.alloc(decl));
         self.enter_node(kind);
+        self.enter_ambient_context(decl.declare);
         decl.bind(self);
         self.visit_span(&decl.span);
         self.visit_ts_module_declaration_name(&decl.id);
@@ -2637,11 +2653,26 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         }
         self.leave_scope();
         self.leave_node(kind);
+        self.leave_ambient_context(decl.declare);
+    }
+
+    fn visit_ts_global_declaration(&mut self, decl: &TSGlobalDeclaration<'a>) {
+        let kind = AstKind::TSGlobalDeclaration(self.alloc(decl));
+        self.enter_node(kind);
+        self.enter_ambient_context(decl.declare);
+        self.enter_scope(ScopeFlags::TsModuleBlock, &decl.scope_id);
+        self.visit_span(&decl.span);
+        self.visit_span(&decl.global_span);
+        self.visit_ts_module_block(&decl.body);
+        self.leave_scope();
+        self.leave_node(kind);
+        self.leave_ambient_context(decl.declare);
     }
 
     fn visit_ts_type_alias_declaration(&mut self, decl: &TSTypeAliasDeclaration<'a>) {
         let kind = AstKind::TSTypeAliasDeclaration(self.alloc(decl));
         self.enter_node(kind);
+        self.enter_ambient_context(decl.declare);
         decl.bind(self);
         self.visit_span(&decl.span);
         self.visit_binding_identifier(&decl.id);
@@ -2652,11 +2683,13 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         self.visit_ts_type(&decl.type_annotation);
         self.leave_scope();
         self.leave_node(kind);
+        self.leave_ambient_context(decl.declare);
     }
 
     fn visit_ts_interface_declaration(&mut self, decl: &TSInterfaceDeclaration<'a>) {
         let kind = AstKind::TSInterfaceDeclaration(self.alloc(decl));
         self.enter_node(kind);
+        self.enter_ambient_context(decl.declare);
         decl.bind(self);
         self.visit_span(&decl.span);
         self.visit_binding_identifier(&decl.id);
@@ -2668,11 +2701,13 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         self.visit_ts_interface_body(&decl.body);
         self.leave_scope();
         self.leave_node(kind);
+        self.leave_ambient_context(decl.declare);
     }
 
     fn visit_ts_enum_declaration(&mut self, decl: &TSEnumDeclaration<'a>) {
         let kind = AstKind::TSEnumDeclaration(self.alloc(decl));
         self.enter_node(kind);
+        self.enter_ambient_context(decl.declare);
         decl.bind(self);
         self.visit_span(&decl.span);
         self.visit_binding_identifier(&decl.id);
@@ -2682,6 +2717,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
             crate::ts_enum::eval::evaluate_enum_members(decl, &mut self.scoping);
         }
         self.leave_node(kind);
+        self.leave_ambient_context(decl.declare);
     }
 
     fn visit_ts_enum_member(&mut self, member: &TSEnumMember<'a>) {
@@ -2839,10 +2875,12 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
     fn visit_variable_declaration(&mut self, it: &VariableDeclaration<'a>) {
         let kind = AstKind::VariableDeclaration(self.alloc(it));
         self.enter_node(kind);
+        self.enter_ambient_context(it.declare);
         control_flow!(self, |cfg| cfg.enter_statement(self.node_store.current_node_id));
         self.visit_span(&it.span);
         self.visit_variable_declarators(&it.declarations);
         self.leave_node(kind);
+        self.leave_ambient_context(it.declare);
     }
 
     fn visit_empty_statement(&mut self, it: &EmptyStatement) {

crates/oxc_semantic/src/checker/javascript.rs

@@ -11,7 +11,7 @@ use oxc_syntax::{
     number::NumberBase,
     operator::UnaryOperator,
     scope::{ScopeFlags, ScopeId},
-    symbol::{SymbolFlags, SymbolId},
+    symbol::SymbolFlags,
 };
 
 use crate::{
@@ -151,20 +151,10 @@ pub fn check_duplicate_class_elements(ctx: &SemanticBuilder<'_>) {
     });
 }
 
-pub fn check_identifier(
-    name: &str,
-    span: Span,
-    symbol_id: Option<SymbolId>,
-    ctx: &SemanticBuilder<'_>,
-) {
-    // reserved keywords are allowed in ambient contexts
-    fn is_allowed_context(symbol_id: Option<SymbolId>, ctx: &SemanticBuilder<'_>) -> bool {
-        ctx.source_type.is_typescript_definition() || is_in_ambient_context(symbol_id, ctx)
-    }
-
+pub fn check_identifier(name: &str, span: Span, ctx: &SemanticBuilder<'_>) {
     match name {
         "await" => {
-            if is_allowed_context(symbol_id, ctx) {
+            if ctx.in_ambient_context() {
                 return;
             }
 
@@ -181,7 +171,7 @@ pub fn check_identifier(
         // becomes better for performance again.
         "implements" | "interface" | "let" | "package" | "private" | "protected" | "public"
         | "static" | "yield" => {
-            if !ctx.strict_mode() || is_allowed_context(symbol_id, ctx) {
+            if !ctx.strict_mode() || ctx.in_ambient_context() {
                 return;
             }
             // It is a Syntax Error if this phrase is contained in strict mode code and the StringValue of IdentifierName is: "implements", "interface", "let", "package", "private", "protected", "public", "static", or "yield".
@@ -191,24 +181,6 @@ pub fn check_identifier(
     }
 }
 
-fn is_in_ambient_context(symbol_id: Option<SymbolId>, ctx: &SemanticBuilder<'_>) -> bool {
-    if ctx.current_scope_flags().is_ts_module_block() {
-        return true;
-    }
-
-    if let Some(symbol_id) = symbol_id
-        && ctx.scoping.symbol_flags(symbol_id).contains(SymbolFlags::Ambient)
-    {
-        return true;
-    }
-
-    ctx.ancestry().ancestor_kinds().any(|kind| match kind {
-        AstKind::VariableDeclaration(decl) => decl.declare,
-        AstKind::PropertyDefinition(prop) => prop.declare,
-        _ => false,
-    })
-}
-
 pub fn check_binding_identifier(ident: &BindingIdentifier, ctx: &SemanticBuilder<'_>) {
     // `.d.ts` files are allowed to use `eval` and `arguments` as binding identifiers
     if ctx.source_type.is_typescript_definition() {

crates/oxc_semantic/src/checker/mod.rs

@@ -24,14 +24,14 @@ pub fn check<'a>(kind: AstKind<'a>, ctx: &SemanticBuilder<'a>) {
             ts::check_ts_export_assignment_in_program(program, ctx);
         }
         AstKind::BindingIdentifier(ident) => {
-            js::check_identifier(&ident.name, ident.span, ident.symbol_id.get(), ctx);
+            js::check_identifier(&ident.name, ident.span, ctx);
             js::check_binding_identifier(ident, ctx);
         }
         AstKind::IdentifierReference(ident) => {
-            js::check_identifier(&ident.name, ident.span, None, ctx);
+            js::check_identifier(&ident.name, ident.span, ctx);
             js::check_identifier_reference(ident, ctx);
         }
-        AstKind::LabelIdentifier(ident) => js::check_identifier(&ident.name, ident.span, None, ctx),
+        AstKind::LabelIdentifier(ident) => js::check_identifier(&ident.name, ident.span, ctx),
         AstKind::PrivateIdentifier(ident) => js::check_private_identifier_outside_class(ident, ctx),
         AstKind::NumericLiteral(lit) => js::check_number_literal(lit, ctx),
         AstKind::StringLiteral(lit) => js::check_string_literal(lit, ctx),

crates/oxc_semantic/src/checker/typescript.rs

@@ -117,7 +117,7 @@ pub fn check_ts_module_declaration<'a>(decl: &TSModuleDeclaration<'a>, ctx: &Sem
 pub fn check_ts_global_declaration<'a>(decl: &TSGlobalDeclaration<'a>, ctx: &SemanticBuilder<'a>) {
     check_ts_module_or_global_declaration(decl.span, ctx);
 
-    if !decl.declare && !ctx.in_declare_scope() {
+    if !decl.declare && !ctx.in_ambient_context() {
         ctx.error(diagnostics::global_scope_augmentation_should_have_declare_modifier(
             decl.global_span,
         ));
@@ -179,7 +179,7 @@ pub fn check_class<'a>(class: &Class<'a>, ctx: &SemanticBuilder<'a>) {
         }
     }
 
-    if !class.r#declare && !ctx.in_declare_scope() {
+    if !ctx.in_ambient_context() {
         let mut is_in_overload_group = false;
         for (a, b) in class.body.body.iter().map(Some).chain(vec![None]).tuple_windows() {
             if let Some(ClassElement::MethodDefinition(a)) = a
@@ -242,23 +242,8 @@ pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &Semantic
     }
 
     // Illegal to have `get foo();` or `set foo(a)`
-    if method.kind.is_accessor() && is_empty_body && !is_abstract {
-        let is_declare = ctx.class_table_builder.current_class_id.map_or(
-            ctx.source_type.is_typescript_definition(),
-            |id| {
-                let node_id = ctx.class_table_builder.classes.declarations[id];
-                let AstKind::Class(class) = ctx.ancestry().find_kind_by_node_id(node_id) else {
-                    #[cfg(debug_assertions)]
-                    panic!("current_class_id is set, but does not point to a Class node.");
-                    #[cfg(not(debug_assertions))]
-                    return ctx.source_type.is_typescript_definition();
-                };
-                class.declare || ctx.source_type.is_typescript_definition()
-            },
-        );
-        if !is_declare {
-            ctx.error(diagnostics::accessor_without_body(method.key.span()));
-        }
+    if method.kind.is_accessor() && is_empty_body && !is_abstract && !ctx.in_ambient_context() {
+        ctx.error(diagnostics::accessor_without_body(method.key.span()));
     }
 }
 

crates/oxc_syntax/src/scope.rs

@@ -36,7 +36,7 @@ bitflags! {
         const Function         = 1 << 2;
         const Arrow            = 1 << 3;
         const ClassStaticBlock = 1 << 4;
-        const TsModuleBlock    = 1 << 5; // `declare namespace`
+        const TsModuleBlock    = 1 << 5; // `namespace`
         const Constructor      = 1 << 6;
         const GetAccessor      = 1 << 7;
         const SetAccessor      = 1 << 8;

tasks/coverage/misc/pass/reserved-kw-ambient-context.ts

@@ -1,4 +1,8 @@
 export {};
 
 declare let f: (package: number) => interface;
-class C2 { declare  field: package; }
+class C { declare field: package; }
+
+declare function f2(package: number): interface;
+
+declare class C2 { field: package; }

tasks/coverage/snapshots/parser_typescript.snap

@@ -3,7 +3,7 @@ commit: 7964e22f
 parser_typescript Summary:
 AST Parsed     : 9779/9779 (100.00%)
 Positive Passed: 9779/9779 (100.00%)
-Negative Passed: 1642/2640 (62.20%)
+Negative Passed: 1644/2640 (62.27%)
 Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/FunctionDeclaration3.ts
 
 Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/FunctionDeclaration4.ts
@@ -644,12 +644,8 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/moduleAugmen
 
 Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/moduleAugmentationGlobal7.ts
 
-Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/moduleAugmentationGlobal7_1.ts
-
 Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/moduleAugmentationGlobal8.ts
 
-Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/moduleAugmentationGlobal8_1.ts
-
 Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/moduleAugmentationImportsAndExports2.ts
 
 Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/moduleAugmentationImportsAndExports3.ts
@@ -9249,6 +9245,22 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/types/wit
  2 │     interface Array<T> { x }
    ╰────
 
+  × TS(2670): Augmentations for the global scope should have 'declare' modifier unless they appear in already ambient context.
+   ╭─[typescript/tests/cases/compiler/moduleAugmentationGlobal7_1.ts:2:5]
+ 1 │ namespace A {
+ 2 │     global {
+   ·     ──────
+ 3 │         interface Array<T> { x }
+   ╰────
+
+  × TS(2670): Augmentations for the global scope should have 'declare' modifier unless they appear in already ambient context.
+   ╭─[typescript/tests/cases/compiler/moduleAugmentationGlobal8_1.ts:2:5]
+ 1 │ namespace A {
+ 2 │     global {
+   ·     ──────
+ 3 │         interface Array<T> { x }
+   ╰────
+
   × Identifier `Kettle` has already been declared
     ╭─[typescript/tests/cases/compiler/moduleDuplicateIdentifiers.ts:20:14]
  19 │ 
@@ -12539,6 +12551,13 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/types/wit
    ·           ───────
    ╰────
 
+  × The keyword 'public' is reserved
+   ╭─[typescript/tests/cases/compiler/strictModeReservedWordInModuleDeclaration.ts:6:19]
+ 5 │ }
+ 6 │ namespace private.public.foo { }
+   ·                   ──────
+   ╰────
+
   × The keyword 'package' is reserved
    ╭─[typescript/tests/cases/compiler/strictModeWordInImportDeclaration.ts:2:13]
  1 │ "use strict"

tasks/coverage/snapshots/semantic_misc.snap

@@ -92,6 +92,6 @@ rebuilt        : SymbolId(9): []
 
 semantic Error: tasks/coverage/misc/pass/reserved-kw-ambient-context.ts
 Bindings mismatch:
-after transform: ScopeId(0): ["C2", "f"]
-rebuilt        : ScopeId(0): ["C2"]
+after transform: ScopeId(0): ["C", "C2", "f", "f2"]
+rebuilt        : ScopeId(0): ["C"]
 

tasks/coverage/snapshots/semantic_typescript.snap

@@ -2,7 +2,7 @@ commit: 7964e22f
 
 semantic_typescript Summary:
 AST Parsed     : 4720/4720 (100.00%)
-Positive Passed: 3486/4720 (73.86%)
+Positive Passed: 3489/4720 (73.92%)
 semantic Error: tasks/coverage/typescript/tests/cases/compiler/APISample_Watch.ts
 Bindings mismatch:
 after transform: ScopeId(0): ["console", "formatHost", "os", "process", "reportDiagnostic", "reportWatchStatusChanged", "ts", "watchMain"]
@@ -3277,11 +3277,6 @@ Unresolved references mismatch:
 after transform: []
 rebuilt        : ["mapped"]
 
-semantic Error: tasks/coverage/typescript/tests/cases/compiler/declarationTypecheckNoUseBeforeReferenceCheck.ts
-Unresolved references mismatch:
-after transform: ["Object"]
-rebuilt        : []
-
 semantic Error: tasks/coverage/typescript/tests/cases/compiler/declareDottedExtend.ts
 Bindings mismatch:
 after transform: ScopeId(0): ["A", "D", "E", "ab"]
@@ -17524,9 +17519,6 @@ semantic Error: tasks/coverage/typescript/tests/cases/conformance/externalModule
 Bindings mismatch:
 after transform: ScopeId(0): ["B", "ns"]
 rebuilt        : ScopeId(0): []
-Unresolved references mismatch:
-after transform: ["A"]
-rebuilt        : []
 
 semantic Error: tasks/coverage/typescript/tests/cases/conformance/externalModules/typeOnly/exportDeclaration.ts
 Bindings mismatch:
@@ -18164,16 +18156,6 @@ Unresolved references mismatch:
 after transform: []
 rebuilt        : ["React"]
 
-semantic Error: tasks/coverage/typescript/tests/cases/conformance/nonjsExtensions/declarationFileForHtmlFileWithinDeclarationFile.ts
-Unresolved references mismatch:
-after transform: ["HTMLElement"]
-rebuilt        : []
-
-semantic Error: tasks/coverage/typescript/tests/cases/conformance/nonjsExtensions/declarationFileForHtmlImport.ts
-Unresolved references mismatch:
-after transform: ["HTMLElement"]
-rebuilt        : []
-
 semantic Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration17.ts
 Bindings mismatch:
 after transform: ScopeId(0): ["Enumerator"]