fix: import elision for namespace imports, arrow function bodies, empty imports, and star export barrels (#18)

- Elide namespace imports (`import * as X`) when only used in type positions
- Traverse arrow function bodies to find value references (e.g., `forwardRef(() => Comp)`)
- Remove empty imports (`import {} from 'module'`)
- Fix cross-file type detection through star export barrels (`export * from './module'`)
- Remove duplicate match arms in expand_safe_reads

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: Brooooooklyn

crates/oxc_angular_compiler/src/component/cross_file_elision.rs

@@ -312,6 +312,9 @@ impl CrossFileAnalyzer {
             exports.get(export_name).cloned()
         };
 
+        // If not found directly, check star exports (export * from './other')
+        let export_info = export_info.or_else(|| self.find_in_star_exports(file_path, export_name));
+
         let Some(export_info) = export_info else {
             return false;
         };
@@ -920,4 +923,55 @@ export { doSomething } from './utils';
         let resolved = analyzer.resolve_import_source_path("./index", "RenamedExport", &main_file);
         assert_eq!(resolved, Some("./original".to_string()));
     }
+
+    /// Regression test for quick-access.component.ts:
+    /// `import { WIDGET_CONTROL, WidgetControlService } from '../../widget-control'`
+    /// where `widget-control/index.ts` has `export * from './widget-control.service'`
+    /// and `widget-control.service.ts` has `export interface WidgetControlService { ... }`
+    ///
+    /// The interface should be detected as type-only through the barrel star export chain.
+    #[test]
+    fn test_interface_through_star_export_barrel_is_type_only() {
+        let dir = TempDir::new().unwrap();
+        // widget-control/widget-control.service.ts — interface (type-only)
+        create_test_file(
+            dir.path(),
+            "widget-control/widget-control.service.ts",
+            "export interface WidgetControlService { hideWidget(widgetId: string): void; }",
+        );
+        // widget-control/widget-control.token.ts — const (runtime value)
+        create_test_file(
+            dir.path(),
+            "widget-control/widget-control.token.ts",
+            "import { InjectionToken } from '@angular/core';\nexport const WIDGET_CONTROL = new InjectionToken('WidgetControlService');",
+        );
+        // widget-control/index.ts — barrel re-export via star
+        create_test_file(
+            dir.path(),
+            "widget-control/index.ts",
+            "export * from './widget-control.service';\nexport * from './widget-control.token';",
+        );
+
+        let main_file = dir.path().join("quick-access/quick-access/main.ts");
+        // Create parent dir so the path is valid
+        std::fs::create_dir_all(main_file.parent().unwrap()).unwrap();
+
+        let mut analyzer = CrossFileAnalyzer::new(dir.path(), None);
+
+        // WidgetControlService is an interface — should be type-only
+        assert!(
+            analyzer.is_type_only_import(
+                "../../widget-control",
+                "WidgetControlService",
+                &main_file
+            ),
+            "WidgetControlService interface should be detected as type-only through star export barrel"
+        );
+
+        // WIDGET_CONTROL is a const — should NOT be type-only
+        assert!(
+            !analyzer.is_type_only_import("../../widget-control", "WIDGET_CONTROL", &main_file),
+            "WIDGET_CONTROL const should NOT be type-only"
+        );
+    }
 }

crates/oxc_angular_compiler/src/component/import_elision.rs

@@ -114,8 +114,13 @@ impl<'a> ImportElisionAnalyzer<'a> {
                             type_only_specifiers.insert(name.clone());
                         }
                     }
-                    ImportDeclarationSpecifier::ImportNamespaceSpecifier(_) => {
-                        // Namespace imports are generally kept (hard to determine type-only usage)
+                    ImportDeclarationSpecifier::ImportNamespaceSpecifier(spec) => {
+                        // Check if the namespace import is type-only using semantic analysis.
+                        // e.g., `import * as moment from 'moment'` where `moment` is only
+                        // used in type annotations like `moment.Moment`.
+                        if Self::is_type_only_import(&spec.local, semantic) {
+                            type_only_specifiers.insert(spec.local.name.clone());
+                        }
                     }
                 }
             }
@@ -587,9 +592,25 @@ impl<'a> ImportElisionAnalyzer<'a> {
                     }
                 }
             }
-            Expression::ArrowFunctionExpression(_) => {
-                // For arrow functions, we don't need to deeply analyze the body
-                // since we're only checking decorator positions
+            Expression::ArrowFunctionExpression(arrow) => {
+                // Arrow function bodies may contain value references, e.g.,
+                // `forwardRef(() => TagPickerComponent)` in Component imports.
+                for stmt in &arrow.body.statements {
+                    match stmt {
+                        Statement::ExpressionStatement(expr_stmt) => {
+                            Self::collect_value_uses_from_expr(
+                                &expr_stmt.expression,
+                                other_value_uses,
+                            );
+                        }
+                        Statement::ReturnStatement(ret) => {
+                            if let Some(arg) = &ret.argument {
+                                Self::collect_value_uses_from_expr(arg, other_value_uses);
+                            }
+                        }
+                        _ => {}
+                    }
+                }
             }
             _ => {}
         }
