Skip to content

Commit

Permalink
fix(language-service): tolerate errors in decorators
Browse files Browse the repository at this point in the history
Fixes #14631
  • Loading branch information
chuckjaz committed Feb 22, 2017
1 parent bb0460b commit 290f12b
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 38 deletions.
4 changes: 2 additions & 2 deletions modules/@angular/compiler-cli/src/compiler_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -35,7 +35,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, '/');
Expand Down
34 changes: 23 additions & 11 deletions modules/@angular/compiler/src/aot/static_reflector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,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.
Expand Down Expand Up @@ -335,7 +343,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)));
}
Expand All @@ -362,7 +371,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});
Expand Down Expand Up @@ -529,7 +538,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.
Expand All @@ -543,16 +553,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 {
Expand Down Expand Up @@ -678,10 +694,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;
Expand Down
3 changes: 1 addition & 2 deletions modules/@angular/compiler/test/aot/README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Tests in this directory are excluded from running in the browser and only running
in node.
Tests in this directory are excluded from running in the browser and only run in node.
38 changes: 33 additions & 5 deletions modules/@angular/compiler/test/aot/static_reflector_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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('', '');
}

Expand Down Expand Up @@ -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) {}
Expand Down Expand Up @@ -1264,5 +1292,5 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
export class Dep {
@Input f: Forward;
}
`,
`
};
Original file line number Diff line number Diff line change
Expand Up @@ -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$/;

Expand Down Expand Up @@ -366,9 +365,11 @@ export class MockSummaryResolver implements SummaryResolver<StaticSymbol> {
}

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 {
Expand Down
3 changes: 2 additions & 1 deletion modules/@angular/language-service/src/reflector_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down
5 changes: 5 additions & 0 deletions tools/@angular/tsc-wrapped/src/collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export class CollectorOptions {
* the source.
*/
quotedNames?: boolean;

/**
* Do not simplify invalid expressions.
*/
verboseInvalidExpression?: boolean;
}

/**
Expand Down
30 changes: 17 additions & 13 deletions tools/@angular/tsc-wrapped/src/evaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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} = {};
Expand All @@ -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 {
Expand All @@ -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.
}
Expand Down Expand Up @@ -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 = <MetadataValue[]>this.evaluateNode(
(<ts.PropertyAccessExpression>callExpression.expression).expression);
if (isMetadataError(arrayValue)) return arrayValue;
if (isFoldableError(arrayValue)) return arrayValue;
return arrayValue.concat(args[0]);
}
}
Expand All @@ -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};
Expand All @@ -326,7 +330,7 @@ export class Evaluator {
case ts.SyntaxKind.NewExpression:
const newExpression = <ts.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);
Expand All @@ -341,11 +345,11 @@ export class Evaluator {
case ts.SyntaxKind.PropertyAccessExpression: {
const propertyAccessExpression = <ts.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))
Expand All @@ -361,11 +365,11 @@ export class Evaluator {
case ts.SyntaxKind.ElementAccessExpression: {
const elementAccessExpression = <ts.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) &&
Expand Down Expand Up @@ -404,15 +408,15 @@ export class Evaluator {
} else {
const identifier = <ts.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(
errorSymbol('Could not resolve type', node, {typeName: identifier.text}), node);
}
};
const typeReference = getReference(typeNameNode);
if (isMetadataError(typeReference)) {
if (isFoldableError(typeReference)) {
return recordEntry(typeReference, node);
}
if (!isMetadataModuleReferenceExpression(typeReference) &&
Expand Down

0 comments on commit 290f12b

Please sign in to comment.