refactor(compiler-cli): remove ModuleWithProviders generic type transform (#41996)

The `ModuleWithProviders` type has required a generic type since Angular 10,
so it is no longer necessary for the compiler to transform usages of the
`ModuleWithProviders` type without the generic type, as that should have
been reported as a compile error. This commit removes the detection logic
from ngtsc.

PR Close #41996

packages/compiler-cli/src/ngtsc/core/BUILD.bazel

@@ -23,7 +23,6 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/incremental/semantic_graph",
         "//packages/compiler-cli/src/ngtsc/indexer",
         "//packages/compiler-cli/src/ngtsc/metadata",
-        "//packages/compiler-cli/src/ngtsc/modulewithproviders",
         "//packages/compiler-cli/src/ngtsc/partial_evaluator",
         "//packages/compiler-cli/src/ngtsc/perf",
         "//packages/compiler-cli/src/ngtsc/program_driver",

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Type} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, NoopReferencesRegistry, PipeDecoratorHandler, ReferencesRegistry} from '../../annotations';
@@ -19,7 +18,6 @@ import {IncrementalBuildStrategy, IncrementalCompilation, IncrementalState} from
 import {SemanticSymbol} from '../../incremental/semantic_graph';
 import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer';
 import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, MetadataReader, PipeMeta, ResourceRegistry} from '../../metadata';
-import {ModuleWithProvidersScanner} from '../../modulewithproviders';
 import {PartialEvaluator} from '../../partial_evaluator';
 import {ActivePerfRecorder, DelegatingPerfRecorder, PerfCheckpoint, PerfEvent, PerfPhase} from '../../perf';
 import {ProgramDriver, UpdateMode} from '../../program_driver';
@@ -51,7 +49,6 @@ interface LazyCompilationState {
   exportReferenceGraph: ReferenceGraph|null;
   routeAnalyzer: NgModuleRouteAnalyzer;
   dtsTransforms: DtsTransformRegistry;
-  mwpScanner: ModuleWithProvidersScanner;
   aliasingHost: AliasingHost|null;
   refEmitter: ReferenceEmitter;
   templateTypeChecker: TemplateTypeChecker;
@@ -565,7 +562,6 @@ export class NgCompiler {
         }
 
         let analysisPromise = this.compilation.traitCompiler.analyzeAsync(sf);
-        this.scanForMwp(sf);
         if (analysisPromise !== undefined) {
           promises.push(analysisPromise);
         }
@@ -693,7 +689,6 @@ export class NgCompiler {
           continue;
         }
         this.compilation.traitCompiler.analyzeSync(sf);
-        this.scanForMwp(sf);
       }
 
       this.perfRecorder.memory(PerfCheckpoint.Analysis);
@@ -888,16 +883,6 @@ export class NgCompiler {
     return this.nonTemplateDiagnostics;
   }
 
