skills/angular__angular
1
0
Fork 0
mirror of https://github.com/angular/angular.git synced 2026-09-14 13:54:52 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(compiler-cli): do not report unused declarations coming from an imported array (#57940)

Browse Source 
Some apps follow a pattern where they have an array of common declarations which is imported in most standalone components, but only some of the declarations are used. Such cases will currently raise the unused imports diagnostic but can be hard to fix, because it would require either removing declarations from the common array which can break other components, or copying only the necessary declarations from the array. Since neither of these solutions is great, this commit tweaks the logic for the diagnostic so that unused imports coming from _exported_ arrays are not reported (either from the same file or another one).

PR Close #57940
This commit is contained in:
Kristiyan Kostadinov
2024-09-25 08:51:32 +02:00
committed by Alex Rickabaugh
parent 2545743ad1
commit 33fe252c58
2 changed files with 350 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts
+42 -4
View File
@@ -105,18 +105,24 @@ export class UnusedStandaloneImportsRule implements SourceFileValidatorRule {
usedDirectives: Set<ts.ClassDeclaration>,
usedPipes: Set<string>,
) {
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;
}
}
packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
+308
View File
@@ -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: \`
<section>
<div other-used></div>
<span used></span>
</section>
\`,
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: \`
<section>
<div></div>
<span used></span>
</section>
\`,
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: \`
<section>
<div other-used></div>
<span used></span>
</section>
\`,
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: \`
<section>
<div other-used></div>
<span used></span>
</section>
\`,
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: \`
<section>
<div other-used></div>
<span used></span>
</section>
\`,
standalone: true,
imports: [UsedDir, COMMON]
})
export class MyComp {}
`,
);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
});
});
});