From 4ef86891a3390751313430a5d42082e98dfdb7b5 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Mon, 11 Jul 2016 17:26:35 -0700 Subject: [PATCH] fix(compiler): Ignore references to declared modules and unneeded types (#9776) Fixes: #9670 --- .../compiler-cli/src/reflector_host.ts | 11 ++- .../compiler-cli/src/static_reflector.ts | 2 +- .../compiler-cli/test/reflector_host_spec.ts | 9 +++ .../test/static_reflector_spec.ts | 2 +- tools/@angular/tsc-wrapped/src/collector.ts | 14 ++-- tools/@angular/tsc-wrapped/src/evaluator.ts | 7 +- .../tsc-wrapped/test/evaluator.spec.ts | 71 ++++++++++--------- 7 files changed, 70 insertions(+), 46 deletions(-) diff --git a/modules/@angular/compiler-cli/src/reflector_host.ts b/modules/@angular/compiler-cli/src/reflector_host.ts index 29daaf9cdd..90d47c4cf9 100644 --- a/modules/@angular/compiler-cli/src/reflector_host.ts +++ b/modules/@angular/compiler-cli/src/reflector_host.ts @@ -123,7 +123,10 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator { const filePath = this.resolve(module, containingFile); if (!filePath) { - throw new Error(`Could not resolve module ${module} relative to ${containingFile}`); + // If the file cannot be found the module is probably referencing a declared module + // for which there is no disambiguating file and we also don't need to track + // re-exports. Just use the module name. + return this.getStaticSymbol(module, symbolName); } const tc = this.program.getTypeChecker(); @@ -174,10 +177,12 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator { return result; } - // TODO(alexeagle): take a statictype getMetadataFor(filePath: string): ModuleMetadata { if (!this.context.exists(filePath)) { - throw new Error(`No such file '${filePath}'`); + // If the file doesn't exists then we cannot return metadata for the file. + // This will occur if the user refernced a declared module for which no file + // exists for the module (i.e. jQuery or angularjs). + return; } if (DTS.test(filePath)) { const metadataPath = filePath.replace(DTS, '.metadata.json'); diff --git a/modules/@angular/compiler-cli/src/static_reflector.ts b/modules/@angular/compiler-cli/src/static_reflector.ts index d93f4f0000..78f400f61a 100644 --- a/modules/@angular/compiler-cli/src/static_reflector.ts +++ b/modules/@angular/compiler-cli/src/static_reflector.ts @@ -474,7 +474,7 @@ export class StaticReflector implements ReflectorReader { let message = produceErrorMessage(expression); if (expression['line']) { message = - `${message} (position ${expression['line']}:${expression['character']} in the original .ts file)`; + `${message} (position ${expression['line']+1}:${expression['character']+1} in the original .ts file)`; } throw new Error(message); } diff --git a/modules/@angular/compiler-cli/test/reflector_host_spec.ts b/modules/@angular/compiler-cli/test/reflector_host_spec.ts index b7a778423b..9b19b7b480 100644 --- a/modules/@angular/compiler-cli/test/reflector_host_spec.ts +++ b/modules/@angular/compiler-cli/test/reflector_host_spec.ts @@ -82,12 +82,17 @@ describe('reflector_host', () => { .toBeDefined(); }); + it('should be produce the same symbol if asked twice', () => { let foo1 = reflectorHost.getStaticSymbol('main.ts', 'foo'); let foo2 = reflectorHost.getStaticSymbol('main.ts', 'foo'); expect(foo1).toBe(foo2); }); + it('should be able to produce a symbol for a module with no file', () => { + expect(reflectorHost.getStaticSymbol('angularjs', 'SomeAngularSymbol')).toBeDefined(); + }); + it('should be able to read a metadata file', () => { expect(reflectorHost.getMetadataFor('node_modules/@angular/core.d.ts')) @@ -96,6 +101,10 @@ describe('reflector_host', () => { it('should be able to read metadata from an otherwise unused .d.ts file ', () => { expect(reflectorHost.getMetadataFor('node_modules/@angular/unused.d.ts')).toBeUndefined(); }); + + it('should return undefined for missing modules', () => { + expect(reflectorHost.getMetadataFor('node_modules/@angular/missing.d.ts')).toBeUndefined(); + }); }); const dummyModule = 'export let foo: any[];' diff --git a/modules/@angular/compiler-cli/test/static_reflector_spec.ts b/modules/@angular/compiler-cli/test/static_reflector_spec.ts index 8d187d15a2..b719c4b7eb 100644 --- a/modules/@angular/compiler-cli/test/static_reflector_spec.ts +++ b/modules/@angular/compiler-cli/test/static_reflector_spec.ts @@ -112,7 +112,7 @@ describe('StaticReflector', () => { let SomeClass = host.findDeclaration('src/error-reporting', 'SomeClass'); expect(() => reflector.annotations(SomeClass)) .toThrow(new Error( - 'Error encountered resolving symbol values statically. A reasonable error message (position 12:33 in the original .ts file), resolving symbol ErrorSym in /tmp/src/error-references.d.ts, resolving symbol Link2 in /tmp/src/error-references.d.ts, resolving symbol Link1 in /tmp/src/error-references.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts')); + 'Error encountered resolving symbol values statically. A reasonable error message (position 13:34 in the original .ts file), resolving symbol ErrorSym in /tmp/src/error-references.d.ts, resolving symbol Link2 in /tmp/src/error-references.d.ts, resolving symbol Link1 in /tmp/src/error-references.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts')); }); it('should simplify primitive into itself', () => { diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index 17d37a97fc..c80bdf5a32 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/tools/@angular/tsc-wrapped/src/collector.ts @@ -1,10 +1,11 @@ import * as ts from 'typescript'; import {Evaluator, errorSymbol, isPrimitive} from './evaluator'; -import {ClassMetadata, ConstructorMetadata, MemberMetadata, MetadataError, MetadataMap, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isMetadataError, isMetadataSymbolicReferenceExpression} from './schema'; +import {ClassMetadata, ConstructorMetadata, MemberMetadata, MetadataError, MetadataMap, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isMetadataError, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression} from './schema'; import {Symbols} from './symbols'; + /** * Collect decorator metadata from a TypeScript module. */ @@ -38,9 +39,11 @@ export class MetadataCollector { return undefined; } - function referenceFrom(node: ts.Node): MetadataSymbolicReferenceExpression|MetadataError { + function referenceFrom(node: ts.Node): MetadataSymbolicReferenceExpression|MetadataError| + MetadataSymbolicSelectExpression { const result = evaluator.evaluateNode(node); - if (isMetadataError(result) || isMetadataSymbolicReferenceExpression(result)) { + if (isMetadataError(result) || isMetadataSymbolicReferenceExpression(result) || + isMetadataSymbolicSelectExpression(result)) { return result; } else { return errorSym('Symbol reference expected', node); @@ -70,8 +73,9 @@ export class MetadataCollector { const methodDecorators = getDecorators(method.decorators); const parameters = method.parameters; const parameterDecoratorData: (MetadataSymbolicExpression | MetadataError)[][] = []; - const parametersData: (MetadataSymbolicReferenceExpression | MetadataError | null)[] = - []; + const parametersData: + (MetadataSymbolicReferenceExpression | MetadataError | + MetadataSymbolicSelectExpression | null)[] = []; let hasDecoratorData: boolean = false; let hasParameterData: boolean = false; for (const parameter of parameters) { diff --git a/tools/@angular/tsc-wrapped/src/evaluator.ts b/tools/@angular/tsc-wrapped/src/evaluator.ts index 00f3922845..dc0d8662ed 100644 --- a/tools/@angular/tsc-wrapped/src/evaluator.ts +++ b/tools/@angular/tsc-wrapped/src/evaluator.ts @@ -354,8 +354,8 @@ export class Evaluator { case ts.SyntaxKind.TypeReference: const typeReferenceNode = node; const typeNameNode = typeReferenceNode.typeName; - const getReference: (typeNameNode: ts.Identifier | ts.QualifiedName) => - MetadataSymbolicReferenceExpression | MetadataError = node => { + const getReference: (typeNameNode: ts.Identifier | ts.QualifiedName) => MetadataValue = + node => { if (typeNameNode.kind === ts.SyntaxKind.QualifiedName) { const qualifiedName = node; const left = this.evaluateNode(qualifiedName.left); @@ -364,7 +364,8 @@ export class Evaluator { __symbolic: 'reference', module: left.module, name: qualifiedName.right.text } } - return errorSymbol('Qualified type names not supported', node); + // Record a type reference to a declared type as a select. + return {__symbolic: 'select', expression: left, member: qualifiedName.right.text}; } else { const identifier = typeNameNode; let symbol = this.symbols.resolve(identifier.text); diff --git a/tools/@angular/tsc-wrapped/test/evaluator.spec.ts b/tools/@angular/tsc-wrapped/test/evaluator.spec.ts index 88b48c6127..6798fe4dbc 100644 --- a/tools/@angular/tsc-wrapped/test/evaluator.spec.ts +++ b/tools/@angular/tsc-wrapped/test/evaluator.spec.ts @@ -18,7 +18,7 @@ describe('Evaluator', () => { beforeEach(() => { host = new Host(FILES, [ 'expressions.ts', 'consts.ts', 'const_expr.ts', 'forwardRef.ts', 'classes.ts', - 'newExpression.ts', 'errors.ts' + 'newExpression.ts', 'errors.ts', 'declared.ts' ]); service = ts.createLanguageService(host, documentRegistry); program = service.getProgram(); @@ -39,7 +39,7 @@ describe('Evaluator', () => { }); it('should be able to fold literal expressions', () => { - var consts = program.getSourceFile('consts.ts'); + const consts = program.getSourceFile('consts.ts'); expect(evaluator.isFoldable(findVar(consts, 'someName').initializer)).toBeTruthy(); expect(evaluator.isFoldable(findVar(consts, 'someBool').initializer)).toBeTruthy(); expect(evaluator.isFoldable(findVar(consts, 'one').initializer)).toBeTruthy(); @@ -47,7 +47,7 @@ describe('Evaluator', () => { }); it('should be able to fold expressions with foldable references', () => { - var expressions = program.getSourceFile('expressions.ts'); + const expressions = program.getSourceFile('expressions.ts'); symbols.define('someName', 'some-name'); symbols.define('someBool', true); symbols.define('one', 1); @@ -61,7 +61,7 @@ describe('Evaluator', () => { }); it('should be able to evaluate literal expressions', () => { - var consts = program.getSourceFile('consts.ts'); + const consts = program.getSourceFile('consts.ts'); expect(evaluator.evaluateNode(findVar(consts, 'someName').initializer)).toBe('some-name'); expect(evaluator.evaluateNode(findVar(consts, 'someBool').initializer)).toBe(true); expect(evaluator.evaluateNode(findVar(consts, 'one').initializer)).toBe(1); @@ -69,7 +69,7 @@ describe('Evaluator', () => { }); it('should be able to evaluate expressions', () => { - var expressions = program.getSourceFile('expressions.ts'); + const expressions = program.getSourceFile('expressions.ts'); symbols.define('someName', 'some-name'); symbols.define('someBool', true); symbols.define('one', 1); @@ -117,7 +117,7 @@ describe('Evaluator', () => { }); it('should report recursive references as symbolic', () => { - var expressions = program.getSourceFile('expressions.ts'); + const expressions = program.getSourceFile('expressions.ts'); expect(evaluator.evaluateNode(findVar(expressions, 'recursiveA').initializer)) .toEqual({__symbolic: 'reference', name: 'recursiveB'}); expect(evaluator.evaluateNode(findVar(expressions, 'recursiveB').initializer)) @@ -125,13 +125,13 @@ describe('Evaluator', () => { }); it('should correctly handle special cases for CONST_EXPR', () => { - var const_expr = program.getSourceFile('const_expr.ts'); + const const_expr = program.getSourceFile('const_expr.ts'); expect(evaluator.evaluateNode(findVar(const_expr, 'bTrue').initializer)).toEqual(true); expect(evaluator.evaluateNode(findVar(const_expr, 'bFalse').initializer)).toEqual(false); }); it('should resolve a forwardRef', () => { - var forwardRef = program.getSourceFile('forwardRef.ts'); + const forwardRef = program.getSourceFile('forwardRef.ts'); expect(evaluator.evaluateNode(findVar(forwardRef, 'bTrue').initializer)).toEqual(true); expect(evaluator.evaluateNode(findVar(forwardRef, 'bFalse').initializer)).toEqual(false); }); @@ -139,7 +139,7 @@ describe('Evaluator', () => { it('should return new expressions', () => { symbols.define('Value', {__symbolic: 'reference', module: './classes', name: 'Value'}); evaluator = new Evaluator(symbols); - var newExpression = program.getSourceFile('newExpression.ts'); + const newExpression = program.getSourceFile('newExpression.ts'); expect(evaluator.evaluateNode(findVar(newExpression, 'someValue').initializer)).toEqual({ __symbolic: 'new', expression: {__symbolic: 'reference', name: 'Value', module: './classes'}, @@ -152,54 +152,57 @@ describe('Evaluator', () => { }); }); - it('should return errors for unsupported expressions', () => { - let errors = program.getSourceFile('errors.ts'); - let aDecl = findVar(errors, 'a'); + it('should support referene to a declared module type', () => { + const declared = program.getSourceFile('declared.ts'); + const aDecl = findVar(declared, 'a'); expect(evaluator.evaluateNode(aDecl.type)).toEqual({ - __symbolic: 'error', - message: 'Qualified type names not supported', - line: 5, - character: 10 + __symbolic: 'select', + expression: {__symbolic: 'reference', name: 'Foo'}, + member: 'A' }); - let fDecl = findVar(errors, 'f'); + }); + + it('should return errors for unsupported expressions', () => { + const errors = program.getSourceFile('errors.ts'); + const fDecl = findVar(errors, 'f'); expect(evaluator.evaluateNode(fDecl.initializer)) .toEqual( - {__symbolic: 'error', message: 'Function call not supported', line: 6, character: 11}); - let eDecl = findVar(errors, 'e'); + {__symbolic: 'error', message: 'Function call not supported', line: 1, character: 11}); + const eDecl = findVar(errors, 'e'); expect(evaluator.evaluateNode(eDecl.type)).toEqual({ __symbolic: 'error', message: 'Could not resolve type', - line: 7, + line: 2, character: 10, context: {typeName: 'NotFound'} }); - let sDecl = findVar(errors, 's'); + const sDecl = findVar(errors, 's'); expect(evaluator.evaluateNode(sDecl.initializer)).toEqual({ __symbolic: 'error', message: 'Name expected', - line: 8, + line: 3, character: 13, context: {received: '1'} }); - let tDecl = findVar(errors, 't'); + const tDecl = findVar(errors, 't'); expect(evaluator.evaluateNode(tDecl.initializer)).toEqual({ __symbolic: 'error', message: 'Expression form not supported', - line: 9, + line: 4, character: 11 }); }); it('should be able to fold an array spread', () => { - let expressions = program.getSourceFile('expressions.ts'); + const expressions = program.getSourceFile('expressions.ts'); symbols.define('arr', [1, 2, 3, 4]); - let arrSpread = findVar(expressions, 'arrSpread'); + const arrSpread = findVar(expressions, 'arrSpread'); expect(evaluator.evaluateNode(arrSpread.initializer)).toEqual([0, 1, 2, 3, 4, 5]); }); it('should be able to produce a spread expression', () => { - let expressions = program.getSourceFile('expressions.ts'); - let arrSpreadRef = findVar(expressions, 'arrSpreadRef'); + const expressions = program.getSourceFile('expressions.ts'); + const arrSpreadRef = findVar(expressions, 'arrSpreadRef'); expect(evaluator.evaluateNode(arrSpreadRef.initializer)).toEqual([ 0, {__symbolic: 'spread', expression: {__symbolic: 'reference', name: 'arrImport'}}, 5 ]); @@ -296,14 +299,16 @@ const FILES: Directory = { export const complex = CONST_EXPR(new Value("name", forwardRef(() => 12))); `, 'errors.ts': ` - declare namespace Foo { - type A = string; - } - - let a: Foo.A = 'some value'; let f = () => 1; let e: NotFound; let s = { 1: 1, 2: 2 }; let t = typeof 12; `, + 'declared.ts': ` + declare namespace Foo { + type A = string; + } + + let a: Foo.A = 'some value'; + ` };