-  private scanForMwp(sf: ts.SourceFile): void {
-    this.compilation!.mwpScanner.scan(sf, {
-      addTypeReplacement: (node: ts.Declaration, type: Type): void => {
-        // Only obtain the return type transform for the source file once there's a type to replace,
-        // so that no transform is allocated when there's nothing to do.
-        this.compilation!.dtsTransforms!.getReturnTypeTransform(sf).addTypeReplacement(node, type);
-      }
-    });
-  }
-
   private makeCompilation(): LazyCompilationState {
     const checker = this.inputProgram.getTypeChecker();
 
@@ -992,8 +977,6 @@ export class NgCompiler {
 
     const dtsTransforms = new DtsTransformRegistry();
 
-    const mwpScanner = new ModuleWithProvidersScanner(reflector, evaluator, refEmitter);
-
     const isCore = isAngularCorePackage(this.inputProgram);
 
     const resourceRegistry = new ResourceRegistry();
@@ -1071,7 +1054,6 @@ export class NgCompiler {
       dtsTransforms,
       exportReferenceGraph,
       routeAnalyzer,
-      mwpScanner,
       metaReader,
       typeCheckScopeRegistry,
       aliasingHost,
@@ -1207,4 +1189,4 @@ function versionMapFromProgram(
     versions.set(absoluteFromSourceFile(sf), driver.getSourceFileVersion(sf));
   }
   return versions;
-}
+}

packages/compiler-cli/src/ngtsc/modulewithproviders/BUILD.bazel

@@ -1,17 +0,0 @@
-load("//tools:defaults.bzl", "ts_library")
-
-package(default_visibility = ["//visibility:public"])
-
-ts_library(
-    name = "modulewithproviders",
-    srcs = ["index.ts"] + glob([
-        "src/**/*.ts",
-    ]),
-    deps = [
-        "//packages/compiler",
-        "//packages/compiler-cli/src/ngtsc/imports",
-        "//packages/compiler-cli/src/ngtsc/partial_evaluator",
-        "//packages/compiler-cli/src/ngtsc/reflection",
-        "@npm//typescript",
-    ],
-)

packages/compiler-cli/src/ngtsc/modulewithproviders/index.ts

@@ -1,9 +0,0 @@
-/**
- * @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
- */
-
-export * from './src/scanner';

packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts

@@ -1,166 +0,0 @@
-/**
- * @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 {ExpressionType, ExternalExpr, R3Identifiers as Identifiers, Type} from '@angular/compiler';
-import * as ts from 'typescript';
-
-import {ImportFlags, Reference, ReferenceEmitter} from '../../imports';
-import {PartialEvaluator, ResolvedValueMap} from '../../partial_evaluator';
-import {ReflectionHost} from '../../reflection';
-
-export interface DtsHandler {
-  addTypeReplacement(node: ts.Declaration, type: Type): void;
-}
-
-export class ModuleWithProvidersScanner {
-  constructor(
-      private host: ReflectionHost, private evaluator: PartialEvaluator,
-      private emitter: ReferenceEmitter) {}
-
-  scan(sf: ts.SourceFile, dts: DtsHandler): void {
-    for (const stmt of sf.statements) {
-      this.visitStatement(dts, stmt);
-    }
-  }
-
-  private visitStatement(dts: DtsHandler, stmt: ts.Statement): void {
-    // Detect whether a statement is exported, which is used as one of the hints whether to look
-    // more closely at possible MWP functions within. This is a syntactic check, not a semantic
-    // check, so it won't detect cases like:
-    //
-    // var X = ...;
-    // export {X}
-    //
-    // This is intentional, because the alternative is slow and this will catch 99% of the cases we
-    // need to handle.
-    const isExported = stmt.modifiers !== undefined &&
-        stmt.modifiers.some(mod => mod.kind === ts.SyntaxKind.ExportKeyword);
-
-    if (!isExported) {
-      return;
-    }
-
-    if (ts.isClassDeclaration(stmt)) {
-      for (const member of stmt.members) {
-        if (!ts.isMethodDeclaration(member) || !isStatic(member)) {
-          continue;
-        }
-
-        this.visitFunctionOrMethodDeclaration(dts, member);
-      }
-    } else if (ts.isFunctionDeclaration(stmt)) {
-      this.visitFunctionOrMethodDeclaration(dts, stmt);
-    }
-  }
-
-  private visitFunctionOrMethodDeclaration(
-      dts: DtsHandler, decl: ts.MethodDeclaration|ts.FunctionDeclaration): void {
-    // First, some sanity. This should have a method body with a single return statement.
-    if (decl.body === undefined || decl.body.statements.length !== 1) {
-      return;
-    }
-    const retStmt = decl.body.statements[0];
-    if (!ts.isReturnStatement(retStmt) || retStmt.expression === undefined) {
-      return;
-    }
-    const retValue = retStmt.expression;
-
-    // Now, look at the return type of the method. Maybe bail if the type is already marked, or if
-    // it's incompatible with a MWP function.
-    const returnType = this.returnTypeOf(decl);
-    if (returnType === ReturnType.OTHER || returnType === ReturnType.MWP_WITH_TYPE) {
-      // Don't process this declaration, it either already declares the right return type, or an
-      // incompatible one.
-      return;
-    }
-
-    const value = this.evaluator.evaluate(retValue);
-    if (!(value instanceof Map) || !value.has('ngModule')) {
-      // The return value does not provide sufficient information to be able to add a generic type.
-      return;
-    }
-
-    if (returnType === ReturnType.INFERRED && !isModuleWithProvidersType(value)) {
-      // The return type is inferred but the returned object is not of the correct shape, so we
-      // shouldn's modify the return type to become `ModuleWithProviders`.
-      return;
-    }
-
-    // The return type has been verified to represent the `ModuleWithProviders` type, but either the
-    // return type is inferred or the generic type argument is missing. In both cases, a new return
-    // type is created where the `ngModule` type is included as generic type argument.
-    const ngModule = value.get('ngModule');
-    if (!(ngModule instanceof Reference) || !ts.isClassDeclaration(ngModule.node)) {
-      return;
-    }
-
-    const ngModuleExpr =
-        this.emitter.emit(ngModule, decl.getSourceFile(), ImportFlags.ForceNewImport);
-    const ngModuleType = new ExpressionType(ngModuleExpr.expression);
-    const mwpNgType = new ExpressionType(
-        new ExternalExpr(Identifiers.ModuleWithProviders), [/* modifiers */], [ngModuleType]);
-
-    dts.addTypeReplacement(decl, mwpNgType);
-  }
-
-  private returnTypeOf(decl: ts.FunctionDeclaration|ts.MethodDeclaration|
-                       ts.VariableDeclaration): ReturnType {
-    if (decl.type === undefined) {
-      return ReturnType.INFERRED;
-    } else if (!ts.isTypeReferenceNode(decl.type)) {
-      return ReturnType.OTHER;
-    }
-
-    // Try to figure out if the type is of a familiar form, something that looks like it was
-    // imported.
-    let typeId: ts.Identifier;
-    if (ts.isIdentifier(decl.type.typeName)) {
-      // def: ModuleWithProviders
-      typeId = decl.type.typeName;
-    } else if (ts.isQualifiedName(decl.type.typeName) && ts.isIdentifier(decl.type.typeName.left)) {
-      // def: i0.ModuleWithProviders
-      typeId = decl.type.typeName.right;
-    } else {
-      return ReturnType.OTHER;
-    }
-
-    const importDecl = this.host.getImportOfIdentifier(typeId);
-    if (importDecl === null || importDecl.from !== '@angular/core' ||
-        importDecl.name !== 'ModuleWithProviders') {
-      return ReturnType.OTHER;
-    }
-
-    if (decl.type.typeArguments === undefined || decl.type.typeArguments.length === 0) {
-      // The return type is indeed ModuleWithProviders, but no generic type parameter was found.
-      return ReturnType.MWP_NO_TYPE;
-    } else {
-      // The return type is ModuleWithProviders, and the user has already specified a generic type.
-      return ReturnType.MWP_WITH_TYPE;
-    }
-  }
-}
-
-enum ReturnType {
-  INFERRED,
-  MWP_NO_TYPE,
-  MWP_WITH_TYPE,
-  OTHER,
-}
-
-/** Whether the resolved value map represents a ModuleWithProviders object */
-function isModuleWithProvidersType(value: ResolvedValueMap): boolean {
-  const ngModule = value.has('ngModule');
-  const providers = value.has('providers');
-
-  return ngModule && (value.size === 1 || (providers && value.size === 2));
-}
-
-function isStatic(node: ts.Node): boolean {
-  return node.modifiers !== undefined &&
-      node.modifiers.some(mod => mod.kind === ts.SyntaxKind.StaticKeyword);
-}

