From a2e4ee0cb3d40cadc05e28d58b06853973944456 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 3 Sep 2024 21:17:33 +0200 Subject: [PATCH] feat(compiler): add diagnostic for unused standalone imports (#57605) Adds a new diagnostic that will report cases where a declaration is in the `imports` array, but isn't being used anywhere. The diagnostic is reported as a warning by default and can be controlled using the following option in the tsconfig: ``` { "angularCompilerOptions": { "extendedDiagnostics": { "checks": { "unusedStandaloneImports": "suppress" } } } } ``` **Note:** I'll look into a codefix for the language service in a follow-up. Fixes #46766. PR Close #57605 --- adev/src/app/sub-navigation-data.ts | 5 + .../reference/extended-diagnostics/NG8113.md | 48 +++ .../extended-diagnostics/overview.md | 1 + .../public-api/compiler-cli/error_code.api.md | 1 + .../extended_template_diagnostic_name.api.md | 4 +- .../annotations/component/src/handler.ts | 1 + .../annotations/directive/src/handler.ts | 1 + .../src/ngtsc/core/src/compiler.ts | 13 +- .../src/ngtsc/diagnostics/src/error.ts | 3 +- .../src/ngtsc/diagnostics/src/error_code.ts | 5 + .../src/extended_template_diagnostic_name.ts | 1 + .../src/ngtsc/metadata/src/api.ts | 5 + .../src/ngtsc/metadata/src/dts.ts | 1 + .../src/ngtsc/scope/test/local_spec.ts | 1 + .../src/ngtsc/typecheck/api/api.ts | 7 + .../src/ngtsc/typecheck/extended/index.ts | 1 + .../typecheck/test/type_check_block_spec.ts | 1 + .../src/ngtsc/typecheck/testing/index.ts | 5 + .../src/ngtsc/validation/BUILD.bazel | 1 + .../src/ngtsc/validation/src/config.ts | 13 + .../rules/unused_standalone_imports_rule.ts | 140 ++++++++ .../validation/src/source_file_validator.ts | 20 +- .../test/ngtsc/standalone_spec.ts | 2 +- .../test/ngtsc/template_typecheck_spec.ts | 302 ++++++++++++++++++ 24 files changed, 576 insertions(+), 6 deletions(-) create mode 100644 adev/src/content/reference/extended-diagnostics/NG8113.md create mode 100644 packages/compiler-cli/src/ngtsc/validation/src/config.ts create mode 100644 packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts diff --git a/adev/src/app/sub-navigation-data.ts b/adev/src/app/sub-navigation-data.ts index c46b6e5da8b78..2f71db14394e6 100644 --- a/adev/src/app/sub-navigation-data.ts +++ b/adev/src/app/sub-navigation-data.ts @@ -1368,6 +1368,11 @@ const REFERENCE_SUB_NAVIGATION_DATA: NavigationItem[] = [ path: 'extended-diagnostics/NG8111', contentPath: 'reference/extended-diagnostics/NG8111', }, + { + label: 'NG8113: Unused Standalone Imports', + path: 'extended-diagnostics/NG8113', + contentPath: 'reference/extended-diagnostics/NG8113', + }, ], }, { diff --git a/adev/src/content/reference/extended-diagnostics/NG8113.md b/adev/src/content/reference/extended-diagnostics/NG8113.md new file mode 100644 index 0000000000000..92a50a251cd8d --- /dev/null +++ b/adev/src/content/reference/extended-diagnostics/NG8113.md @@ -0,0 +1,48 @@ +# Unused Standalone Imports + +This diagnostic detects cases where the `imports` array of a `@Component` contains symbols that +aren't used within the template. + + + +@Component({ + imports: [UsedDirective, UnusedPipe] +}) +class AwesomeCheckbox {} + + + +## What's wrong with that? + +The unused imports add unnecessary noise to your code and can increase your compilation time. + +## What should I do instead? + +Delete the unused import. + + + +@Component({ + imports: [UsedDirective] +}) +class AwesomeCheckbox {} + + + +## What if I can't avoid this? + +This diagnostic can be disabled by editing the project's `tsconfig.json` file: + + +{ + "angularCompilerOptions": { + "extendedDiagnostics": { + "checks": { + "unusedStandaloneImports": "suppress" + } + } + } +} + + +See [extended diagnostic configuration](extended-diagnostics#configuration) for more info. diff --git a/adev/src/content/reference/extended-diagnostics/overview.md b/adev/src/content/reference/extended-diagnostics/overview.md index d64df7a3614b9..94714804e9d96 100644 --- a/adev/src/content/reference/extended-diagnostics/overview.md +++ b/adev/src/content/reference/extended-diagnostics/overview.md @@ -20,6 +20,7 @@ Currently, Angular supports the following extended diagnostics: | `NG8108` | [`skipHydrationNotStatic`](extended-diagnostics/NG8108) | | `NG8109` | [`interpolatedSignalNotInvoked`](extended-diagnostics/NG8109) | | `NG8111` | [`uninvokedFunctionInEventBinding`](extended-diagnostics/NG8111) | +| `NG8113` | [`unusedStandaloneImports`](extended-diagnostics/NG8113) | ## Configuration diff --git a/goldens/public-api/compiler-cli/error_code.api.md b/goldens/public-api/compiler-cli/error_code.api.md index 7c307e96f2ff0..011fcd785e62c 100644 --- a/goldens/public-api/compiler-cli/error_code.api.md +++ b/goldens/public-api/compiler-cli/error_code.api.md @@ -106,6 +106,7 @@ export enum ErrorCode { UNINVOKED_FUNCTION_IN_EVENT_BINDING = 8111, UNSUPPORTED_INITIALIZER_API_USAGE = 8110, UNUSED_LET_DECLARATION = 8112, + UNUSED_STANDALONE_IMPORTS = 8113, // (undocumented) VALUE_HAS_WRONG_TYPE = 1010, // (undocumented) diff --git a/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md b/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md index 36777b64fa9fe..ee99d7d584756 100644 --- a/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md +++ b/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md @@ -29,7 +29,9 @@ export enum ExtendedTemplateDiagnosticName { // (undocumented) UNINVOKED_FUNCTION_IN_EVENT_BINDING = "uninvokedFunctionInEventBinding", // (undocumented) - UNUSED_LET_DECLARATION = "unusedLetDeclaration" + UNUSED_LET_DECLARATION = "unusedLetDeclaration", + // (undocumented) + UNUSED_STANDALONE_IMPORTS = "unusedStandaloneImports" } // (No @packageDocumentation comment for this package) diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts index 24b7c3b2456a1..287fa1150619f 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts @@ -902,6 +902,7 @@ export class ComponentDecoratorHandler isStandalone: analysis.meta.isStandalone, isSignal: analysis.meta.isSignal, imports: analysis.resolvedImports, + rawImports: analysis.rawImports, deferredImports: analysis.resolvedDeferredImports, animationTriggerNames: analysis.animationTriggerNames, schemas: analysis.schemas, diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts index fcf1861a5234a..8b03f1d7ba7b7 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts @@ -270,6 +270,7 @@ export class DirectiveDecoratorHandler isStandalone: analysis.meta.isStandalone, isSignal: analysis.meta.isSignal, imports: null, + rawImports: null, deferredImports: null, schemas: null, ngContentSelectors: null, diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 441293dbd2a99..b8f2ded863b5e 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -1037,6 +1037,8 @@ export class NgCompiler { suggestionsForSuboptimalTypeInference: this.enableTemplateTypeChecker && !strictTemplates, controlFlowPreventingContentProjection: this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning, + unusedStandaloneImports: + this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning, allowSignalsInTwoWayBindings, }; } else { @@ -1069,6 +1071,8 @@ export class NgCompiler { suggestionsForSuboptimalTypeInference: false, controlFlowPreventingContentProjection: this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning, + unusedStandaloneImports: + this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning, allowSignalsInTwoWayBindings, }; } @@ -1114,6 +1118,10 @@ export class NgCompiler { typeCheckingConfig.controlFlowPreventingContentProjection = this.options.extendedDiagnostics.checks.controlFlowPreventingContentProjection; } + if (this.options.extendedDiagnostics?.checks?.unusedStandaloneImports !== undefined) { + typeCheckingConfig.unusedStandaloneImports = + this.options.extendedDiagnostics.checks.unusedStandaloneImports; + } return typeCheckingConfig; } @@ -1541,11 +1549,12 @@ export class NgCompiler { }, ); + const typeCheckingConfig = this.getTypeCheckingConfig(); const templateTypeChecker = new TemplateTypeCheckerImpl( this.inputProgram, notifyingDriver, traitCompiler, - this.getTypeCheckingConfig(), + typeCheckingConfig, refEmitter, reflector, this.adapter, @@ -1576,7 +1585,7 @@ export class NgCompiler { const sourceFileValidator = this.constructionDiagnostics.length === 0 - ? new SourceFileValidator(reflector, importTracker) + ? new SourceFileValidator(reflector, importTracker, templateTypeChecker, typeCheckingConfig) : null; return { diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts index 5902c44e8abaf..1eb0c48588817 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts @@ -50,10 +50,11 @@ export function makeDiagnostic( node: ts.Node, messageText: string | ts.DiagnosticMessageChain, relatedInformation?: ts.DiagnosticRelatedInformation[], + category: ts.DiagnosticCategory = ts.DiagnosticCategory.Error, ): ts.DiagnosticWithLocation { node = ts.getOriginalNode(node); return { - category: ts.DiagnosticCategory.Error, + category, code: ngErrorCode(code), file: ts.getOriginalNode(node).getSourceFile(), start: node.getStart(undefined, false), diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 14584d91f7783..e2fb34ff493df 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -508,6 +508,11 @@ export enum ErrorCode { */ UNUSED_LET_DECLARATION = 8112, + /** + * A symbol referenced in `@Component.imports` isn't being used within the template. + */ + UNUSED_STANDALONE_IMPORTS = 8113, + /** * The template type-checking engine would need to generate an inline type check block for a * component, but the current type-checking environment doesn't support it. diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts index 1b968e610b136..7b847e70946de 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts @@ -28,4 +28,5 @@ export enum ExtendedTemplateDiagnosticName { INTERPOLATED_SIGNAL_NOT_INVOKED = 'interpolatedSignalNotInvoked', CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 'controlFlowPreventingContentProjection', UNUSED_LET_DECLARATION = 'unusedLetDeclaration', + UNUSED_STANDALONE_IMPORTS = 'unusedStandaloneImports', } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index 6cbc25598b4fa..207e114ef282f 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -244,6 +244,11 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta { */ imports: Reference[] | null; + /** + * Node declaring the `imports` of a standalone component. Used to produce diagnostics. + */ + rawImports: ts.Expression | null; + /** * For standalone components, the list of imported types that can be used * in `@defer` blocks (when only explicit dependencies are allowed). diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index 19f1b0c038600..43a2d3b458fe6 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -181,6 +181,7 @@ export class DtsMetadataReader implements MetadataReader { // Imports are tracked in metadata only for template type-checking purposes, // so standalone components from .d.ts files don't have any. imports: null, + rawImports: null, deferredImports: null, // The same goes for schemas. schemas: null, diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index 6c44f7aa30389..68834e99f610a 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -346,6 +346,7 @@ function fakeDirective(ref: Reference): DirectiveMeta { isStandalone: false, isSignal: false, imports: null, + rawImports: null, schemas: null, decorator: null, hostDirectives: null, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index b0d6591765854..654db37a0c7ba 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -40,6 +40,8 @@ export interface TypeCheckableDirectiveMeta extends DirectiveMeta, DirectiveType hostDirectives: HostDirectiveMeta[] | null; decorator: ts.Decorator | null; isExplicitlyDeferred: boolean; + imports: Reference[] | null; + rawImports: ts.Expression | null; } export type TemplateId = string & {__brand: 'TemplateId'}; @@ -294,6 +296,11 @@ export interface TypeCheckingConfig { */ controlFlowPreventingContentProjection: 'error' | 'warning' | 'suppress'; + /** + * Whether to check if `@Component.imports` contains unused symbols. + */ + unusedStandaloneImports: 'error' | 'warning' | 'suppress'; + /** * Whether to use any generic types of the context component. * diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts index c56e9d945aa04..ed7a9a5f79e3b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts @@ -40,5 +40,6 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory< export const SUPPORTED_DIAGNOSTIC_NAMES = new Set([ ExtendedTemplateDiagnosticName.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION, + ExtendedTemplateDiagnosticName.UNUSED_STANDALONE_IMPORTS, ...ALL_DIAGNOSTIC_FACTORIES.map((factory) => factory.name), ]); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 82fd517d0c64f..0b76073392991 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -963,6 +963,7 @@ describe('type check blocks', () => { useInlineTypeConstructors: true, suggestionsForSuboptimalTypeInference: false, controlFlowPreventingContentProjection: 'warning', + unusedStandaloneImports: 'warning', allowSignalsInTwoWayBindings: true, }; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index b8ef092605670..e7ccdcdf6e921 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -282,6 +282,7 @@ export const ALL_ENABLED_CONFIG: Readonly = { useInlineTypeConstructors: true, suggestionsForSuboptimalTypeInference: false, controlFlowPreventingContentProjection: 'warning', + unusedStandaloneImports: 'warning', allowSignalsInTwoWayBindings: true, }; @@ -414,6 +415,7 @@ export function tcb( checkControlFlowBodies: true, alwaysCheckSchemaInTemplateBodies: true, controlFlowPreventingContentProjection: 'warning', + unusedStandaloneImports: 'warning', strictSafeNavigationTypes: true, useContextGenericType: true, strictLiteralTypes: true, @@ -893,6 +895,8 @@ function getDirectiveMetaFromDeclaration( ngContentSelectors: decl.ngContentSelectors || null, preserveWhitespaces: decl.preserveWhitespaces ?? false, isExplicitlyDeferred: false, + imports: decl.imports, + rawImports: null, hostDirectives: decl.hostDirectives === undefined ? null @@ -948,6 +952,7 @@ function makeScope(program: ts.Program, sf: ts.SourceFile, decls: TestDeclaratio isStandalone: false, isSignal: false, imports: null, + rawImports: null, deferredImports: null, schemas: null, decorator: null, diff --git a/packages/compiler-cli/src/ngtsc/validation/BUILD.bazel b/packages/compiler-cli/src/ngtsc/validation/BUILD.bazel index 7a201566e688a..fe862ea541a9e 100644 --- a/packages/compiler-cli/src/ngtsc/validation/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/validation/BUILD.bazel @@ -12,6 +12,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/reflection", + "//packages/compiler-cli/src/ngtsc/typecheck/api", "@npm//@types/node", "@npm//typescript", ], diff --git a/packages/compiler-cli/src/ngtsc/validation/src/config.ts b/packages/compiler-cli/src/ngtsc/validation/src/config.ts new file mode 100644 index 0000000000000..ba93a3a32f4b1 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/validation/src/config.ts @@ -0,0 +1,13 @@ +/*! + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/** + * Whether the rule to check for unused standalone imports is enabled. + * Used to disable it conditionally in internal builds. + */ +export const UNUSED_STANDALONE_IMPORTS_RULE_ENABLED = true; 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 new file mode 100644 index 0000000000000..45c32050f0c49 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts @@ -0,0 +1,140 @@ +/*! + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import ts from 'typescript'; + +import {ErrorCode, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics'; +import {ImportedSymbolsTracker, Reference} from '../../../imports'; +import { + TemplateTypeChecker, + TypeCheckableDirectiveMeta, + TypeCheckingConfig, +} from '../../../typecheck/api'; + +import {SourceFileValidatorRule} from './api'; + +/** + * Rule that flags unused symbols inside of the `imports` array of a component. + */ +export class UnusedStandaloneImportsRule implements SourceFileValidatorRule { + constructor( + private templateTypeChecker: TemplateTypeChecker, + private typeCheckingConfig: TypeCheckingConfig, + private importedSymbolsTracker: ImportedSymbolsTracker, + ) {} + + shouldCheck(sourceFile: ts.SourceFile): boolean { + return ( + this.typeCheckingConfig.unusedStandaloneImports !== 'suppress' && + (this.importedSymbolsTracker.hasNamedImport(sourceFile, 'Component', '@angular/core') || + this.importedSymbolsTracker.hasNamespaceImport(sourceFile, '@angular/core')) + ); + } + + checkNode(node: ts.Node): ts.Diagnostic | null { + if (!ts.isClassDeclaration(node)) { + return null; + } + + const metadata = this.templateTypeChecker.getDirectiveMetadata(node); + + if ( + !metadata || + !metadata.isStandalone || + metadata.rawImports === null || + metadata.imports === null || + metadata.imports.length === 0 + ) { + return null; + } + + const usedDirectives = this.templateTypeChecker.getUsedDirectives(node); + const usedPipes = this.templateTypeChecker.getUsedPipes(node); + + // These will be null if the component is invalid for some reason. + if (!usedDirectives || !usedPipes) { + return null; + } + + const unused = this.getUnusedSymbols( + metadata, + new Set(usedDirectives.map((dir) => dir.ref.node as ts.ClassDeclaration)), + new Set(usedPipes), + ); + + if (unused === null) { + return null; + } + + const category = + this.typeCheckingConfig.unusedStandaloneImports === 'error' + ? ts.DiagnosticCategory.Error + : ts.DiagnosticCategory.Warning; + + if (unused.length === metadata.imports.length) { + return makeDiagnostic( + ErrorCode.UNUSED_STANDALONE_IMPORTS, + metadata.rawImports, + 'All imports are unused', + undefined, + category, + ); + } + + return makeDiagnostic( + ErrorCode.UNUSED_STANDALONE_IMPORTS, + metadata.rawImports, + 'Imports array contains unused imports', + unused.map(([ref, type, name]) => + makeRelatedInformation( + ref.getOriginForDiagnostics(metadata.rawImports!), + `${type} "${name}" is not used within the template`, + ), + ), + category, + ); + } + + private getUnusedSymbols( + metadata: TypeCheckableDirectiveMeta, + usedDirectives: Set, + usedPipes: Set, + ) { + if (metadata.imports === null || metadata.rawImports === null) { + return null; + } + + let unused: [ref: Reference, type: string, name: string][] | null = null; + + for (const current of metadata.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))) { + unused ??= []; + unused.push([current, dirMeta.isComponent ? 'Component' : 'Directive', dirMeta.name]); + } + continue; + } + + const pipeMeta = this.templateTypeChecker.getPipeMetadata(currentNode); + + if ( + pipeMeta !== null && + pipeMeta.isStandalone && + (usedPipes === null || !usedPipes.has(pipeMeta.name)) + ) { + unused ??= []; + unused.push([current, 'Pipe', pipeMeta.ref.node.name.text]); + } + } + + return unused; + } +} diff --git a/packages/compiler-cli/src/ngtsc/validation/src/source_file_validator.ts b/packages/compiler-cli/src/ngtsc/validation/src/source_file_validator.ts index 63133ca0174c2..229bb0fca1cc7 100644 --- a/packages/compiler-cli/src/ngtsc/validation/src/source_file_validator.ts +++ b/packages/compiler-cli/src/ngtsc/validation/src/source_file_validator.ts @@ -13,6 +13,9 @@ import {ReflectionHost} from '../../reflection'; import {SourceFileValidatorRule} from './rules/api'; import {InitializerApiUsageRule} from './rules/initializer_api_usage_rule'; +import {UnusedStandaloneImportsRule} from './rules/unused_standalone_imports_rule'; +import {TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api'; +import {UNUSED_STANDALONE_IMPORTS_RULE_ENABLED} from './config'; /** * Validates that TypeScript files match a specific set of rules set by the Angular compiler. @@ -20,8 +23,23 @@ import {InitializerApiUsageRule} from './rules/initializer_api_usage_rule'; export class SourceFileValidator { private rules: SourceFileValidatorRule[]; - constructor(reflector: ReflectionHost, importedSymbolsTracker: ImportedSymbolsTracker) { + constructor( + reflector: ReflectionHost, + importedSymbolsTracker: ImportedSymbolsTracker, + templateTypeChecker: TemplateTypeChecker, + typeCheckingConfig: TypeCheckingConfig, + ) { this.rules = [new InitializerApiUsageRule(reflector, importedSymbolsTracker)]; + + if (UNUSED_STANDALONE_IMPORTS_RULE_ENABLED) { + this.rules.push( + new UnusedStandaloneImportsRule( + templateTypeChecker, + typeCheckingConfig, + importedSymbolsTracker, + ), + ); + } } /** diff --git a/packages/compiler-cli/test/ngtsc/standalone_spec.ts b/packages/compiler-cli/test/ngtsc/standalone_spec.ts index 0cdd48c9c1752..3ccb5d93cbe2e 100644 --- a/packages/compiler-cli/test/ngtsc/standalone_spec.ts +++ b/packages/compiler-cli/test/ngtsc/standalone_spec.ts @@ -1127,7 +1127,7 @@ runInEachFileSystem(() => { standalone: true, selector: 'standalone-cmp', imports: [DepCmp], - template: '', + template: '', }) export class StandaloneCmp {} diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 7d73f906eaaab..c349070852014 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -7438,5 +7438,307 @@ suppress ); }); }); + + describe('unused standalone imports', () => { + it('should report when a directive is not used within a template', () => { + 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'; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: [UsedDir, UnusedDir] + }) + 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 when a pipe is not used within a template', () => { + env.write( + 'used.ts', + ` + import {Pipe} from '@angular/core'; + + @Pipe({name: 'used', standalone: true}) + export class UsedPipe { + transform(value: number) { + return value * 2; + } + } + `, + ); + + env.write( + 'unused.ts', + ` + import {Pipe} from '@angular/core'; + + @Pipe({name: 'unused', standalone: true}) + export class UnusedPipe { + transform(value: number) { + return value * 2; + } + } + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {UsedPipe} from './used'; + import {UnusedPipe} from './unused'; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: [UsedPipe, UnusedPipe] + }) + 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( + 'Pipe "UnusedPipe" is not used within the template', + ); + }); + + it('should not report imports only used inside @defer blocks', () => { + env.write( + 'test.ts', + ` + import {Component, Directive, Pipe} from '@angular/core'; + + @Directive({selector: '[used]', standalone: true}) + export class UsedDir {} + + @Pipe({name: 'used', standalone: true}) + export class UsedPipe { + transform(value: number) { + return value * 2; + } + } + + @Component({ + template: \` +
+ @defer (on idle) { +
+ + } +
+ \`, + standalone: true, + imports: [UsedDir, UsedPipe] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should report when all imports in an import array are not used', () => { + env.write( + 'test.ts', + ` + import {Component, Directive, Pipe} from '@angular/core'; + + @Directive({selector: '[unused]', standalone: true}) + export class UnusedDir {} + + @Pipe({name: 'unused', standalone: true}) + export class UnusedPipe { + transform(value: number) { + return value * 2; + } + } + + @Component({ + template: '', + standalone: true, + imports: [UnusedDir, UnusedPipe] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe('All imports are unused'); + expect(diags[0].relatedInformation).toBeFalsy(); + }); + + it('should not report unused imports coming from modules', () => { + env.write( + 'module.ts', + ` + import {Directive, NgModule} from '@angular/core'; + + @Directive({selector: '[unused-from-module]'}) + export class UnusedDirFromModule {} + + @NgModule({ + declarations: [UnusedDirFromModule], + exports: [UnusedDirFromModule] + }) + export class UnusedModule {} + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {UnusedModule} from './module'; + + @Component({ + template: '', + standalone: true, + imports: [UnusedModule] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should be able to opt out for checking for unused imports via the tsconfig', () => { + env.tsconfig({ + extendedDiagnostics: { + checks: { + unusedStandaloneImports: DiagnosticCategoryLabel.Suppress, + }, + }, + }); + + env.write( + 'test.ts', + ` + import {Component, Directive} from '@angular/core'; + + @Directive({selector: '[unused]', standalone: true}) + export class UnusedDir {} + + @Component({ + template: '', + standalone: true, + imports: [UnusedDir] + }) + export class MyComp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should unused imports from external modules', () => { + // Note: we don't use the existing fake `@angular/common`, + // because all the declarations there are non-standalone. + env.write( + 'node_modules/fake-common/index.d.ts', + ` + import * as i0 from '@angular/core'; + + export declare class NgIf { + static ɵdir: i0.ɵɵDirectiveDeclaration, "[ngIf]", never, {}, {}, never, never, true, never>; + static ɵfac: i0.ɵɵFactoryDeclaration, never>; + } + + export declare class NgFor { + static ɵdir: i0.ɵɵDirectiveDeclaration, "[ngFor]", never, {}, {}, never, never, true, never>; + static ɵfac: i0.ɵɵFactoryDeclaration, never>; + } + + export class PercentPipe { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵpipe: i0.ɵɵPipeDeclaration; + } + `, + ); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + import {NgIf, NgFor, PercentPipe} from 'fake-common'; + + @Component({ + template: \` +
+
+ +
+ \`, + standalone: true, + imports: [NgFor, NgIf, PercentPipe] + }) + 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(2); + expect(diags[0].relatedInformation![0].messageText).toBe( + 'Directive "NgFor" is not used within the template', + ); + expect(diags[0].relatedInformation![1].messageText).toBe( + 'Pipe "PercentPipe" is not used within the template', + ); + }); + }); }); });