From 12a119b26369012b1813ec9ef77203a7cd50b937 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Mon, 22 Jan 2018 11:30:58 -0800 Subject: [PATCH] fix(compiler-cli): do not fold errors past calls in the collector (#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: #21273 PR Close #21708 --- .../compiler-cli/src/metadata/collector.ts | 4 +-- .../compiler-cli/src/metadata/evaluator.ts | 3 -- .../test/metadata/collector_spec.ts | 18 +++++++--- packages/compiler-cli/test/ngc_spec.ts | 34 +++++++++++++++++++ .../test/aot/static_reflector_spec.ts | 3 +- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/packages/compiler-cli/src/metadata/collector.ts b/packages/compiler-cli/src/metadata/collector.ts index e2748ce9dfd19..77611644f155f 100644 --- 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 index 273db143c2843..cf5c2ea269649 100644 --- 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 = this.evaluateNode( diff --git a/packages/compiler-cli/test/metadata/collector_spec.ts b/packages/compiler-cli/test/metadata/collector_spec.ts index 34ca4a4979533..9d4bbf7e0e011 100644 --- 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 index eb80136e453c5..0bd7a62314388 100644 --- 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 index 0d13a2bd250e8..eb6f63640098c 100644 --- 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;