packages/compiler-cli/src/ngtsc/transform/BUILD.bazel

@@ -15,7 +15,6 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/incremental:api",
         "//packages/compiler-cli/src/ngtsc/incremental/semantic_graph",
         "//packages/compiler-cli/src/ngtsc/indexer",
-        "//packages/compiler-cli/src/ngtsc/modulewithproviders",
         "//packages/compiler-cli/src/ngtsc/perf",
         "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/translator",

packages/compiler-cli/test/ngtsc/modulewithproviders_spec.ts

@@ -1,298 +0,0 @@
-/**
- * @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 {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
-import {loadStandardTestFiles} from '../../src/ngtsc/testing';
-
-import {NgtscTestEnvironment} from './env';
-
-const trim = (input: string): string => input.replace(/\s+/g, ' ').trim();
-
-const testFiles = loadStandardTestFiles();
-
-runInEachFileSystem(() => {
-  describe('ModuleWithProviders generic type transform', () => {
-    let env!: NgtscTestEnvironment;
-
-    beforeEach(() => {
-      env = NgtscTestEnvironment.setup(testFiles);
-      env.tsconfig();
-    });
-
-    it('should add a generic type for static methods on exported classes', () => {
-      env.write('test.ts', `
-        import {NgModule} from '@angular/core';
-
-        @NgModule()
-        export class TestModule {
-          static forRoot() {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('import * as i0 from "@angular/core";');
-      expect(dtsContents).toContain('static forRoot(): i0.ModuleWithProviders<TestModule>;');
-    });
-
-    it('should not add a generic type for non-static methods', () => {
-      env.write('test.ts', `
-        import {NgModule} from '@angular/core';
-
-        @NgModule()
-        export class TestModule {
-          forRoot() {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('import * as i0 from "@angular/core";');
-      expect(dtsContents).toContain('forRoot(): { ngModule: typeof TestModule; };');
-      expect(dtsContents).not.toContain('static forRoot()');
-    });
-
-    it('should add a generic type for exported functions', () => {
-      env.write('test.ts', `
-        import {NgModule} from '@angular/core';
-
-        export function forRoot() {
-          return {
-            ngModule: TestModule,
-          };
-        }
-
-        @NgModule()
-        export class TestModule {}
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('import * as i0 from "@angular/core";');
-      expect(dtsContents)
-          .toContain('export declare function forRoot(): i0.ModuleWithProviders<TestModule>;');
-    });
-
-    it('should not add a generic type when already present', () => {
-      env.write('test.ts', `
-        import {NgModule, ModuleWithProviders} from '@angular/core';
-
-        export class TestModule {
-          forRoot(): ModuleWithProviders<InternalTestModule> {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-
-        @NgModule()
-        export class InternalTestModule {}
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('forRoot(): ModuleWithProviders<InternalTestModule>;');
-    });
-
-    it('should add a generic type when missing the generic type parameter', () => {
-      env.write('test.ts', `
-        import {NgModule, ModuleWithProviders} from '@angular/core';
-
-        @NgModule()
-        export class TestModule {
-          static forRoot(): ModuleWithProviders {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('static forRoot(): i0.ModuleWithProviders<TestModule>;');
-    });
-
-    it('should add a generic type when missing the generic type parameter (qualified name)', () => {
-      env.write('test.ts', `
-        import * as ng from '@angular/core';
-
-        @ng.NgModule()
-        export class TestModule {
-          static forRoot(): ng.ModuleWithProviders {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('static forRoot(): i0.ModuleWithProviders<TestModule>;');
-    });
-
-    it('should add a generic type and add an import for external references', () => {
-      env.write('test.ts', `
-        import {ModuleWithProviders} from '@angular/core';
-        import {InternalTestModule} from './internal';
-
-        export class TestModule {
-          static forRoot(): ModuleWithProviders {
-            return {
-              ngModule: InternalTestModule,
-            };
-          }
-        }
-      `);
-      env.write('internal.ts', `
-        import {NgModule} from '@angular/core';
-
-        @NgModule()
-        export class InternalTestModule {}
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('import * as i1 from "./internal";');
-      expect(dtsContents)
-          .toContain('static forRoot(): i0.ModuleWithProviders<i1.InternalTestModule>;');
-    });
-
-    it('should not add a generic type if the return type is not ModuleWithProviders', () => {
-      env.write('test.ts', `
-        import {NgModule} from '@angular/core';
-
-        @NgModule()
-        export class TestModule {
-          static forRoot(): { ngModule: typeof TestModule } {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('static forRoot(): { ngModule: typeof TestModule; };');
-    });
-
-    it('should not add a generic type if the return type is not ModuleWithProviders from @angular/core',
-       () => {
-         env.write('test.ts', `
-        import {NgModule} from '@angular/core';
-        import {ModuleWithProviders} from './mwp';
-
-        @NgModule()
-        export class TestModule {
-          static forRoot(): ModuleWithProviders {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-      `);
-         env.write('mwp.ts', `
-      export type ModuleWithProviders = { ngModule: any };
-      `);
-
-         env.driveMain();
-
-         const dtsContents = trim(env.getContents('test.d.ts'));
-         expect(dtsContents).toContain('static forRoot(): ModuleWithProviders;');
-       });
-
-    it('should not add a generic type when the "ngModule" property is not a reference', () => {
-      env.write('test.ts', `
-        import {NgModule} from '@angular/core';
-
-        @NgModule()
-        export class TestModule {
-          static forRoot() {
-            return {
-              ngModule: 'test',
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).toContain('static forRoot(): { ngModule: string; };');
-    });
-
-    it('should not add a generic type when the class is not exported', () => {
-      env.write('test.ts', `
-        import {NgModule} from '@angular/core';
-
-        @NgModule()
-        class TestModule {
-          static forRoot() {
-            return {
-              ngModule: TestModule,
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      // The TestModule class is not exported so doesn't even show up in the declaration file
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents).not.toContain('static forRoot()');
-    });
-
-    it('should add a generic type only when ngModule/providers are present', () => {
-      env.write('test.ts', `
-        import {NgModule, ModuleWithProviders} from '@angular/core';
-
-        @NgModule()
-        export class TestModule {
-          static hasNgModuleAndProviders() {
-            return {
-              ngModule: TestModule,
-              providers: [],
-            };
-          }
-          static hasNgModuleAndFoo() {
-            return {
-              ngModule: TestModule,
-              foo: 'test',
-            };
-          }
-        }
-      `);
-
-      env.driveMain();
-
-      const dtsContents = trim(env.getContents('test.d.ts'));
-      expect(dtsContents)
-          .toContain('static hasNgModuleAndProviders(): i0.ModuleWithProviders<TestModule>;');
-      expect(dtsContents)
-          .toContain('static hasNgModuleAndFoo(): { ngModule: typeof TestModule; foo: string; };');
-    });
-  });
-});