From 0e08ad628a4dee99448ad14dca15eab79ede8e4d Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Tue, 15 Oct 2019 22:29:42 +0200
Subject: [PATCH] fix(ivy): throw better error for missing generic type in
 ModuleWithProviders (#33187)

Currently if a `ModuleWithProviders` is missng its generic type, we throw a cryptic error like:

```
error TS-991010: Value at position 3 in the NgModule.imports of TodosModule is not a reference: [object Object]
```

These changes add a better error to make it easier to debug.

PR Close #33187
---
 .../src/ngtsc/annotations/src/ng_module.ts    | 16 +++++++++---
 .../src/ngtsc/diagnostics/src/code.ts         |  8 +++++-
 .../test/ngtsc/fake_core/index.ts             |  3 ++-
 .../compiler-cli/test/ngtsc/ngtsc_spec.ts     | 25 +++++++++++++++++++
 4 files changed, 47 insertions(+), 5 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 ea2c2b142a..7a814e7bec 100644
--- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
+++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
@@ -358,7 +358,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
                                                   ts.FunctionExpression): ts.Expression|null {
     const type = node.type || null;
     return type &&
-        (this._reflectModuleFromTypeParam(type) || this._reflectModuleFromLiteralType(type));
+        (this._reflectModuleFromTypeParam(type, node) || this._reflectModuleFromLiteralType(type));
   }
 
   /**
@@ -367,7 +367,9 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
    * @param type The type to reflect on.
    * @returns the identifier of the NgModule type if found, or null otherwise.
    */
-  private _reflectModuleFromTypeParam(type: ts.TypeNode): ts.Expression|null {
+  private _reflectModuleFromTypeParam(
+      type: ts.TypeNode,
+      node: ts.FunctionDeclaration|ts.MethodDeclaration|ts.FunctionExpression): ts.Expression|null {
     // Examine the type of the function to see if it's a ModuleWithProviders reference.
     if (!ts.isTypeReferenceNode(type)) {
       return null;
@@ -395,7 +397,15 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
 
     // If there's no type parameter specified, bail.
     if (type.typeArguments === undefined || type.typeArguments.length !== 1) {
-      return null;
+      const parent =
+          ts.isMethodDeclaration(node) && ts.isClassDeclaration(node.parent) ? node.parent : null;
+      const symbolName = (parent && parent.name ? parent.name.getText() + '.' : '') +
+          (node.name ? node.name.getText() : 'anonymous');
+      throw new FatalDiagnosticError(
+          ErrorCode.NGMODULE_MODULE_WITH_PROVIDERS_MISSING_GENERIC, type,
+          `${symbolName} returns a ModuleWithProviders type without a generic type argument. ` +
+              `Please add a generic type argument to the ModuleWithProviders type. If this ` +
+              `occurrence is in library code you don't control, please contact the library authors.`);
     }
 
     const arg = type.typeArguments[0];
diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts
index 85be9dbf47..7534049f18 100644
--- a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts
+++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts
@@ -58,6 +58,12 @@ export enum ErrorCode {
    */
   NGMODULE_INVALID_REEXPORT = 6004,
 
+  /**
+   * Raised when a `ModuleWithProviders` with a missing
+   * generic type argument is passed into an `NgModule`.
+   */
+  NGMODULE_MODULE_WITH_PROVIDERS_MISSING_GENERIC = 6005,
+
   /**
    * Raised when ngcc tries to inject a synthetic decorator over one that already exists.
    */
@@ -87,4 +93,4 @@ export enum ErrorCode {
 
 export function ngErrorCode(code: ErrorCode): number {
   return parseInt('-99' + code);
-}
\ No newline at end of file
+}
diff --git a/packages/compiler-cli/test/ngtsc/fake_core/index.ts b/packages/compiler-cli/test/ngtsc/fake_core/index.ts
index 4e494f06d1..884553dcad 100644
--- a/packages/compiler-cli/test/ngtsc/fake_core/index.ts
+++ b/packages/compiler-cli/test/ngtsc/fake_core/index.ts
@@ -44,7 +44,8 @@ export const Output = callablePropDecorator();
 export const ViewChild = callablePropDecorator();
 export const ViewChildren = callablePropDecorator();
 
-export type ModuleWithProviders<T> = any;
+// T defaults to `any` to reflect what is currently in core.
+export type ModuleWithProviders<T = any> = any;
 
 export class ChangeDetectorRef {}
 export class ElementRef {}
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
index 321b44d78f..7e349632d7 100644
--- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -1469,6 +1469,31 @@ runInEachFileSystem(os => {
                 'i0.ɵɵNgModuleDefWithMeta<TestModule, never, [typeof i1.RouterModule], never>');
       });
 
+      it('should throw if ModuleWithProviders is missing its generic type argument', () => {
+        env.write(`test.ts`, `
+          import {NgModule} from '@angular/core';
+          import {RouterModule} from 'router';
+
+          @NgModule({imports: [RouterModule.forRoot()]})
+          export class TestModule {}
+        `);
+
+        env.write('node_modules/router/index.d.ts', `
+          import {ModuleWithProviders, ɵɵNgModuleDefWithMeta} from '@angular/core';
+
+          declare class RouterModule {
+            static forRoot(): ModuleWithProviders;
+            static ɵmod: ɵɵNgModuleDefWithMeta<RouterModule, never, never, never>;
+          }
+        `);
+        const errors = env.driveDiagnostics();
+        expect(trim(errors[0].messageText as string))
+            .toContain(
+                `RouterModule.forRoot returns a ModuleWithProviders type without a generic type argument. ` +
+                `Please add a generic type argument to the ModuleWithProviders type. If this ` +
+                `occurrence is in library code you don't control, please contact the library authors.`);
+      });
+
       it('should extract the generic type if it is provided as qualified type name', () => {
         env.write(`test.ts`, `
         import {NgModule} from '@angular/core';