From 60aeee7abfcc149d9f33ed9519b242b4bccea641 Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Mon, 9 Jul 2018 11:36:30 -0700
Subject: [PATCH] feat(ivy): selector side of ModuleWithProviders via type
 metadata (#24862)

Within an @NgModule it's common to include in the imports a call to
a ModuleWithProviders function, for example RouterModule.forRoot().
The old ngc compiler was able to handle this pattern because it had
global knowledge of metadata of not only the input compilation unit
but also all dependencies.

The ngtsc compiler for Ivy doesn't have this knowledge, so the
pattern of ModuleWithProviders functions is more difficult. ngtsc
must be able to determine which module is imported via the function
in order to expand the selector scope and properly tree-shake
directives and pipes.

This commit implements a solution to this problem, by adding a type
parameter to ModuleWithProviders through which the actual module
type can be passed between compilation units.

The provider side isn't a problem because the imports are always
copied directly to the ngInjectorDef.

PR Close #24862
---
 .../src/ngtsc/annotations/src/ng_module.ts    | 48 ++++++++++++++++-
 .../src/ngtsc/metadata/src/resolver.ts        | 52 ++++++++++++++++---
 .../test/ngtsc/fake_core/index.ts             |  1 +
 .../compiler-cli/test/ngtsc/ngtsc_spec.ts     | 26 ++++++++++
 packages/core/src/metadata/ng_module.ts       |  7 +--
 packages/forms/src/form_providers.ts          |  2 +-
 packages/platform-browser/src/browser.ts      |  2 +-
 packages/router/src/router_module.ts          |  4 +-
 .../testing/src/router_testing_module.ts      |  3 +-
 tools/public_api_guard/core/core.d.ts         |  4 +-
 tools/public_api_guard/forms/forms.d.ts       |  2 +-
 .../platform-browser/platform-browser.d.ts    |  2 +-
 tools/public_api_guard/router/router.d.ts     |  4 +-
 tools/public_api_guard/router/testing.d.ts    |  2 +-
 14 files changed, 136 insertions(+), 23 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
index 54ccce6ae2..aac8f1bfef 100644
--- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
+++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
@@ -64,12 +64,16 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
     }
     let imports: Reference[] = [];
     if (ngModule.has('imports')) {
-      const importsMeta = staticallyResolve(ngModule.get('imports') !, this.checker);
+      const importsMeta = staticallyResolve(
+          ngModule.get('imports') !, this.checker,
+          node => this._extractModuleFromModuleWithProvidersFn(node));
       imports = resolveTypeList(importsMeta, 'imports');
     }
     let exports: Reference[] = [];
     if (ngModule.has('exports')) {
-      const exportsMeta = staticallyResolve(ngModule.get('exports') !, this.checker);
+      const exportsMeta = staticallyResolve(
+          ngModule.get('exports') !, this.checker,
+          node => this._extractModuleFromModuleWithProvidersFn(node));
       exports = resolveTypeList(exportsMeta, 'exports');
     }
 
@@ -125,6 +129,46 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
       },
     ];
   }
+
+  /**
+   * Given a `FunctionDeclaration` or `MethodDeclaration`, check if it is typed as a
+   * `ModuleWithProviders` and return an expression referencing the module if available.
+   */
+  private _extractModuleFromModuleWithProvidersFn(node: ts.FunctionDeclaration|
+                                                  ts.MethodDeclaration): ts.Expression|null {
+    const type = node.type;
+    // Examine the type of the function to see if it's a ModuleWithProviders reference.
+    if (type === undefined || !ts.isTypeReferenceNode(type) || !ts.isIdentifier(type.typeName)) {
+      return null;
+    }
+
+    // Look at the type itself to see where it comes from.
+    const id = this.reflector.getImportOfIdentifier(type.typeName);
+
+    // If it's not named ModuleWithProviders, bail.
+    if (id === null || id.name !== 'ModuleWithProviders') {
+      return null;
+    }
+
+    // If it's not from @angular/core, bail.
+    if (!this.isCore && id.from !== '@angular/core') {
+      return null;
+    }
+
+    // If there's no type parameter specified, bail.
+    if (type.typeArguments === undefined || type.typeArguments.length !== 1) {
+      return null;
+    }
+
+    const arg = type.typeArguments[0];
+
+    // If the argument isn't an Identifier, bail.
+    if (!ts.isTypeReferenceNode(arg) || !ts.isIdentifier(arg.typeName)) {
+      return null;
+    }
+
+    return arg.typeName;
+  }
 }
 
 /**
diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts
index 9740d4e222..9c3f7891f7 100644
--- a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts
+++ b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts
@@ -107,6 +107,8 @@ export abstract class Reference {
  * referenceable.
  */
 export class NodeReference extends Reference {
+  constructor(node: ts.Node, readonly moduleName: string|null) { super(node); }
+
   toExpression(context: ts.SourceFile): null { return null; }
 }
 
