fix(compiler): support interface types in injectable constuctors (angular#14894)

chuckjaz · Zhicheng Wang · commit b7fe0e912deb · 2017-08-11T08:08:43.000+08:00
Fixes angular#12631
diff --git a/packages/compiler-cli/integrationtest/src/basic.ts b/packages/compiler-cli/integrationtest/src/basic.ts
@@ -8,6 +8,7 @@
 
 import {Component, Inject, LOCALE_ID, TRANSLATIONS_FORMAT} from '@angular/core';
 
+import {CUSTOM, Named} from './custom_token';
 
 @Component({
   selector: 'basic',
@@ -21,7 +22,8 @@ export class BasicComp {
   ctxArr: any[] = [];
   constructor(
       @Inject(LOCALE_ID) public localeId: string,
-      @Inject(TRANSLATIONS_FORMAT) public translationsFormat: string) {
+      @Inject(TRANSLATIONS_FORMAT) public translationsFormat: string,
+      @Inject(CUSTOM) public custom: Named) {
     this.ctxProp = 'initialValue';
   }
 }
diff --git a/packages/compiler-cli/integrationtest/src/custom_token.ts b/packages/compiler-cli/integrationtest/src/custom_token.ts
@@ -0,0 +1,13 @@
+/**
+ * @license
+ * Copyright Google Inc. 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 {InjectionToken} from '@angular/core';
+
+export interface Named { name: string; }
+
+export const CUSTOM = new InjectionToken<Named>('CUSTOM');
diff --git a/packages/compiler-cli/integrationtest/src/module.ts b/packages/compiler-cli/integrationtest/src/module.ts
@@ -20,6 +20,7 @@ import {MultipleComponentsMyComp, NextComp} from './a/multiple_components';
 import {AnimateCmp} from './animate';
 import {BasicComp} from './basic';
 import {ComponentUsingThirdParty} from './comp_using_3rdp';
+import {CUSTOM, Named} from './custom_token';
 import {CompWithAnalyzeEntryComponentsProvider, CompWithEntryComponents} from './entry_components';
 import {CompConsumingEvents, CompUsingPipes, CompWithProviders, CompWithReferences, DirPublishingEvents, ModuleUsingCustomElements} from './features';
 import {CompUsingRootModuleDirectiveAndPipe, SomeDirectiveInRootModule, SomeLibModule, SomePipeInRootModule, SomeService} from './module_fixtures';
@@ -75,6 +76,7 @@ export const SERVER_ANIMATIONS_PROVIDERS: Provider[] = [{
   providers: [
     SomeService,
     SERVER_ANIMATIONS_PROVIDERS,
+    {provide: CUSTOM, useValue: {name: 'some name'}},
   ],
   entryComponents: [
     AnimateCmp,
diff --git a/packages/compiler/test/aot/static_reflector_spec.ts b/packages/compiler/test/aot/static_reflector_spec.ts
@@ -726,6 +726,31 @@ describe('StaticReflector', () => {
       expect(reflector.annotations(reflector.getStaticSymbol('/tmp/src/main.ts', 'Child')))
           .toEqual([]);
     });
+
+    it('should support constructor parameters with @Inject and an interface type', () => {
+      const data = Object.create(DEFAULT_TEST_DATA);
+      const file = '/tmp/src/inject_interface.ts';
+      data[file] = `
+        import {Injectable, Inject} from '@angular/core';
+        import {F} from './f';
+
+        export interface InjectedInterface {
+
+        }
+
+        export class Token {}
+
+        @Injectable()
+        export class SomeClass {
+          constructor (@Inject(Token) injected: InjectedInterface, t: Token, @Inject(Token) f: F) {}
+        }
+      `;
+
+      init(data);
+
+      expect(reflector.parameters(reflector.getStaticSymbol(file, 'SomeClass'))[0].length)
+          .toEqual(1);
+    });
   });
 
 });
diff --git a/packages/compiler/test/aot/static_symbol_resolver_spec.ts b/packages/compiler/test/aot/static_symbol_resolver_spec.ts
@@ -424,7 +424,9 @@ export class MockStaticSymbolResolverHost implements StaticSymbolResolverHost {
             filePath, this.data[filePath], ts.ScriptTarget.ES5, /* setParentNodes */ true);
         const diagnostics: ts.Diagnostic[] = (<any>sf).parseDiagnostics;
         if (diagnostics && diagnostics.length) {
-          throw Error(`Error encountered during parse of file ${filePath}`);
+          const errors = diagnostics.map(d => `(${d.start}-${d.start+d.length}): ${d.messageText}`)
+                             .join('\n   ');
+          throw Error(`Error encountered during parse of file ${filePath}\n${errors}`);
         }
         return [this.collector.getMetadata(sf)];
       }
diff --git a/tools/@angular/tsc-wrapped/src/bundler.ts b/tools/@angular/tsc-wrapped/src/bundler.ts
@@ -9,8 +9,8 @@ import * as path from 'path';
 import * as ts from 'typescript';
 
 import {MetadataCollector} from './collector';
+import {ClassMetadata, ConstructorMetadata, FunctionMetadata, MemberMetadata, MetadataArray, MetadataEntry, MetadataError, MetadataImportedSymbolReferenceExpression, MetadataMap, MetadataObject, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isInterfaceMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMethodMetadata} from './schema';
 
-import {ClassMetadata, ConstructorMetadata, FunctionMetadata, MemberMetadata, MetadataArray, MetadataEntry, MetadataError, MetadataImportedSymbolReferenceExpression, MetadataMap, MetadataObject, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMethodMetadata} from './schema';
 
 // The character set used to produce private names.
 const PRIVATE_NAME_CHARS = [
@@ -268,6 +268,9 @@ export class MetadataBundler {
     if (isFunctionMetadata(value)) {
       return this.convertFunction(moduleName, value);
     }
+    if (isInterfaceMetadata(value)) {
+      return value;
+    }
     return this.convertValue(moduleName, value);
   }
 
diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts
@@ -9,7 +9,7 @@
 import * as ts from 'typescript';
 
 import {Evaluator, errorSymbol} from './evaluator';
-import {ClassMetadata, ConstructorMetadata, FunctionMetadata, MemberMetadata, MetadataEntry, MetadataError, MetadataMap, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression, isMethodMetadata} from './schema';
+import {ClassMetadata, ConstructorMetadata, FunctionMetadata, InterfaceMetadata, MemberMetadata, MetadataEntry, MetadataError, MetadataMap, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression, isMethodMetadata} from './schema';
 import {Symbols} from './symbols';
 
 // In TypeScript 2.1 these flags moved
@@ -56,7 +56,8 @@ export class MetadataCollector {
    */
   public getMetadata(sourceFile: ts.SourceFile, strict: boolean = false): ModuleMetadata {
     const locals = new Symbols(sourceFile);
-    const nodeMap = new Map<MetadataValue|ClassMetadata|FunctionMetadata, ts.Node>();
+    const nodeMap =
+        new Map<MetadataValue|ClassMetadata|InterfaceMetadata|FunctionMetadata, ts.Node>();
     const evaluator = new Evaluator(locals, nodeMap, this.options);
     let metadata: {[name: string]: MetadataValue | ClassMetadata | FunctionMetadata}|undefined;
     let exports: ModuleExportMetadata[];
@@ -264,13 +265,14 @@ export class MetadataCollector {
     });
 
     const isExportedIdentifier = (identifier: ts.Identifier) => exportMap.has(identifier.text);
-    const isExported = (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.EnumDeclaration) =>
-        isExport(node) || isExportedIdentifier(node.name);
+    const isExported =
+        (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.InterfaceDeclaration |
+         ts.EnumDeclaration) => isExport(node) || isExportedIdentifier(node.name);
     const exportedIdentifierName = (identifier: ts.Identifier) =>
         exportMap.get(identifier.text) || identifier.text;
     const exportedName =
-        (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.EnumDeclaration) =>
-            exportedIdentifierName(node.name);
+        (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.InterfaceDeclaration |
+         ts.EnumDeclaration) => exportedIdentifierName(node.name);
 
 
     // Predeclare classes and functions
@@ -290,6 +292,15 @@ export class MetadataCollector {
           }
           break;
 
+        case ts.SyntaxKind.InterfaceDeclaration:
+          const interfaceDeclaration = <ts.InterfaceDeclaration>node;
+          if (interfaceDeclaration.name) {
+            const interfaceName = interfaceDeclaration.name.text;
+            // All references to interfaces should be converted to references to `any`.
+            locals.define(interfaceName, {__symbolic: 'reference', name: 'any'});
+          }
+          break;
+
         case ts.SyntaxKind.FunctionDeclaration:
           const functionDeclaration = <ts.FunctionDeclaration>node;
           if (!isExported(functionDeclaration)) {
@@ -356,6 +367,14 @@ export class MetadataCollector {
           // Otherwise don't record metadata for the class.
           break;
 
+        case ts.SyntaxKind.InterfaceDeclaration:
+          const interfaceDeclaration = <ts.InterfaceDeclaration>node;
+          if (interfaceDeclaration.name && isExported(interfaceDeclaration)) {
+            if (!metadata) metadata = {};
+            metadata[exportedName(interfaceDeclaration)] = {__symbolic: 'interface'};
+          }
+          break;
+
         case ts.SyntaxKind.FunctionDeclaration:
           // Record functions that return a single value. Record the parameter
           // names substitution will be performed by the StaticReflector.
diff --git a/tools/@angular/tsc-wrapped/src/schema.ts b/tools/@angular/tsc-wrapped/src/schema.ts
@@ -17,7 +17,7 @@
 
 export const VERSION = 3;
 
-export type MetadataEntry = ClassMetadata | FunctionMetadata | MetadataValue;
+export type MetadataEntry = ClassMetadata | InterfaceMetadata | FunctionMetadata | MetadataValue;
 
 export interface ModuleMetadata {
   __symbolic: 'module';
@@ -47,6 +47,11 @@ export function isClassMetadata(value: any): value is ClassMetadata {
   return value && value.__symbolic === 'class';
 }
 
+export interface InterfaceMetadata { __symbolic: 'interface'; }
+export function isInterfaceMetadata(value: any): value is InterfaceMetadata {
+  return value && value.__symbolic === 'interface';
+}
+
 export interface MetadataMap { [name: string]: MemberMetadata[]; }
 
 export interface MemberMetadata {
diff --git a/tools/@angular/tsc-wrapped/test/collector.spec.ts b/tools/@angular/tsc-wrapped/test/collector.spec.ts
@@ -50,7 +50,8 @@ describe('Collector', () => {
       'static-method-with-default.ts',
       'class-inheritance.ts',
       'class-inheritance-parent.ts',
-      'class-inheritance-declarations.d.ts'
+      'class-inheritance-declarations.d.ts',
+      'interface-reference.ts'
     ]);
     service = ts.createLanguageService(host, documentRegistry);
     program = service.getProgram();
@@ -60,11 +61,18 @@ describe('Collector', () => {
   it('should not have errors in test data', () => { expectValidSources(service, program); });
 
   it('should return undefined for modules that have no metadata', () => {
-    const sourceFile = program.getSourceFile('app/hero.ts');
+    const sourceFile = program.getSourceFile('app/empty.ts');
     const metadata = collector.getMetadata(sourceFile);
     expect(metadata).toBeUndefined();
   });
 
+  it('should return an interface reference for interfaces', () => {
+    const sourceFile = program.getSourceFile('app/hero.ts');
+    const metadata = collector.getMetadata(sourceFile);
+    expect(metadata).toEqual(
+        {__symbolic: 'module', version: 3, metadata: {Hero: {__symbolic: 'interface'}}});
+  });
+
   it('should be able to collect a simple component\'s metadata', () => {
     const sourceFile = program.getSourceFile('app/hero-detail.component.ts');
     const metadata = collector.getMetadata(sourceFile);
@@ -609,6 +617,22 @@ describe('Collector', () => {
     });
   });
 
+  it('should collect any for interface parameter reference', () => {
+    const source = program.getSourceFile('/interface-reference.ts');
+    const metadata = collector.getMetadata(source);
+    expect((metadata.metadata['SomeClass'] as ClassMetadata).members).toEqual({
+      __ctor__: [{
+        __symbolic: 'constructor',
+        parameterDecorators: [[{
+          __symbolic: 'call',
+          expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Inject'},
+          arguments: ['a']
+        }]],
+        parameters: [{__symbolic: 'reference', name: 'any'}]
+      }]
+    });
+  });
+
   describe('in strict mode', () => {
     it('should throw if an error symbol is collecting a reference to a non-exported symbol', () => {
       const source = program.getSourceFile('/local-symbol-ref.ts');
@@ -759,6 +783,7 @@ const FILES: Directory = {
         id: number;
         name: string;
       }`,
+    'empty.ts': ``,
     'hero-detail.component.ts': `
       import {Component, Input} from 'angular2/core';
       import {Hero} from './hero';
@@ -927,6 +952,15 @@ const FILES: Directory = {
       }
     }
   `,
+  'interface-reference.ts': `
+    import {Injectable, Inject} from 'angular2/core';
+    export interface Test {}
+
+    @Injectable()
+    export class SomeClass {
+      constructor(@Inject("a") test: Test) {}
+    }
+  `,
   'import-star.ts': `
     import {Injectable} from 'angular2/core';
     import * as common from 'angular2/common';
@@ -1146,6 +1180,11 @@ const FILES: Directory = {
               (): any;
           }
           export declare var Injectable: InjectableFactory;
+          export interface InjectFactory {
+            (binding?: any): any;
+            new (binding?: any): any;
+          }
+          export declare var Inject: InjectFactory;
           export interface OnInit {
               ngOnInit(): any;
           }

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@`
`8`	`8`
`9`	`9`	`import {Component, Inject, LOCALE_ID, TRANSLATIONS_FORMAT} from '@angular/core';`
`10`	`10`
	`11`	`+import {CUSTOM, Named} from './custom_token';`
`11`	`12`
`12`	`13`	`@Component({`
`13`	`14`	`selector: 'basic',`
`@@ -21,7 +22,8 @@ export class BasicComp {`
`21`	`22`	`ctxArr: any[] = [];`
`22`	`23`	`constructor(`
`23`	`24`	`@Inject(LOCALE_ID) public localeId: string,`
`24`		`- @Inject(TRANSLATIONS_FORMAT) public translationsFormat: string) {`
	`25`	`+ @Inject(TRANSLATIONS_FORMAT) public translationsFormat: string,`
	`26`	`+ @Inject(CUSTOM) public custom: Named) {`
`25`	`27`	`this.ctxProp = 'initialValue';`
`26`	`28`	`}`
`27`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -424,7 +424,9 @@ export class MockStaticSymbolResolverHost implements StaticSymbolResolverHost {`
`424`	`424`	`filePath, this.data[filePath], ts.ScriptTarget.ES5, /* setParentNodes */ true);`
`425`	`425`	`const diagnostics: ts.Diagnostic[] = (<any>sf).parseDiagnostics;`
`426`	`426`	`if (diagnostics && diagnostics.length) {`
`427`		- throw Error(`Error encountered during parse of file ${filePath}`);
	`427`	+ const errors = diagnostics.map(d => `(${d.start}-${d.start+d.length}): ${d.messageText}`)
	`428`	`+ .join('\n ');`
	`429`	+ throw Error(`Error encountered during parse of file ${filePath}\n${errors}`);
`428`	`430`	`}`
`429`	`431`	`return [this.collector.getMetadata(sf)];`
`430`	`432`	`}`