diff --git a/packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts b/packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts index 0aca2abcd03..ba45d37ead1 100644 --- a/packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts +++ b/packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts @@ -105,18 +105,24 @@ export class UnusedStandaloneImportsRule implements SourceFileValidatorRule { usedDirectives: Set, usedPipes: Set, ) { - if (metadata.imports === null || metadata.rawImports === null) { + const {imports, rawImports} = metadata; + + if (imports === null || rawImports === null) { return null; } let unused: [ref: Reference, type: string, name: string][] | null = null; - for (const current of metadata.imports) { + for (const current of imports) { const currentNode = current.node as ts.ClassDeclaration; const dirMeta = this.templateTypeChecker.getDirectiveMetadata(currentNode); if (dirMeta !== null) { - if (dirMeta.isStandalone && (usedDirectives === null || !usedDirectives.has(currentNode))) { + if ( + dirMeta.isStandalone && + !usedDirectives.has(currentNode) && + !this.isPotentialSharedReference(current, rawImports) + ) { unused ??= []; unused.push([current, dirMeta.isComponent ? 'Component' : 'Directive', dirMeta.name]); } @@ -128,7 +134,8 @@ export class UnusedStandaloneImportsRule implements SourceFileValidatorRule { if ( pipeMeta !== null && pipeMeta.isStandalone && - (usedPipes === null || !usedPipes.has(pipeMeta.name)) + !usedPipes.has(pipeMeta.name) && + !this.isPotentialSharedReference(current, rawImports) ) { unused ??= []; unused.push([current, 'Pipe', pipeMeta.ref.node.name.text]); @@ -137,4 +144,35 @@ export class UnusedStandaloneImportsRule implements SourceFileValidatorRule { return unused; } + + /** + * Determines if an import reference *might* be coming from a shared imports array. + * @param reference Reference to be checked. + * @param rawImports AST node that defines the `imports` array. + */ + private isPotentialSharedReference(reference: Reference, rawImports: ts.Expression): boolean { + // If the reference is defined directly in the `imports` array, it cannot be shared. + if (reference.getIdentityInExpression(rawImports) !== null) { + return false; + } + + // The reference might be shared if it comes from an exported array. If the variable is local + /// to the file, then it likely isn't shared. Note that this has the potential for false + // positives if a non-exported array of imports is shared between components in the same + // file. This scenario is unlikely and even if we report the diagnostic for it, it would be + // okay since the user only has to refactor components within the same file, rather than the + // entire application. + let current: ts.Node | null = reference.getIdentityIn(rawImports.getSourceFile()); + + while (current !== null) { + if (ts.isVariableStatement(current)) { + return !!current.modifiers?.some((m) => m.kind === ts.SyntaxKind.ExportKeyword); + } + current = current.parent; + } + + // Otherwise the reference likely comes from an imported + // symbol like an array of shared common components. + return true; + } } diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 289e31e11ae..b94b2d9167f 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -7765,6 +7765,314 @@ suppress 'Pipe "PercentPipe" is not used within the template', ); }); + + it('should report unused imports coming from a nested array from the same file', () => { + env.write( + 'used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[used]', standalone: true}) + export class UsedDir {} + `, + ); + + env.write( + 'other-used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[other-used]', standalone: true}) + export class OtherUsedDir {} + `, + ); + + env.write( + 'unused.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[unused]', standalone: true}) + export class UnusedDir {} + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {UsedDir} from './used'; + import {OtherUsedDir} from './other-used'; + import {UnusedDir} from './unused'; + + const COMMON = [OtherUsedDir, UnusedDir]; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: [UsedDir, COMMON] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe('Imports array contains unused imports'); + expect(diags[0].relatedInformation?.length).toBe(1); + expect(diags[0].relatedInformation![0].messageText).toBe( + 'Directive "UnusedDir" is not used within the template', + ); + }); + + it('should report unused imports coming from an array used as the `imports` initializer', () => { + env.write( + 'used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[used]', standalone: true}) + export class UsedDir {} + `, + ); + + env.write( + 'unused.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[unused]', standalone: true}) + export class UnusedDir {} + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {UsedDir} from './used'; + import {UnusedDir} from './unused'; + + const IMPORTS = [UsedDir, UnusedDir]; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: IMPORTS + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe('Imports array contains unused imports'); + expect(diags[0].relatedInformation?.length).toBe(1); + expect(diags[0].relatedInformation![0].messageText).toBe( + 'Directive "UnusedDir" is not used within the template', + ); + }); + + it('should not report unused imports coming from an array through a spread expression from a different file', () => { + env.write( + 'used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[used]', standalone: true}) + export class UsedDir {} + `, + ); + + env.write( + 'other-used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[other-used]', standalone: true}) + export class OtherUsedDir {} + `, + ); + + env.write( + 'unused.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[unused]', standalone: true}) + export class UnusedDir {} + `, + ); + + env.write( + 'common.ts', + ` + import {OtherUsedDir} from './other-used'; + import {UnusedDir} from './unused'; + + export const COMMON = [OtherUsedDir, UnusedDir]; + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {UsedDir} from './used'; + import {COMMON} from './common'; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: [UsedDir, ...COMMON] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should not report unused imports coming from a nested array from a different file', () => { + env.write( + 'used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[used]', standalone: true}) + export class UsedDir {} + `, + ); + + env.write( + 'other-used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[other-used]', standalone: true}) + export class OtherUsedDir {} + `, + ); + + env.write( + 'unused.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[unused]', standalone: true}) + export class UnusedDir {} + `, + ); + + env.write( + 'common.ts', + ` + import {OtherUsedDir} from './other-used'; + import {UnusedDir} from './unused'; + + export const COMMON = [OtherUsedDir, UnusedDir]; + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {UsedDir} from './used'; + import {COMMON} from './common'; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: [UsedDir, COMMON] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should not report unused imports coming from an exported array in the same file', () => { + env.write( + 'used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[used]', standalone: true}) + export class UsedDir {} + `, + ); + + env.write( + 'other-used.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[other-used]', standalone: true}) + export class OtherUsedDir {} + `, + ); + + env.write( + 'unused.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[unused]', standalone: true}) + export class UnusedDir {} + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {UsedDir} from './used'; + import {OtherUsedDir} from './other-used'; + import {UnusedDir} from './unused'; + + export const COMMON = [OtherUsedDir, UnusedDir]; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: [UsedDir, COMMON] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); }); }); });