@@ -177,11 +179,22 @@ export class AbsoluteReference extends Reference {
  *
  * @param node the expression to statically resolve if possible
  * @param checker a `ts.TypeChecker` used to understand the expression
+ * @param foreignFunctionResolver a function which will be used whenever a "foreign function" is
+ * encountered. A foreign function is a function which has no body - usually the result of calling
+ * a function declared in another library's .d.ts file. In these cases, the foreignFunctionResolver
+ * will be called with the function's declaration, and can optionally return a `ts.Expression`
+ * (possibly extracted from the foreign function's type signature) which will be used as the result
+ * of the call.
  * @returns a `ResolvedValue` representing the resolved value
  */
-export function staticallyResolve(node: ts.Expression, checker: ts.TypeChecker): ResolvedValue {
-  return new StaticInterpreter(checker).visit(
-      node, {absoluteModuleName: null, scope: new Map<ts.ParameterDeclaration, ResolvedValue>()});
+export function staticallyResolve(
+    node: ts.Expression, checker: ts.TypeChecker,
+    foreignFunctionResolver?: (node: ts.FunctionDeclaration | ts.MethodDeclaration) =>
+        ts.Expression | null): ResolvedValue {
+  return new StaticInterpreter(checker).visit(node, {
+    absoluteModuleName: null,
+    scope: new Map<ts.ParameterDeclaration, ResolvedValue>(), foreignFunctionResolver,
+  });
 }
 
 interface BinaryOperatorDef {
@@ -226,6 +239,7 @@ const UNARY_OPERATORS = new Map<ts.SyntaxKind, (a: any) => any>([
 interface Context {
   absoluteModuleName: string|null;
   scope: Scope;
+  foreignFunctionResolver?(node: ts.FunctionDeclaration|ts.MethodDeclaration): ts.Expression|null;
 }
 
 class StaticInterpreter {
@@ -472,6 +486,10 @@ class StaticInterpreter {
     } else if (lhs instanceof Reference) {
       const ref = lhs.node;
       if (ts.isClassDeclaration(ref)) {
+        let absoluteModuleName = context.absoluteModuleName;
+        if (lhs instanceof NodeReference || lhs instanceof AbsoluteReference) {
+          absoluteModuleName = lhs.moduleName || absoluteModuleName;
+        }
         let value: ResolvedValue = undefined;
         const member =
             ref.members.filter(member => isStatic(member))
@@ -482,7 +500,7 @@ class StaticInterpreter {
           if (ts.isPropertyDeclaration(member) && member.initializer !== undefined) {
             value = this.visitExpression(member.initializer, context);
           } else if (ts.isMethodDeclaration(member)) {
-            value = new NodeReference(member);
+            value = new NodeReference(member, absoluteModuleName);
           }
         }
         return value;
@@ -495,13 +513,35 @@ class StaticInterpreter {
     const lhs = this.visitExpression(node.expression, context);
     if (!(lhs instanceof Reference)) {
       throw new Error(`attempting to call something that is not a function: ${lhs}`);
-    } else if (!isFunctionOrMethodDeclaration(lhs.node) || !lhs.node.body) {
+    } else if (!isFunctionOrMethodDeclaration(lhs.node)) {
       throw new Error(
           `calling something that is not a function declaration? ${ts.SyntaxKind[lhs.node.kind]}`);
     }
 
     const fn = lhs.node;
-    const body = fn.body as ts.Block;
+
+    // If the function is foreign (declared through a d.ts file), attempt to resolve it with the
+    // foreignFunctionResolver, if one is specified.
+    if (fn.body === undefined) {
+      let expr: ts.Expression|null = null;
+      if (context.foreignFunctionResolver) {
+        expr = context.foreignFunctionResolver(fn);
+      }
+      if (expr === null) {
+        throw new Error(`could not resolve foreign function declaration`);
+      }
+
+      // If the function is declared in a different file, resolve the foreign function expression
+      // using the absolute module name of that file (if any).
+      let absoluteModuleName: string|null = context.absoluteModuleName;
+      if (lhs instanceof NodeReference || lhs instanceof AbsoluteReference) {
+        absoluteModuleName = lhs.moduleName || absoluteModuleName;
+      }
+
+      return this.visitExpression(expr, {...context, absoluteModuleName});
+    }
+
+    const body = fn.body;
     if (body.statements.length !== 1 || !ts.isReturnStatement(body.statements[0])) {
       throw new Error('Function body must have a single return statement only.');
     }
diff --git a/packages/compiler-cli/test/ngtsc/fake_core/index.ts b/packages/compiler-cli/test/ngtsc/fake_core/index.ts
index beceb52459..7999ab1cc4 100644
--- a/packages/compiler-cli/test/ngtsc/fake_core/index.ts
+++ b/packages/compiler-cli/test/ngtsc/fake_core/index.ts
@@ -26,3 +26,4 @@ export const Inject = callableParamDecorator();
 export const Self = callableParamDecorator();
 export const SkipSelf = callableParamDecorator();
 export const Optional = callableParamDecorator();
+export type ModuleWithProviders<T> = any;
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
index e690129325..3046f5dbc6 100644
--- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -344,4 +344,30 @@ describe('ngtsc behavioral tests', () => {
     const dtsContents = getContents('test.d.ts');
     expect(dtsContents).toContain('i0.ɵNgModuleDef<TestModule, [TestPipe,TestCmp], [], []>');
   });
+
+  it('should unwrap a ModuleWithProviders functoin if a generic type is provided for it', () => {
+    writeConfig();
+    write(`test.ts`, `
+        import {NgModule} from '@angular/core';
+        import {RouterModule} from 'router';
+
+        @NgModule({imports: [RouterModule.forRoot()]})
+        export class TestModule {}
+    `);
+
+    write('node_modules/router/index.d.ts', `
+        import {ModuleWithProviders} from '@angular/core';
+
+        declare class RouterModule {
+          static forRoot(): ModuleWithProviders<RouterModule>;
+        }
+    `);
+
+    const exitCode = main(['-p', basePath], errorSpy);
+    expect(errorSpy).not.toHaveBeenCalled();
+    expect(exitCode).toBe(0);
+    const dtsContents = getContents('test.d.ts');
+    expect(dtsContents).toContain(`import * as i1 from 'router';`);
+    expect(dtsContents).toContain('i0.ɵNgModuleDef<TestModule, [], [i1.RouterModule], []>');
+  });
 });
diff --git a/packages/core/src/metadata/ng_module.ts b/packages/core/src/metadata/ng_module.ts
index ed4634a25c..78de8a4347 100644
--- a/packages/core/src/metadata/ng_module.ts
+++ b/packages/core/src/metadata/ng_module.ts
@@ -73,10 +73,11 @@ export interface NgModuleDef<T, Declarations, Imports, Exports> {
 /**
  * A wrapper around an NgModule that associates it with the providers.
  *
- *
+ * @param T the module type. In Ivy applications, this must be explicitly
+ * provided.
  */
-export interface ModuleWithProviders {
-  ngModule: Type<any>;
+export interface ModuleWithProviders<T = any> {
+  ngModule: Type<T>;
   providers?: Provider[];
 }
 
diff --git a/packages/forms/src/form_providers.ts b/packages/forms/src/form_providers.ts
index 586a13b506..a0759331f8 100644
--- a/packages/forms/src/form_providers.ts
+++ b/packages/forms/src/form_providers.ts
@@ -38,7 +38,7 @@ export class FormsModule {
 export class ReactiveFormsModule {
   static withConfig(opts: {
     /** @deprecated as of v6 */ warnOnNgModelWithFormControl: 'never' | 'once' | 'always'
-  }): ModuleWithProviders {
+  }): ModuleWithProviders<ReactiveFormsModule> {
     return {
       ngModule: ReactiveFormsModule,
       providers: [{
diff --git a/packages/platform-browser/src/browser.ts b/packages/platform-browser/src/browser.ts
index 8123f422ee..be4821570b 100644
--- a/packages/platform-browser/src/browser.ts
+++ b/packages/platform-browser/src/browser.ts
@@ -112,7 +112,7 @@ export class BrowserModule {
    *
    * @experimental
    */
-  static withServerTransition(params: {appId: string}): ModuleWithProviders {
+  static withServerTransition(params: {appId: string}): ModuleWithProviders<BrowserModule> {
     return {
       ngModule: BrowserModule,
       providers: [
diff --git a/packages/router/src/router_module.ts b/packages/router/src/router_module.ts
index 73995ef7d3..7566ee1400 100644
--- a/packages/router/src/router_module.ts
+++ b/packages/router/src/router_module.ts
@@ -155,7 +155,7 @@ export class RouterModule {
    * * `paramsInheritanceStrategy` defines how the router merges params, data and resolved data
    * from parent to child routes.
    */
-  static forRoot(routes: Routes, config?: ExtraOptions): ModuleWithProviders {
+  static forRoot(routes: Routes, config?: ExtraOptions): ModuleWithProviders<RouterModule> {
     return {
       ngModule: RouterModule,
       providers: [
@@ -193,7 +193,7 @@ export class RouterModule {
   /**
    * Creates a module with all the router directives and a provider registering routes.
    */
-  static forChild(routes: Routes): ModuleWithProviders {
+  static forChild(routes: Routes): ModuleWithProviders<RouterModule> {
     return {ngModule: RouterModule, providers: [provideRoutes(routes)]};
   }
 }
diff --git a/packages/router/testing/src/router_testing_module.ts b/packages/router/testing/src/router_testing_module.ts
index ef4c5ab387..4f2935a483 100644
--- a/packages/router/testing/src/router_testing_module.ts
+++ b/packages/router/testing/src/router_testing_module.ts
@@ -180,7 +180,8 @@ export function setupTestingRouter(
   ]
 })
 export class RouterTestingModule {
-  static withRoutes(routes: Routes, config?: ExtraOptions): ModuleWithProviders {
+  static withRoutes(routes: Routes, config?: ExtraOptions):
+      ModuleWithProviders<RouterTestingModule> {
     return {
       ngModule: RouterTestingModule,
       providers: [
diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts
index 09052c005c..dc2ebf8627 100644
--- a/tools/public_api_guard/core/core.d.ts
+++ b/tools/public_api_guard/core/core.d.ts
@@ -510,8 +510,8 @@ export declare class ModuleWithComponentFactories<T> {
     constructor(ngModuleFactory: NgModuleFactory<T>, componentFactories: ComponentFactory<any>[]);
 }
 
-export interface ModuleWithProviders {
-    ngModule: Type<any>;
+export interface ModuleWithProviders<T = any> {
+    ngModule: Type<T>;
     providers?: Provider[];
 }
 
diff --git a/tools/public_api_guard/forms/forms.d.ts b/tools/public_api_guard/forms/forms.d.ts
index 4af0d6af73..9defb7acc0 100644
--- a/tools/public_api_guard/forms/forms.d.ts
+++ b/tools/public_api_guard/forms/forms.d.ts
@@ -458,7 +458,7 @@ export declare class RadioControlValueAccessor implements ControlValueAccessor,
 
 export declare class ReactiveFormsModule {
     static withConfig(opts: { warnOnNgModelWithFormControl: 'never' | 'once' | 'always';
-    }): ModuleWithProviders;
+    }): ModuleWithProviders<ReactiveFormsModule>;
 }
 
 export declare class RequiredValidator implements Validator {
diff --git a/tools/public_api_guard/platform-browser/platform-browser.d.ts b/tools/public_api_guard/platform-browser/platform-browser.d.ts
index 52231287f2..df3c1c135a 100644
--- a/tools/public_api_guard/platform-browser/platform-browser.d.ts
+++ b/tools/public_api_guard/platform-browser/platform-browser.d.ts
@@ -2,7 +2,7 @@ export declare class BrowserModule {
     constructor(parentModule: BrowserModule | null);
     /** @experimental */ static withServerTransition(params: {
         appId: string;
-    }): ModuleWithProviders;
+    }): ModuleWithProviders<BrowserModule>;
 }
 
 /** @experimental */
diff --git a/tools/public_api_guard/router/router.d.ts b/tools/public_api_guard/router/router.d.ts
index 8965f82018..6575a15279 100644
--- a/tools/public_api_guard/router/router.d.ts
+++ b/tools/public_api_guard/router/router.d.ts
@@ -414,8 +414,8 @@ export declare class RouterLinkWithHref implements OnChanges, OnDestroy {
 
 export declare class RouterModule {
     constructor(guard: any, router: Router);
-    static forChild(routes: Routes): ModuleWithProviders;
-    static forRoot(routes: Routes, config?: ExtraOptions): ModuleWithProviders;
+    static forChild(routes: Routes): ModuleWithProviders<RouterModule>;
+    static forRoot(routes: Routes, config?: ExtraOptions): ModuleWithProviders<RouterModule>;
 }
 
 export declare class RouterOutlet implements OnDestroy, OnInit {
diff --git a/tools/public_api_guard/router/testing.d.ts b/tools/public_api_guard/router/testing.d.ts
index d401d5a72d..a6396352c0 100644
--- a/tools/public_api_guard/router/testing.d.ts
+++ b/tools/public_api_guard/router/testing.d.ts
@@ -1,5 +1,5 @@
 export declare class RouterTestingModule {
-    static withRoutes(routes: Routes, config?: ExtraOptions): ModuleWithProviders;
+    static withRoutes(routes: Routes, config?: ExtraOptions): ModuleWithProviders<RouterTestingModule>;
 }
 
 export declare function setupTestingRouter(urlSerializer: UrlSerializer, contexts: ChildrenOutletContexts, location: Location, loader: NgModuleFactoryLoader, compiler: Compiler, injector: Injector, routes: Route[][], opts?: ExtraOptions, urlHandlingStrategy?: UrlHandlingStrategy): Router;