fix(compiler-cli): do not fold errors past calls in the collector (an…

…gular#21708) Folding errors passed calls prevented the static reflector from begin able to ignore errors in annotations it doesn't know as the call to the unknown annotation was elided from the metadata. Fixes: angular#21273 PR Close angular#21708
leo6104 · Mar 25, 2018 · 12a119b · 12a119b
1 parent f4ed8f4
commit 12a119b
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 10 deletions.
diff --git a/packages/compiler-cli/src/metadata/collector.ts b/packages/compiler-cli/src/metadata/collector.ts
@@ -289,7 +289,7 @@ export class MetadataCollector {
          ts.TypeAliasDeclaration | ts.EnumDeclaration) => exportedIdentifierName(node.name);
 
 
-    // Predeclare classes and functions
+    // Pre-declare classes and functions
     ts.forEachChild(sourceFile, node => {
       switch (node.kind) {
         case ts.SyntaxKind.ClassDeclaration:
@@ -454,7 +454,7 @@ export class MetadataCollector {
                 };
               } else {
                 nextDefaultValue =
-                    recordEntry(errorSym('Unsuppported enum member name', member.name), node);
+                    recordEntry(errorSym('Unsupported enum member name', member.name), node);
               }
             }
             if (writtenMembers) {

diff --git a/packages/compiler-cli/src/metadata/evaluator.ts b/packages/compiler-cli/src/metadata/evaluator.ts
@@ -356,9 +356,6 @@ export class Evaluator {
           }
         }
         const args = arrayOrEmpty(callExpression.arguments).map(arg => this.evaluateNode(arg));
-        if (!this.options.verboseInvalidExpression && args.some(isMetadataError)) {
-          return args.find(isMetadataError);
-        }
         if (this.isFoldable(callExpression)) {
           if (isMethodCallOf(callExpression, 'concat')) {
             const arrayValue = <MetadataValue[]>this.evaluateNode(

diff --git a/packages/compiler-cli/test/metadata/collector_spec.ts b/packages/compiler-cli/test/metadata/collector_spec.ts
@@ -1054,10 +1054,20 @@ describe('Collector', () => {
       expect(metadata.metadata.MyComponent).toEqual({
         __symbolic: 'class',
         decorators: [{
-          __symbolic: 'error',
-          message: 'Expression form not supported',
-          line: 5,
-          character: 55
+          __symbolic: 'call',
+          expression: {
+            __symbolic: 'reference',
+            module: '@angular/core',
+            name: 'Component',
+            line: 4,
+            character: 9
+          },
+          arguments: [{
+            __symbolic: 'error',
+            message: 'Expression form not supported',
+            line: 5,
+            character: 55
+          }]
         }]
       });
     });

diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts
@@ -1698,6 +1698,40 @@ describe('ngc transformer command-line', () => {
       expect(messages[0]).toContain(`is imported recursively by the module 'MyFaultyImport`);
     });
 
+    // Regression test for #21273
+    it('should not report errors for unknown property annotations', () => {
+      write('src/tsconfig.json', `{
+        "extends": "../tsconfig-base.json",
+        "files": ["test-module.ts"]
+      }`);
+
+      write('src/test-decorator.ts', `
+        export function Convert(p: any): any {
+          // Make sur this doesn't look like a macro function
+          var r = p;
+          return r;
+        }
+      `);
+      write('src/test-module.ts', `
+        import {Component, Input, NgModule} from '@angular/core';
+        import {Convert} from './test-decorator';
+
+        @Component({template: '{{name}}'})
+        export class TestComponent {
+          @Input() @Convert(convert) name: string;
+        }
+
+        function convert(n: any) { return n; }
+
+        @NgModule({declarations: [TestComponent]})
+        export class TestModule {}
+      `);
+      const messages: string[] = [];
+      expect(
+          main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message)))
+          .toBe(0, `Compile failed:\n ${messages.join('\n    ')}`);
+    });
+
     it('should allow using 2 classes with the same name in declarations with noEmitOnError=true',
        () => {
          write('src/tsconfig.json', `{

diff --git a/packages/compiler/test/aot/static_reflector_spec.ts b/packages/compiler/test/aot/static_reflector_spec.ts
@@ -364,7 +364,8 @@ describe('StaticReflector', () => {
       const classData: any = moduleMetadata['InvalidMetadata'];
       expect(classData).toBeDefined();
       simplify(
-          reflector.getStaticSymbol('/tmp/src/invalid-metadata.ts', ''), classData.decorators[0]);
+          reflector.getStaticSymbol('/tmp/src/invalid-metadata.ts', ''),
+          classData.decorators[0].arguments);
     } catch (e) {
       expect(e.position).toBeDefined();
       threw = true;