@@ -701,13 +722,24 @@ impl<'a> ImportElisionAnalyzer<'a> {
 /// Filter import declarations to remove type-only specifiers.
 ///
 /// Returns a new source string with type-only import specifiers removed.
-/// Entire import declarations are removed if all their specifiers are type-only.
+/// Entire import declarations are removed if all their specifiers are type-only,
+/// or if the import has no specifiers at all (`import {} from 'module'`).
 pub fn filter_imports<'a>(
     source: &str,
     program: &Program<'a>,
     analyzer: &ImportElisionAnalyzer<'a>,
 ) -> String {
-    if !analyzer.has_type_only_imports() {
+    // Check if there are empty imports that need removal (import {} from '...')
+    let has_empty_imports = program.body.iter().any(|stmt| {
+        if let Statement::ImportDeclaration(import_decl) = stmt {
+            if let Some(specifiers) = &import_decl.specifiers {
+                return specifiers.is_empty();
+            }
+        }
+        false
+    });
+
+    if !analyzer.has_type_only_imports() && !has_empty_imports {
         return source.to_string();
     }
 
@@ -740,8 +772,8 @@ pub fn filter_imports<'a>(
             !analyzer.should_elide(name)
         });
 
-        if removed.is_empty() {
-            // All specifiers kept, no changes needed
+        if removed.is_empty() && !kept.is_empty() {
+            // All specifiers kept and at least one exists, no changes needed
             continue;
         }
 
@@ -1870,4 +1902,104 @@ class MyComponent {
             "myKey in parenthesized type literal should be preserved"
         );
     }
+
+    // =========================================================================
+    // Regression tests for ClickUp import elision mismatches
+    // =========================================================================
+
+    #[test]
+    fn test_namespace_import_type_only_should_be_elided() {
+        // Reproduces: bookmark.component.ts
+        // `import * as moment from 'moment'` where `moment` is only used as
+        // `moment.Moment` in type annotations should be elided.
+        let source = r#"
+import * as moment from 'moment';
+import { Component } from '@angular/core';
+
+@Component({ selector: 'app-bookmark' })
+class BookmarkComponent {
+    dueDate: moment.Moment = null;
+}
+"#;
+        let type_only = analyze_source(source);
+        // `moment` is only referenced in type position (moment.Moment)
+        // so it should be marked for elision
+        assert!(
+            type_only.contains("moment"),
+            "Namespace import `moment` used only in type annotation `moment.Moment` should be elided"
+        );
+    }
+
+    #[test]
+    fn test_namespace_import_with_value_usage_preserved() {
+        // Namespace import that IS used at runtime should be preserved
+        let source = r#"
+import * as moment from 'moment';
+import { Component } from '@angular/core';
+
+@Component({ selector: 'app-test' })
+class TestComponent {
+    now = moment();
+}
+"#;
+        let type_only = analyze_source(source);
+        assert!(
+            !type_only.contains("moment"),
+            "Namespace import `moment` used in value expression `moment()` should be preserved"
+        );
+    }
+
+    #[test]
+    fn test_forward_ref_arrow_function_preserves_value_use() {
+        // Reproduces: tags.component.ts
+        // `forwardRef(() => TagPickerComponent)` in @Component imports array
+        // uses TagPickerComponent as a value inside an arrow function.
+        // The arrow function body MUST be traversed to find value uses.
+        let source = r#"
+import { Component, forwardRef, Inject, Optional, SkipSelf } from '@angular/core';
+import { TagPickerComponent } from './tag-picker/tag-picker.component';
+
+@Component({
+    selector: 'app-tags',
+    imports: [forwardRef(() => TagPickerComponent)]
+})
+class TagsComponent {
+    constructor(
+        @Optional()
+        @SkipSelf()
+        @Inject(TagPickerComponent)
+        readonly tagPickerComponent: TagPickerComponent,
+    ) {}
+}
+"#;
+        let type_only = analyze_source(source);
+        // TagPickerComponent is used as a value in forwardRef(() => TagPickerComponent)
+        // and as @Inject(TagPickerComponent) argument. It must NOT be elided.
+        assert!(
+            !type_only.contains("TagPickerComponent"),
+            "TagPickerComponent used in forwardRef arrow function and @Inject should be preserved"
+        );
+    }
+
+    #[test]
+    fn test_empty_import_should_be_elided() {
+        // Reproduces: users-table.component.ts
+        // `import {} from '@cu/teams-pulse/types'` is an empty import that
+        // should be completely removed (no specifiers to keep).
+        let source = r#"
+import { Component } from '@angular/core';
+import {} from '@cu/teams-pulse/types';
+
+@Component({ selector: 'app-users-table' })
+class UsersTableComponent {}
+"#;
+        let filtered = filter_source(source);
+
+        // The empty import should be removed entirely
+        assert!(
+            !filtered.contains("@cu/teams-pulse/types"),
+            "Empty import `import {{}} from '@cu/teams-pulse/types'` should be removed.\nFiltered:\n{}",
+            filtered
+        );
+    }
 }

crates/oxc_angular_compiler/src/pipeline/phases/expand_safe_reads.rs

@@ -174,20 +174,6 @@ fn needs_temporary_in_safe_access(expr: &IrExpression<'_>) -> bool {
             needs_temporary_in_safe_access(&keyed.receiver)
                 || needs_temporary_in_safe_access(&keyed.key)
         }
-        // Binary operators need to check both operands
-        IrExpression::Binary(bin) => {
-            needs_temporary_in_safe_access(&bin.lhs) || needs_temporary_in_safe_access(&bin.rhs)
-        }
-        // Ternary needs to check all branches
-        IrExpression::Ternary(ternary) => {
-            needs_temporary_in_safe_access(&ternary.condition)
-                || needs_temporary_in_safe_access(&ternary.true_expr)
-                || needs_temporary_in_safe_access(&ternary.false_expr)
-        }
-        // Not expression needs to check operand
-        IrExpression::Not(not) => needs_temporary_in_safe_access(&not.expr),
-        // Unary operator needs to check operand
-        IrExpression::Unary(unary) => needs_temporary_in_safe_access(&unary.expr),
         // Check AST expressions for function calls
         IrExpression::Ast(ast_expr) => needs_temporary_in_ast_expression(ast_expr),
         // Parenthesized expressions need to check their inner expression