feat(compiler-cli): improve error messages produced during structural errors (#20459)

The errors produced when error were encountered while interpreting the content of a directive was often incomprehencible. With this change these kind of error messages should be easier to understand and diagnose. PR Close #20459
2017-11-14 17:49:47 -08:00
parent 1366762d12
commit 8ecda94899
25 changed files with 1247 additions and 308 deletions
--- a/packages/compiler-cli/test/metadata/collector_spec.ts
+++ b/packages/compiler-cli/test/metadata/collector_spec.ts
@ -112,7 +112,13 @@ describe('Collector', () => {
          __symbolic: 'class',
          decorators: [{
            __symbolic: 'call',
-            expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'},
+            expression: {
+              __symbolic: 'reference',
+              module: 'angular2/core',
+              name: 'Component',
+              line: 4,
+              character: 7
+            },
            arguments: [{
              selector: 'my-hero-detail',
              template: `
@ -132,8 +138,13 @@ describe('Collector', () => {
              __symbolic: 'property',
              decorators: [{
                __symbolic: 'call',
-                expression:
-                    {__symbolic: 'reference', module: 'angular2/core', name: 'Input'}
+                expression: {
+                  __symbolic: 'reference',
+                  module: 'angular2/core',
+                  name: 'Input',
+                  line: 18,
+                  character: 9
+                }
              }]
            }]
          }
@ -153,7 +164,13 @@ describe('Collector', () => {
          __symbolic: 'class',
          decorators: [{
            __symbolic: 'call',
-            expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'},
+            expression: {
+              __symbolic: 'reference',
+              module: 'angular2/core',
+              name: 'Component',
+              line: 9,
+              character: 7
+            },
            arguments: [{
              selector: 'my-app',
              template: `
@ -172,20 +189,52 @@ describe('Collector', () => {
                  __symbolic: 'reference',
                  module: './hero-detail.component',
                  name: 'HeroDetailComponent',
+                  line: 22,
+                  character: 21
                },
-                {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor'}
+                {
+                  __symbolic: 'reference',
+                  module: 'angular2/common',
+                  name: 'NgFor',
+                  line: 22,
+                  character: 42
+                }
              ],
-              providers: [{__symbolic: 'reference', module: './hero.service', default: true}],
+              providers: [{
+                __symbolic: 'reference',
+                module: './hero.service',
+                default: true,
+                line: 23,
+                character: 20
+              }],
              pipes: [
-                {__symbolic: 'reference', module: 'angular2/common', name: 'LowerCasePipe'},
-                {__symbolic: 'reference', module: 'angular2/common', name: 'UpperCasePipe'}
+                {
+                  __symbolic: 'reference',
+                  module: 'angular2/common',
+                  name: 'LowerCasePipe',
+                  line: 24,
+                  character: 16
+                },
+                {
+                  __symbolic: 'reference',
+                  module: 'angular2/common',
+                  name: 'UpperCasePipe',
+                  line: 24,
+                  character: 38
+                }
              ]
            }]
          }],
          members: {
            __ctor__: [{
              __symbolic: 'constructor',
-              parameters: [{__symbolic: 'reference', module: './hero.service', default: true}]
+              parameters: [{
+                __symbolic: 'reference',
+                module: './hero.service',
+                default: true,
+                line: 31,
+                character: 42
+              }]
            }],
            onSelect: [{__symbolic: 'method'}],
            ngOnInit: [{__symbolic: 'method'}],
@ -236,22 +285,23 @@ describe('Collector', () => {
  });

  it('should record annotations on set and get declarations', () => {
-    const propertyData = {
+    const propertyData = (line: number) => ({
      name: [{
        __symbolic: 'property',
        decorators: [{
          __symbolic: 'call',
-          expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Input'},
+          expression:
+              {__symbolic: 'reference', module: 'angular2/core', name: 'Input', line, character: 9},
          arguments: ['firstName']
        }]
      }]
-    };
+    });
    const caseGetProp = <ClassMetadata>casesMetadata.metadata['GetProp'];
-    expect(caseGetProp.members).toEqual(propertyData);
+    expect(caseGetProp.members).toEqual(propertyData(11));
    const caseSetProp = <ClassMetadata>casesMetadata.metadata['SetProp'];
-    expect(caseSetProp.members).toEqual(propertyData);
+    expect(caseSetProp.members).toEqual(propertyData(19));
    const caseFullProp = <ClassMetadata>casesMetadata.metadata['FullProp'];
-    expect(caseFullProp.members).toEqual(propertyData);
+    expect(caseFullProp.members).toEqual(propertyData(27));
  });

  it('should record references to parameterized types', () => {
@ -260,7 +310,13 @@ describe('Collector', () => {
      __symbolic: 'class',
      decorators: [{
        __symbolic: 'call',
-        expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Injectable'}
+        expression: {
+          __symbolic: 'reference',
+          module: 'angular2/core',
+          name: 'Injectable',
+          line: 40,
+          character: 7
+        }
      }],
      members: {
        __ctor__: [{
@ -313,7 +369,7 @@ describe('Collector', () => {
    const ctor = <ConstructorMetadata>someClass.members !['__ctor__'][0];
    const parameters = ctor.parameters;
    expect(parameters).toEqual([
-      {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor'}
+      {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor', line: 6, character: 29}
    ]);
  });

@ -398,7 +454,7 @@ describe('Collector', () => {
    const ctor = <ConstructorMetadata>someClass.members !['__ctor__'][0];
    const parameters = ctor.parameters;
    expect(parameters).toEqual([
-      {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor'}
+      {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor', line: 6, character: 29}
    ]);
  });

@ -427,7 +483,13 @@ describe('Collector', () => {
      B: 1,
      C: 30,
      D: 40,
-      E: {__symbolic: 'reference', module: './exported-consts', name: 'constValue'}
+      E: {
+        __symbolic: 'reference',
+        module: './exported-consts',
+        name: 'constValue',
+        line: 5,
+        character: 75
+      }
    });
  });

@ -457,13 +519,25 @@ describe('Collector', () => {
    expect(classData).toBeDefined();
    expect(classData.decorators).toEqual([{
      __symbolic: 'call',
-      expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'},
+      expression: {
+        __symbolic: 'reference',
+        module: 'angular2/core',
+        name: 'Component',
+        line: 4,
+        character: 5
+      },
      arguments: [{
        providers: {
          __symbolic: 'call',
          expression: {
            __symbolic: 'select',
-            expression: {__symbolic: 'reference', module: './static-method', name: 'MyModule'},
+            expression: {
+              __symbolic: 'reference',
+              module: './static-method',
+              name: 'MyModule',
+              line: 5,
+              character: 17
+            },
            member: 'with'
          },
          arguments: ['a']
@ -489,13 +563,25 @@ describe('Collector', () => {
    expect(classData).toBeDefined();
    expect(classData.decorators).toEqual([{
      __symbolic: 'call',
-      expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'},
+      expression: {
+        __symbolic: 'reference',
+        module: 'angular2/core',
+        name: 'Component',
+        line: 4,
+        character: 5
+      },
      arguments: [{
        providers: [{
          provide: 'a',
          useValue: {
            __symbolic: 'select',
-            expression: {__symbolic: 'reference', module: './static-field', name: 'MyModule'},
+            expression: {
+              __symbolic: 'reference',
+              module: './static-field',
+              name: 'MyModule',
+              line: 5,
+              character: 45
+            },
            member: 'VALUE'
          }
        }]
@ -578,8 +664,20 @@ describe('Collector', () => {
    const metadata = collector.getMetadata(source) !;
    expect(metadata.metadata).toEqual({
      MyClass: Object({__symbolic: 'class'}),
-      OtherModule: {__symbolic: 'reference', module: './static-field-reference', name: 'Foo'},
-      MyOtherModule: {__symbolic: 'reference', module: './static-field', name: 'MyModule'}
+      OtherModule: {
+        __symbolic: 'reference',
+        module: './static-field-reference',
+        name: 'Foo',
+        line: 4,
+        character: 12
+      },
+      MyOtherModule: {
+        __symbolic: 'reference',
+        module: './static-field',
+        name: 'MyModule',
+        line: 4,
+        character: 25
+      }
    });
  });

@ -598,7 +696,13 @@ describe('Collector', () => {
        __symbolic: 'class',
        decorators: [{
          __symbolic: 'call',
-          expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'},
+          expression: {
+            __symbolic: 'reference',
+            module: 'angular2/core',
+            name: 'Component',
+            line: 11,
+            character: 5
+          },
          arguments: [{providers: [{__symbolic: 'reference', name: 'REQUIRED_VALIDATOR'}]}]
        }]
      }
@ -620,7 +724,13 @@ describe('Collector', () => {
        __symbolic: 'class',
        decorators: [{
          __symbolic: 'call',
-          expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'},
+          expression: {
+            __symbolic: 'reference',
+            module: 'angular2/core',
+            name: 'Component',
+            line: 11,
+            character: 5
+          },
          arguments: [{providers: [{__symbolic: 'reference', name: 'REQUIRED_VALIDATOR'}]}]
        }]
      }
@ -653,7 +763,13 @@ describe('Collector', () => {
        __symbolic: 'constructor',
        parameterDecorators: [[{
          __symbolic: 'call',
-          expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Inject'},
+          expression: {
+            __symbolic: 'reference',
+            module: 'angular2/core',
+            name: 'Inject',
+            line: 6,
+            character: 19
+          },
          arguments: ['a']
        }]],
        parameters: [{__symbolic: 'reference', name: 'any'}]
@ -687,13 +803,20 @@ describe('Collector', () => {
          __symbolic: 'reference',
          module: './external',
          name: 'external',
+          line: 0,
+          character: 68,
        }
      });
    });

    it('should simplify a redundant template', () => {
-      e('`${external}`', 'import {external} from "./external";')
-          .toEqual({__symbolic: 'reference', module: './external', name: 'external'});
+      e('`${external}`', 'import {external} from "./external";').toEqual({
+        __symbolic: 'reference',
+        module: './external',
+        name: 'external',
+        line: 0,
+        character: 59
+      });
    });

    it('should be able to collect complex template with imported references', () => {
@ -710,11 +833,18 @@ describe('Collector', () => {
              __symbolic: 'binop',
              operator: '+',
              left: 'foo:',
-              right: {__symbolic: 'reference', module: './external', name: 'foo'}
+              right: {
+                __symbolic: 'reference',
+                module: './external',
+                name: 'foo',
+                line: 0,
+                character: 63
+              }
            },
            right: ', bar:'
          },
-          right: {__symbolic: 'reference', module: './external', name: 'bar'}
+          right:
+              {__symbolic: 'reference', module: './external', name: 'bar', line: 0, character: 75}
        },
        right: ', end'
      });
@ -741,11 +871,11 @@ describe('Collector', () => {
      __ctor__: [{
        __symbolic: 'constructor',
        parameters: [
-          {__symbolic: 'reference', module: './foo', name: 'Foo'},
-          {__symbolic: 'reference', module: './foo', name: 'Foo'},
-          {__symbolic: 'reference', module: './foo', name: 'Foo'},
-          {__symbolic: 'reference', module: './foo', name: 'Foo'},
-          {__symbolic: 'reference', module: './foo', name: 'Foo'}
+          {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24},
+          {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24},
+          {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24},
+          {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24},
+          {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24}
        ]
      }]
    });
@ -825,7 +955,9 @@ describe('Collector', () => {
        extends: {
          __symbolic: 'reference',
          module: './class-inheritance-parent',
-          name: 'ParentClassFromOtherFile'
+          name: 'ParentClassFromOtherFile',
+          line: 9,
+          character: 45,
        }
      });
    });
--- a/packages/compiler-cli/test/metadata/evaluator_spec.ts
+++ b/packages/compiler-cli/test/metadata/evaluator_spec.ts
@ -149,12 +149,14 @@ describe('Evaluator', () => {
    const newExpression = program.getSourceFile('newExpression.ts');
    expect(evaluator.evaluateNode(findVarInitializer(newExpression, 'someValue'))).toEqual({
      __symbolic: 'new',
-      expression: {__symbolic: 'reference', name: 'Value', module: './classes'},
+      expression:
+          {__symbolic: 'reference', name: 'Value', module: './classes', line: 4, character: 33},
      arguments: ['name', 12]
    });
    expect(evaluator.evaluateNode(findVarInitializer(newExpression, 'complex'))).toEqual({
      __symbolic: 'new',
-      expression: {__symbolic: 'reference', name: 'Value', module: './classes'},
+      expression:
+          {__symbolic: 'reference', name: 'Value', module: './classes', line: 5, character: 42},
      arguments: ['name', 12]
    });
  });
@ -173,8 +175,7 @@ describe('Evaluator', () => {
    const errors = program.getSourceFile('errors.ts');
    const fDecl = findVar(errors, 'f') !;
    expect(evaluator.evaluateNode(fDecl.initializer !))
-        .toEqual(
-            {__symbolic: 'error', message: 'Function call not supported', line: 1, character: 12});
+        .toEqual({__symbolic: 'error', message: 'Lambda not supported', line: 1, character: 12});
    const eDecl = findVar(errors, 'e') !;
    expect(evaluator.evaluateNode(eDecl.type !)).toEqual({
      __symbolic: 'error',
--- a/packages/compiler-cli/test/ngc_spec.ts
+++ b/packages/compiler-cli/test/ngc_spec.ts
@ -184,8 +184,7 @@ describe('ngc transformer command-line', () => {

      const exitCode = main(['-p', basePath], errorSpy);
      expect(errorSpy).toHaveBeenCalledTimes(1);
-      expect(errorSpy.calls.mostRecent().args[0])
-          .toContain('Error at ' + path.join(basePath, 'mymodule.ts.MyComp.html'));
+      expect(errorSpy.calls.mostRecent().args[0]).toContain('mymodule.ts.MyComp.html');
      expect(errorSpy.calls.mostRecent().args[0])
          .toContain(`Property 'unknownProp' does not exist on type 'MyComp'`);

@ -215,8 +214,7 @@ describe('ngc transformer command-line', () => {

      const exitCode = main(['-p', basePath], errorSpy);
      expect(errorSpy).toHaveBeenCalledTimes(1);
-      expect(errorSpy.calls.mostRecent().args[0])
-          .toContain('Error at ' + path.join(basePath, 'my.component.html(1,5):'));
+      expect(errorSpy.calls.mostRecent().args[0]).toContain('my.component.html(1,5):');
      expect(errorSpy.calls.mostRecent().args[0])
          .toContain(`Property 'unknownProp' does not exist on type 'MyComp'`);

@ -1566,4 +1564,49 @@ describe('ngc transformer command-line', () => {
         expect(main(['-p', path.join(basePath, 'src/tsconfig.json')])).toBe(0);
       });
  });
+
+  describe('formatted messages', () => {
+    it('should emit a formatted error message for a structural error', () => {
+      write('src/tsconfig.json', `{
+        "extends": "../tsconfig-base.json",
+        "files": ["test-module.ts"]
+      }`);
+      write('src/lib/indirect2.ts', `
+        declare var f: any;
+
+        export const t2 = f\`<p>hello</p>\`;
+      `);
+      write('src/lib/indirect1.ts', `
+        import {t2} from './indirect2';
+        export const t1 = t2 + ' ';
+      `);
+      write('src/lib/test.component.ts', `
+        import {Component} from '@angular/core';
+        import {t1} from './indirect1';
+
+        @Component({
+          template: t1,
+          styleUrls: ['./test.component.css']
+        })
+        export class TestComponent {}
+      `);
+      write('src/test-module.ts', `
+        import {NgModule} from '@angular/core';
+        import {TestComponent} from './lib/test.component';
+
+        @NgModule({declarations: [TestComponent]})
+        export class TestModule {}
+      `);
+      const messages: string[] = [];
+      const exitCode =
+          main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message));
+      expect(exitCode).toBe(1, 'Compile was expected to fail');
+      expect(messages[0])
+          .toEqual(`lib/test.component.ts(6,21): Error during template compile of 'TestComponent'
+  Tagged template expressions are not supported in metadata in 't1'
+    't1' references 't2' at lib/indirect1.ts(3,27)
+      't2' contains the error at lib/indirect2.ts(4,27).
+`);
+    });
+  });
 });
--- a/packages/compiler-cli/test/perform_watch_spec.ts
+++ b/packages/compiler-cli/test/perform_watch_spec.ts
@ -143,7 +143,7 @@ describe('perform watch', () => {

      const errDiags = host.diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error);
      expect(errDiags.length).toBe(1);
-      expect(errDiags[0].messageText).toContain('Function calls are not supported.');
+      expect(errDiags[0].messageText).toContain('Function expressions are not supported');
    }
  });
 });
--- a/packages/compiler-cli/test/transformers/program_spec.ts
+++ b/packages/compiler-cli/test/transformers/program_spec.ts
@ -930,7 +930,7 @@ describe('ng program', () => {

      const structuralErrors = program.getNgStructuralDiagnostics();
      expect(structuralErrors.length).toBe(1);
-      expect(structuralErrors[0].messageText).toContain('Function calls are not supported.');
+      expect(structuralErrors[0].messageText).toContain('Function expressions are not supported');
    });

    it('should not throw on structural errors but collect them (loadNgStructureAsync)', (done) => {
@ -943,7 +943,7 @@ describe('ng program', () => {
      program.loadNgStructureAsync().then(() => {
        const structuralErrors = program.getNgStructuralDiagnostics();
        expect(structuralErrors.length).toBe(1);
-        expect(structuralErrors[0].messageText).toContain('Function calls are not supported.');
+        expect(structuralErrors[0].messageText).toContain('Function expressions are not supported');
        done();
      });
    });
@ -982,7 +982,8 @@ describe('ng program', () => {
         const program = ng.createProgram({rootNames: allRootNames, options, host});
         const structuralErrors = program.getNgStructuralDiagnostics();
         expect(structuralErrors.length).toBe(1);
-         expect(structuralErrors[0].messageText).toContain('Function calls are not supported.');
+         expect(structuralErrors[0].messageText)
+             .toContain('Function expressions are not supported');
       });
  });
 });