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
angular · Sep 3, 2024 · a2e4ee0 · a2e4ee0
1 parent a777bee
commit a2e4ee0
Show file tree

Hide file tree

Showing 24 changed files with 576 additions and 6 deletions.
diff --git 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
@@ -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.
+
+<docs-code language="typescript">
+
+@Component({
+  imports: [UsedDirective, UnusedPipe]
+})
+class AwesomeCheckbox {}
+
+</docs-code>
+
+## 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.
+
+<docs-code language="typescript">
+
+@Component({
+  imports: [UsedDirective]
+})
+class AwesomeCheckbox {}
+
+</docs-code>
+
+## What if I can't avoid this?
+
+This diagnostic can be disabled by editing the project's `tsconfig.json` file:
+
+<docs-code language="json">
+{
+  "angularCompilerOptions": {
+    "extendedDiagnostics": {
+      "checks": {
+        "unusedStandaloneImports": "suppress"
+      }
+    }
+  }
+}
+</docs-code>
+
+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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -244,6 +244,11 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta {
    */
   imports: Reference<ClassDeclaration>[] | 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
@@ -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
@@ -346,6 +346,7 @@ function fakeDirective(ref: Reference<ClassDeclaration>): 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
@@ -40,6 +40,8 @@ export interface TypeCheckableDirectiveMeta extends DirectiveMeta, DirectiveType
   hostDirectives: HostDirectiveMeta[] | null;
   decorator: ts.Decorator | null;
   isExplicitlyDeferred: boolean;
+  imports: Reference<ClassDeclaration>[] | 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
@@ -40,5 +40,6 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory<
 
 export const SUPPORTED_DIAGNOSTIC_NAMES = new Set<string>([
   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
@@ -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
@@ -282,6 +282,7 @@ export const ALL_ENABLED_CONFIG: Readonly<TypeCheckingConfig> = {
   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
@@ -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
@@ -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;