From 14d1e659be7dbc12ded4648655aab078bc6dafad Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Wed, 1 Mar 2017 13:23:34 -0800 Subject: [PATCH] fix(language-service): tolerate errors in decorators (#14634) Fixes #14631 --- .../compiler-cli/src/compiler_host.ts | 4 +- .../compiler/src/aot/static_reflector.ts | 34 +++++++++++------ modules/@angular/compiler/test/aot/README.md | 3 +- .../test/aot/static_reflector_spec.ts | 38 ++++++++++++++++--- .../test/aot/static_symbol_resolver_spec.ts | 9 +++-- .../language-service/src/reflector_host.ts | 3 +- tools/@angular/tsc-wrapped/src/collector.ts | 5 +++ tools/@angular/tsc-wrapped/src/evaluator.ts | 30 ++++++++------- 8 files changed, 88 insertions(+), 38 deletions(-) diff --git a/modules/@angular/compiler-cli/src/compiler_host.ts b/modules/@angular/compiler-cli/src/compiler_host.ts index 7a4e6a3919660..b3def2a01af23 100644 --- a/modules/@angular/compiler-cli/src/compiler_host.ts +++ b/modules/@angular/compiler-cli/src/compiler_host.ts @@ -7,7 +7,7 @@ */ import {AotCompilerHost, StaticSymbol} from '@angular/compiler'; -import {AngularCompilerOptions, MetadataCollector, ModuleMetadata} from '@angular/tsc-wrapped'; +import {AngularCompilerOptions, CollectorOptions, MetadataCollector, ModuleMetadata} from '@angular/tsc-wrapped'; import * as fs from 'fs'; import * as path from 'path'; import * as ts from 'typescript'; @@ -37,7 +37,7 @@ export class CompilerHost implements AotCompilerHost { constructor( protected program: ts.Program, protected options: AngularCompilerOptions, - protected context: CompilerHostContext) { + protected context: CompilerHostContext, collectorOptions?: CollectorOptions) { // normalize the path so that it never ends with '/'. this.basePath = path.normalize(path.join(this.options.basePath, '.')).replace(/\\/g, '/'); this.genDir = path.normalize(path.join(this.options.genDir, '.')).replace(/\\/g, '/'); diff --git a/modules/@angular/compiler/src/aot/static_reflector.ts b/modules/@angular/compiler/src/aot/static_reflector.ts index 7c286c86a35e9..5fd8eb0dad94d 100644 --- a/modules/@angular/compiler/src/aot/static_reflector.ts +++ b/modules/@angular/compiler/src/aot/static_reflector.ts @@ -15,6 +15,14 @@ const ANGULAR_CORE = '@angular/core'; const HIDDEN_KEY = /^\$.*\$$/; +const IGNORE = { + __symbolic: 'ignore' +}; + +function shouldIgnore(value: any): boolean { + return value && value.__symbolic == 'ignore'; +} + /** * A static reflector implements enough of the Reflector API that is necessary to compile * templates statically. @@ -332,7 +340,8 @@ export class StaticReflector implements ɵReflectorReader { if (value && (depth != 0 || value.__symbolic != 'error')) { const parameters: string[] = targetFunction['parameters']; const defaults: any[] = targetFunction.defaults; - args = args.map(arg => simplifyInContext(context, arg, depth + 1)); + args = args.map(arg => simplifyInContext(context, arg, depth + 1)) + .map(arg => shouldIgnore(arg) ? undefined : arg); if (defaults && defaults.length > args.length) { args.push(...defaults.slice(args.length).map((value: any) => simplify(value))); } @@ -359,7 +368,7 @@ export class StaticReflector implements ɵReflectorReader { // If depth is 0 we are evaluating the top level expression that is describing element // decorator. In this case, it is a decorator we don't understand, such as a custom // non-angular decorator, and we should just ignore it. - return {__symbolic: 'ignore'}; + return IGNORE; } return simplify( {__symbolic: 'error', message: 'Function call not supported', context: functionSymbol}); @@ -526,7 +535,8 @@ export class StaticReflector implements ɵReflectorReader { let converter = self.conversionMap.get(staticSymbol); if (converter) { const args = - argExpressions.map(arg => simplifyInContext(context, arg, depth + 1)); + argExpressions.map(arg => simplifyInContext(context, arg, depth + 1)) + .map(arg => shouldIgnore(arg) ? undefined : arg); return converter(context, args); } else { // Determine if the function is one we can simplify. @@ -540,16 +550,22 @@ export class StaticReflector implements ɵReflectorReader { if (expression['line']) { message = `${message} (position ${expression['line']+1}:${expression['character']+1} in the original .ts file)`; - throw positionalError( - message, context.filePath, expression['line'], expression['character']); + self.reportError( + positionalError( + message, context.filePath, expression['line'], expression['character']), + context); + } else { + self.reportError(new Error(message), context); } - throw new Error(message); + return IGNORE; + case 'ignore': + return expression; } return null; } return mapStringMap(expression, (value, name) => simplify(value)); } - return null; + return IGNORE; } try { @@ -675,10 +691,6 @@ class PopulatedScope extends BindingScope { } } -function shouldIgnore(value: any): boolean { - return value && value.__symbolic == 'ignore'; -} - function positionalError(message: string, fileName: string, line: number, column: number): Error { const result = new Error(message); (result as any).fileName = fileName; diff --git a/modules/@angular/compiler/test/aot/README.md b/modules/@angular/compiler/test/aot/README.md index c3072c131ba17..7c522a1396039 100644 --- a/modules/@angular/compiler/test/aot/README.md +++ b/modules/@angular/compiler/test/aot/README.md @@ -1,2 +1 @@ -Tests in this directory are excluded from running in the browser and only running -in node. \ No newline at end of file +Tests in this directory are excluded from running in the browser and only run in node. \ No newline at end of file diff --git a/modules/@angular/compiler/test/aot/static_reflector_spec.ts b/modules/@angular/compiler/test/aot/static_reflector_spec.ts index 0c6832780d5d1..454451afa343b 100644 --- a/modules/@angular/compiler/test/aot/static_reflector_spec.ts +++ b/modules/@angular/compiler/test/aot/static_reflector_spec.ts @@ -8,6 +8,7 @@ import {StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, StaticSymbolResolverHost} from '@angular/compiler'; import {HostListener, Inject, animate, group, keyframes, sequence, state, style, transition, trigger} from '@angular/core'; +import {CollectorOptions} from '@angular/tsc-wrapped'; import {MockStaticSymbolResolverHost, MockSummaryResolver} from './static_symbol_resolver_spec'; @@ -19,11 +20,13 @@ describe('StaticReflector', () => { function init( testData: {[key: string]: any} = DEFAULT_TEST_DATA, - decorators: {name: string, filePath: string, ctor: any}[] = []) { + decorators: {name: string, filePath: string, ctor: any}[] = [], + errorRecorder?: (error: any, fileName: string) => void, collectorOptions?: CollectorOptions) { const symbolCache = new StaticSymbolCache(); - host = new MockStaticSymbolResolverHost(testData); - symbolResolver = new StaticSymbolResolver(host, symbolCache, new MockSummaryResolver([])); - reflector = new StaticReflector(symbolResolver, decorators); + host = new MockStaticSymbolResolverHost(testData, collectorOptions); + symbolResolver = + new StaticSymbolResolver(host, symbolCache, new MockSummaryResolver([]), errorRecorder); + reflector = new StaticReflector(symbolResolver, decorators, [], errorRecorder); noContext = reflector.getStaticSymbol('', ''); } @@ -492,6 +495,31 @@ describe('StaticReflector', () => { expect(() => reflector.propMetadata(appComponent)).not.toThrow(); }); + it('should produce a annotation even if it contains errors', () => { + const data = Object.create(DEFAULT_TEST_DATA); + const file = '/tmp/src/invalid-component.ts'; + data[file] = ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'tmp', + template: () => {}, + providers: [1, 2, (() => {}), 3, !(() => {}), 4, 5, (() => {}) + (() => {}), 6, 7] + }) + export class BadComponent { + + } + `; + init(data, [], () => {}, {verboseInvalidExpression: true}); + + const badComponent = reflector.getStaticSymbol(file, 'BadComponent'); + const annotations = reflector.annotations(badComponent); + const annotation = annotations[0]; + expect(annotation.selector).toEqual('tmp'); + expect(annotation.template).toBeUndefined(); + expect(annotation.providers).toEqual([1, 2, 3, 4, 5, 6, 7]); + }); + describe('inheritance', () => { class ClassDecorator { constructor(public value: any) {} @@ -1264,5 +1292,5 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { export class Dep { @Input f: Forward; } - `, + ` }; diff --git a/modules/@angular/compiler/test/aot/static_symbol_resolver_spec.ts b/modules/@angular/compiler/test/aot/static_symbol_resolver_spec.ts index 5c88f6d22ad95..8037be76f0b33 100644 --- a/modules/@angular/compiler/test/aot/static_symbol_resolver_spec.ts +++ b/modules/@angular/compiler/test/aot/static_symbol_resolver_spec.ts @@ -7,11 +7,10 @@ */ import {StaticSymbol, StaticSymbolCache, StaticSymbolResolver, StaticSymbolResolverHost, Summary, SummaryResolver} from '@angular/compiler'; -import {MetadataCollector} from '@angular/tsc-wrapped'; +import {CollectorOptions, MetadataCollector} from '@angular/tsc-wrapped'; import * as ts from 'typescript'; - // This matches .ts files but not .d.ts files. const TS_EXT = /(^.|(?!\.d)..)\.ts$/; @@ -366,9 +365,11 @@ export class MockSummaryResolver implements SummaryResolver { } export class MockStaticSymbolResolverHost implements StaticSymbolResolverHost { - private collector = new MetadataCollector(); + private collector: MetadataCollector; - constructor(private data: {[key: string]: any}) {} + constructor(private data: {[key: string]: any}, collectorOptions?: CollectorOptions) { + this.collector = new MetadataCollector(collectorOptions); + } // In tests, assume that symbols are not re-exported moduleNameToFileName(modulePath: string, containingFile?: string): string { diff --git a/modules/@angular/language-service/src/reflector_host.ts b/modules/@angular/language-service/src/reflector_host.ts index c84c14e9fc212..a8a8a787c6f94 100644 --- a/modules/@angular/language-service/src/reflector_host.ts +++ b/modules/@angular/language-service/src/reflector_host.ts @@ -33,7 +33,8 @@ export class ReflectorHost extends CompilerHost { options: AngularCompilerOptions) { super( null, options, - new ModuleResolutionHostAdapter(new ReflectorModuleModuleResolutionHost(serviceHost))); + new ModuleResolutionHostAdapter(new ReflectorModuleModuleResolutionHost(serviceHost)), + {verboseInvalidExpression: true}); } protected get program() { return this.getProgram(); } diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index cc370e431563e..dd4a019087e73 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/tools/@angular/tsc-wrapped/src/collector.ts @@ -37,6 +37,11 @@ export class CollectorOptions { * the source. */ quotedNames?: boolean; + + /** + * Do not simplify invalid expressions. + */ + verboseInvalidExpression?: boolean; } /** diff --git a/tools/@angular/tsc-wrapped/src/evaluator.ts b/tools/@angular/tsc-wrapped/src/evaluator.ts index 3b471315c731b..f3445e2df78ce 100644 --- a/tools/@angular/tsc-wrapped/src/evaluator.ts +++ b/tools/@angular/tsc-wrapped/src/evaluator.ts @@ -227,6 +227,10 @@ export class Evaluator { return entry; } + function isFoldableError(value: any): value is MetadataError { + return !t.options.verboseInvalidExpression && isMetadataError(value); + } + switch (node.kind) { case ts.SyntaxKind.ObjectLiteralExpression: let obj: {[name: string]: any} = {}; @@ -241,14 +245,14 @@ export class Evaluator { quoted.push(name); } const propertyName = this.nameOf(assignment.name); - if (isMetadataError(propertyName)) { + if (isFoldableError(propertyName)) { error = propertyName; return true; } const propertyValue = isPropertyAssignment(assignment) ? this.evaluateNode(assignment.initializer) : {__symbolic: 'reference', name: propertyName}; - if (isMetadataError(propertyValue)) { + if (isFoldableError(propertyValue)) { error = propertyValue; return true; // Stop the forEachChild. } else { @@ -267,7 +271,7 @@ export class Evaluator { const value = this.evaluateNode(child); // Check for error - if (isMetadataError(value)) { + if (isFoldableError(value)) { error = value; return true; // Stop the forEachChild. } @@ -299,14 +303,14 @@ export class Evaluator { } } const args = callExpression.arguments.map(arg => this.evaluateNode(arg)); - if (args.some(isMetadataError)) { + if (!this.options.verboseInvalidExpression && args.some(isMetadataError)) { return args.find(isMetadataError); } if (this.isFoldable(callExpression)) { if (isMethodCallOf(callExpression, 'concat')) { const arrayValue = this.evaluateNode( (callExpression.expression).expression); - if (isMetadataError(arrayValue)) return arrayValue; + if (isFoldableError(arrayValue)) return arrayValue; return arrayValue.concat(args[0]); } } @@ -315,7 +319,7 @@ export class Evaluator { return recordEntry(args[0], node); } const expression = this.evaluateNode(callExpression.expression); - if (isMetadataError(expression)) { + if (isFoldableError(expression)) { return recordEntry(expression, node); } let result: MetadataSymbolicCallExpression = {__symbolic: 'call', expression: expression}; @@ -326,7 +330,7 @@ export class Evaluator { case ts.SyntaxKind.NewExpression: const newExpression = node; const newArgs = newExpression.arguments.map(arg => this.evaluateNode(arg)); - if (newArgs.some(isMetadataError)) { + if (!this.options.verboseInvalidExpression && newArgs.some(isMetadataError)) { return recordEntry(newArgs.find(isMetadataError), node); } const newTarget = this.evaluateNode(newExpression.expression); @@ -341,11 +345,11 @@ export class Evaluator { case ts.SyntaxKind.PropertyAccessExpression: { const propertyAccessExpression = node; const expression = this.evaluateNode(propertyAccessExpression.expression); - if (isMetadataError(expression)) { + if (isFoldableError(expression)) { return recordEntry(expression, node); } const member = this.nameOf(propertyAccessExpression.name); - if (isMetadataError(member)) { + if (isFoldableError(member)) { return recordEntry(member, node); } if (expression && this.isFoldable(propertyAccessExpression.expression)) @@ -361,11 +365,11 @@ export class Evaluator { case ts.SyntaxKind.ElementAccessExpression: { const elementAccessExpression = node; const expression = this.evaluateNode(elementAccessExpression.expression); - if (isMetadataError(expression)) { + if (isFoldableError(expression)) { return recordEntry(expression, node); } const index = this.evaluateNode(elementAccessExpression.argumentExpression); - if (isMetadataError(expression)) { + if (isFoldableError(expression)) { return recordEntry(expression, node); } if (this.isFoldable(elementAccessExpression.expression) && @@ -404,7 +408,7 @@ export class Evaluator { } else { const identifier = typeNameNode; const symbol = this.symbols.resolve(identifier.text); - if (isMetadataError(symbol) || isMetadataSymbolicReferenceExpression(symbol)) { + if (isFoldableError(symbol) || isMetadataSymbolicReferenceExpression(symbol)) { return recordEntry(symbol, node); } return recordEntry( @@ -412,7 +416,7 @@ export class Evaluator { } }; const typeReference = getReference(typeNameNode); - if (isMetadataError(typeReference)) { + if (isFoldableError(typeReference)) { return recordEntry(typeReference, node); } if (!isMetadataModuleReferenceExpression(typeReference) &&