fix(compiler): close the remaining ngtsc parity gaps from #623

crates/oxc_angular_compiler/src/class_metadata/builders.rs

@@ -471,7 +471,7 @@ pub fn build_ctor_params_metadata_in<'a>(
             import_map,
             &class_type_params,
             local_decls,
-            const_enums_visible,
+            const_enums_visible.then_some(consts).flatten(),
         )
         .unwrap_or_else(|| {
             OutputExpression::Literal(Box::new_in(
@@ -968,18 +968,21 @@ fn string_literal<'a>(allocator: &'a Allocator, value: Ident<'a>) -> OutputExpre
 /// `const enum`s follow full and partial compilation instead, the modes the
 /// Angular CLI and ng-packagr run: there `typeToValue` gives
 /// `noValueDeclaration` for a `ConstEnum` symbol, so `type:` is `undefined`.
-/// That holds for one declared in the file and for a named import the
-/// cross-file analysis found to be one ([`crate::ImportInfo::is_const_enum`]),
-/// when `const_enums_visible` says the class sees the file-scope names.
+/// That holds for one declared in the file and for an import the cross-file
+/// analysis followed to one ([`crate::ImportInfo::is_const_enum`],
+/// [`StringConsts::is_imported_const_enum`]). `const_enum_scope` holds the
+/// file's consts when the class sees the file-scope names (`None` for a
+/// class nested in a function or block).
 fn build_param_type_expression<'a>(
     allocator: &'a Allocator,
     param: &FormalParameter<'a>,
     namespace_registry: &mut NamespaceRegistry<'a>,
     import_map: &ImportMap<'a>,
     type_params: &FxHashSet<Ident<'a>>,
     local_decls: Option<&LocalDecls<'a>>,
-    const_enums_visible: bool,
+    const_enum_scope: Option<&StringConsts<'a>>,
 ) -> Option<OutputExpression<'a>> {
+    let const_enums_visible = const_enum_scope.is_some();
     // Upstream's getConstructorParameters first narrows `T | null` to `T`
     // (typescript.ts); `typeToValue` then requires a bare `TypeReferenceNode`
     // — `(X)`, `X | undefined`, `X | Y`, `typeof`, `this`, primitives are
@@ -1009,9 +1012,13 @@ fn build_param_type_expression<'a>(
         if import_info.is_type_only || path.len() == 1 && !import_info.is_named_import {
             return None;
         }
-        // An imported `const enum` (`import { KeyCodes }`): no value in full
-        // or partial `typeToValue`, so no `i1` import either.
-        if const_enums_visible && path.len() == 1 && import_info.is_const_enum {
+        // An imported `const enum` (`import { KeyCodes }`, `k.KeyCodes`,
+        // `NS.Inner`): no value in full or partial `typeToValue`, so no `i1`
+        // import either. An enum member (`k.KeyCodes.A`) keeps its reference.
+        if let Some(consts) = const_enum_scope
+            && (path.len() == 1 && import_info.is_const_enum
+                || consts.is_imported_const_enum(&path))
+        {
             return None;
         }
         // `import Foo from 'm'`: upstream emits the local name itself (LOCAL
@@ -1173,6 +1180,31 @@ pub fn type_param_names_of<'a>(class: &Class<'a>) -> FxHashSet<Ident<'a>> {
         .unwrap_or_default()
 }
 
+/// Whether a namespace body holds only types: interfaces, type aliases,
+/// local `import X = A.B` aliases and namespaces like it (TypeScript's
+/// `NonInstantiated` module state). Anything else counts as instantiated,
+/// a nested `const enum` and an empty statement too.
+fn declares_only_types(body: &TSNamespaceDeclarationBody<'_>) -> bool {
+    match body {
+        TSNamespaceDeclarationBody::TSModuleBlock(block) => {
+            block.body.iter().all(|stmt| match stmt {
+                Statement::TSInterfaceDeclaration(_)
+                | Statement::TSTypeAliasDeclaration(_)
+                | Statement::TSImportEqualsDeclaration(_) => true,
+                Statement::TSNamespaceDeclaration(ns) => declares_only_types(&ns.body),
+                Statement::ExportDeclaration(export) => match &export.declaration {
+                    Declaration::TSInterfaceDeclaration(_)
+                    | Declaration::TSTypeAliasDeclaration(_) => true,
+                    Declaration::TSNamespaceDeclaration(ns) => declares_only_types(&ns.body),
+                    _ => false,
+                },
+                _ => false,
+            })
+        }
+        TSNamespaceDeclarationBody::TSNamespaceDeclaration(ns) => declares_only_types(&ns.body),
+    }
+}
+
 /// Names declared in the file, nested under their namespaces.
 ///
 /// This models the local-compilation `typeToValue` (the mode the e2e
@@ -1202,6 +1234,10 @@ pub(crate) struct LocalDecls<'a> {
     /// Names declared by a `const enum` (also in `value_decls`). See
     /// [`Self::is_const_enum`].
     const_enums: FxHashSet<&'a str>,
+    /// Names of namespaces that hold more than types (see
+    /// [`declares_only_types`]). A `const enum` merged with one of them is a
+    /// TypeScript error, so [`Self::is_const_enum`] keeps today's output.
+    instantiated_namespaces: FxHashSet<&'a str>,
 }
 
 impl<'a> LocalDecls<'a> {
@@ -1455,6 +1491,9 @@ impl<'a> LocalDecls<'a> {
         decl: &'a oxc_ast::ast::TSNamespaceDeclaration<'a>,
         globals: &mut std::vec::Vec<&'a Statement<'a>>,
     ) {
+        if !declares_only_types(&decl.body) {
+            self.instantiated_namespaces.insert(decl.id.name.as_str());
+        }
         self.namespaces.entry(decl.id.name.as_str()).or_default().merge(Self::from_namespace_body(
             &decl.body,
             decl.declare,
@@ -1476,6 +1515,8 @@ impl<'a> LocalDecls<'a> {
                 // `NS.E` reads an exported member only: a const enum the block
                 // keeps to itself must not hide an exported `E` that another
                 // `namespace NS` block declares.
+                // The same goes for a namespace the block keeps to itself: it
+                // doesn't merge with an exported `const enum` of another block.
                 if !ambient {
                     decls.const_enums.retain(|name| {
                         block.body.iter().any(|stmt| {
@@ -1484,12 +1525,22 @@ impl<'a> LocalDecls<'a> {
                                     Declaration::TSEnumDeclaration(e) if e.id.name == *name))
                         })
                     });
+                    decls.instantiated_namespaces.retain(|name| {
+                        block.body.iter().any(|stmt| {
+                            matches!(stmt, Statement::ExportDeclaration(export)
+                                if matches!(&export.declaration,
+                                    Declaration::TSNamespaceDeclaration(ns) if ns.id.name == *name))
+                        })
+                    });
                 }
                 decls
             }
             // `namespace Outer.Inner { ... }` nests by qualified name.
             TSNamespaceDeclarationBody::TSNamespaceDeclaration(nested) => {
                 let mut decls = LocalDecls::default();
+                if !declares_only_types(&nested.body) {
+                    decls.instantiated_namespaces.insert(nested.id.name.as_str());
+                }
                 decls.namespaces.insert(
                     nested.id.name.as_str(),
                     Self::from_namespace_body(&nested.body, ambient || nested.declare, globals),
@@ -1503,6 +1554,7 @@ impl<'a> LocalDecls<'a> {
         self.type_decls.extend(other.type_decls);
         self.value_decls.extend(other.value_decls);
         self.const_enums.extend(other.const_enums);
+        self.instantiated_namespaces.extend(other.instantiated_namespaces);
         for (name, nested) in other.namespaces {
             self.namespaces.entry(name).or_default().merge(nested);
         }
@@ -1533,16 +1585,18 @@ impl<'a> LocalDecls<'a> {
     /// (`declare const enum` too). ngtsc's `typeToValue` has no value for
     /// it outside local compilation (`decl.flags & ts.SymbolFlags.ConstEnum`
     /// gives `noValueDeclaration`), so full and partial metadata write
-    /// `type: undefined`. A name a namespace, interface, type alias or
-    /// `declare global` block also declares is left out: one file can't be
-    /// sure what it merges with, so it keeps today's output.
+    /// `type: undefined`. That holds when it merges with namespaces that
+    /// only hold types too. A name an instantiated namespace, interface,
+    /// type alias or `declare global` block also declares is left out: that
+    /// is a TypeScript error, or one file can't be sure what it merges with,
+    /// so it keeps today's output.
     pub(crate) fn is_const_enum(&self, path: &[Ident<'_>]) -> bool {
         match path.split_first() {
             None => false,
             Some((name, [])) => {
                 let name = name.as_str();
                 self.const_enums.contains(name)
-                    && !self.namespaces.contains_key(name)
+                    && !self.instantiated_namespaces.contains(name)
                     && !self.type_decls.contains(name)
                     && !self.global_names.contains(name)
             }

crates/oxc_angular_compiler/src/component/cross_file_elision.rs

@@ -32,6 +32,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
 use super::defer_resolver::{ClassUse, decorated_class_selector};
 use crate::directive::{ImportValueResolver, StringConsts};
 
+mod const_enums;
 mod ng_module_scope;
 
 /// `(file, exported_name, local_name, implicit_standalone)` -> how a
@@ -106,9 +107,9 @@ struct Inner {
     this: Weak<Inner>,
     /// Memoized NgModule scope reads (see [`ng_module_scope`]).
     scope: ng_module_scope::ScopeState,
-    /// File path -> the names it exports a `const enum` under, filled by
-    /// `analyze_file` (see [`const_enum_export_names`]).
-    const_enum_exports: RefCell<FxHashMap<String, FxHashSet<String>>>,
+    /// File path -> what the `const enum` lookups read of it (see
+    /// [`const_enums`]).
+    enum_shapes: RefCell<FxHashMap<String, std::sync::Arc<const_enums::EnumShape>>>,
 }
 
 /// A file parsed once per analyzer: its program, and the consts the
@@ -258,7 +259,7 @@ impl CrossFileAnalyzer {
                 value_dependencies: RefCell::new(FxHashSet::default()),
                 this: this.clone(),
                 scope: ng_module_scope::ScopeState::default(),
-                const_enum_exports: RefCell::new(FxHashMap::default()),
+                enum_shapes: RefCell::new(FxHashMap::default()),
             }),
         }
     }
@@ -276,20 +277,6 @@ impl CrossFileAnalyzer {
         self.inner.is_type_only_import(import_source, import_name, from_file)
     }
 
-    /// Whether `import_name`, imported from `import_source` in `from_file`,
-    /// is a `const enum` where it is declared, following `export { X } from`
-    /// and `export *` chains. `false` when unsure (packages, `.d.ts`-only
-    /// modules, unresolvable paths, `export * as ns`). Files read are
-    /// recorded in [`Self::take_value_dependencies`].
-    pub(crate) fn is_const_enum_import(
-        &self,
-        import_source: &str,
-        import_name: &str,
-        from_file: &Path,
-    ) -> bool {
-        self.inner.is_const_enum_import(import_source, import_name, from_file)
-    }
-
     /// Resolve the actual source file path for an import, tracing through barrel exports.
     ///
     /// Returns the relative path from `from_file` to the actual source file where
@@ -502,68 +489,6 @@ impl Inner {
         self.export_value_at(&resolved, import_name, 0)
     }
 
-    /// See [`CrossFileAnalyzer::is_const_enum_import`].
-    fn is_const_enum_import(
-        &self,
-        import_source: &str,
-        import_name: &str,
-        from_file: &Path,
-    ) -> bool {
-        let Some(parent) = from_file.parent() else { return false };
-        let Some(resolved) = self.resolve_module_spec(parent, import_source) else {
-            return false;
-        };
-        self.const_enum_at(&resolved, import_name, 0)
-    }
-
-    /// Whether `export_name` of `file_path` is a `const enum`, direct or
-    /// through re-exports, like [`Self::export_value_at`] follows them.
-    fn const_enum_at(&self, file_path: &str, export_name: &str, depth: u16) -> bool {
-        // A package's sources aren't read, like `is_type_only_import`.
-        if depth > MAX_EXPORT_CHAIN
-            || Path::new(file_path).components().any(|c| c.as_os_str() == "node_modules")
-        {
-            return false;
-        }
-        self.value_dependencies.borrow_mut().insert(file_path.to_string());
-        if !self.cache.borrow().contains_key(file_path) {
-            if self.analyzing.borrow().contains(file_path) {
-                return false; // Circular import
-            }
-            self.analyze_file(file_path);
-        }
-        let export_info = {
-            let cache = self.cache.borrow();
-            cache.get(file_path).and_then(|exports| exports.get(export_name).cloned())
-        };
-        let Some(export_info) =
-            export_info.or_else(|| self.find_in_star_exports(file_path, export_name))
-        else {
-            return false;
-        };
-        match export_info.re_export_source {
-            // `export * as ns` names a namespace, not one declaration.
-            Some((_, original_name)) if original_name == "*" => false,
-            Some((source_module, original_name)) => {
-                let Some(resolved) = Path::new(file_path)
-                    .parent()
-                    .and_then(|parent| self.resolve_module_spec(parent, &source_module))
-                else {
-                    return false;
-                };
-                self.const_enum_at(&resolved, &original_name, depth + 1)
-            }
-            None => {
-                !export_info.is_type_only
-                    && self
-                        .const_enum_exports
-                        .borrow()
-                        .get(file_path)
-                        .is_some_and(|names| names.contains(export_name))
-            }
-        }
-    }
-
     fn standalone_directive_selector(
         &self,
         import_source: &str,
@@ -1104,9 +1029,6 @@ impl Inner {
         let source_type = SourceType::from_path(file_path).unwrap_or_default();
         let parser_ret = Parser::new(&allocator, &source, source_type).parse();
         self.analyze_program(&parser_ret.program, &mut exports);
-        self.const_enum_exports
-            .borrow_mut()
-            .insert(file_path.to_string(), const_enum_export_names(&parser_ret.program));
 
         self.cache.borrow_mut().insert(file_path.to_string(), exports);
         self.analyzing.borrow_mut().remove(file_path);
@@ -1392,7 +1314,7 @@ impl Inner {
         self.parsed.borrow_mut().clear();
         self.value_memo.borrow_mut().clear();
         self.selector_memo.borrow_mut().clear();
-        self.const_enum_exports.borrow_mut().clear();
+        self.enum_shapes.borrow_mut().clear();
     }
 
     /// Get the number of cached files.
@@ -1401,47 +1323,6 @@ impl Inner {
     }
 }
 
-/// The names `program` exports a top-level `const enum` under:
-/// `export const enum E`, `export declare const enum E`, and `export { E }` /
-/// `export { E as F }` of a top-level one. Empty when the file has a bare
-/// `namespace`, `declare module` or `declare global` block: `analyze_statement`
-/// lists their members as the file's own exports, so a name could mean either.
-fn const_enum_export_names(program: &Program) -> FxHashSet<String> {
-    let mut names = FxHashSet::default();
-    let mut const_enums = FxHashSet::default();
-    for stmt in &program.body {
-        match stmt {
-            Statement::TSNamespaceDeclaration(_)
-            | Statement::TSExternalModuleDeclaration(_)
-            | Statement::TSGlobalDeclaration(_) => return FxHashSet::default(),
-            Statement::TSEnumDeclaration(e) if e.r#const => {
-                const_enums.insert(e.id.name.as_str());
-            }
-            Statement::ExportDeclaration(export) => {
-                if let Declaration::TSEnumDeclaration(e) = &export.declaration
-                    && e.r#const
-                {
-                    const_enums.insert(e.id.name.as_str());
-                    names.insert(e.id.name.to_string());
-                }
-            }
-            _ => {}
-        }
-    }
-    for stmt in &program.body {
-        if let Statement::ExportNamedDeclaration(export) = stmt
-            && !export.export_kind.is_type()
-        {
-            for spec in &export.specifiers {
-                if !spec.export_kind.is_type() && const_enums.contains(spec.local.name().as_str()) {
-                    names.insert(spec.exported.name().to_string());
-                }
-            }
-        }
-    }
-    names
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;