fix(language-service): tolerate errors in decorators (#14634)

chuckjaz · IgorMinar · commit 6bae7378b1ec · 2017-03-01T13:23:34.000-08:00
Fixes #14631
diff --git 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
@@ -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
@@ -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.
diff --git 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
@@ -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<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 {
diff --git 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
@@ -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
@@ -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 = <MetadataValue[]>this.evaluateNode(
                 (<ts.PropertyAccessExpression>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 = <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);
@@ -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))
@@ -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) &&
@@ -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) &&

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`		`-Tests in this directory are excluded from running in the browser and only running`
`2`		`-in node.`
	`1`	`+Tests in this directory are excluded from running in the browser and only run in node.`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,8 @@ export class ReflectorHost extends CompilerHost {`
`33`	`33`	`options: AngularCompilerOptions) {`
`34`	`34`	`super(`
`35`	`35`	`null, options,`
`36`		`- new ModuleResolutionHostAdapter(new ReflectorModuleModuleResolutionHost(serviceHost)));`
	`36`	`+ new ModuleResolutionHostAdapter(new ReflectorModuleModuleResolutionHost(serviceHost)),`
	`37`	`+ {verboseInvalidExpression: true});`
`37`	`38`	`}`
`38`	`39`
`39`	`40`	`protected get program() { return this.getProgram(); }`