feat(compiler): add name spans for property reads and method calls

packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts

@@ -10,7 +10,7 @@ import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional,
 import * as ts from 'typescript';
 
 import {TypeCheckingConfig} from './api';
-import {addParseSpanInfo, ignoreDiagnostics, wrapForDiagnostics} from './diagnostics';
+import {addParseSpanInfo, wrapForDiagnostics} from './diagnostics';
 import {tsCastToAny} from './ts_util';
 
 export const NULL_AS_ANY =
@@ -179,6 +179,7 @@ class AstTranslator implements AstVisitor {
   visitMethodCall(ast: MethodCall): ts.Expression {
     const receiver = wrapForDiagnostics(this.translate(ast.receiver));
     const method = ts.createPropertyAccess(receiver, ast.name);
+    addParseSpanInfo(method, ast.nameSpan);
     const args = ast.args.map(expr => this.translate(expr));
     const node = ts.createCall(method, undefined, args);
     addParseSpanInfo(node, ast.sourceSpan);
@@ -207,18 +208,28 @@ class AstTranslator implements AstVisitor {
     // This is a normal property read - convert the receiver to an expression and emit the correct
     // TypeScript expression to read the property.
     const receiver = wrapForDiagnostics(this.translate(ast.receiver));
-    const node = ts.createPropertyAccess(receiver, ast.name);
+    const name = ts.createPropertyAccess(receiver, ast.name);
+    addParseSpanInfo(name, ast.nameSpan);
+    const node = wrapForDiagnostics(name);
     addParseSpanInfo(node, ast.sourceSpan);
     return node;
   }
 
   visitPropertyWrite(ast: PropertyWrite): ts.Expression {
     const receiver = wrapForDiagnostics(this.translate(ast.receiver));
     const left = ts.createPropertyAccess(receiver, ast.name);
-    // TODO(joost): annotate `left` with the span of the property access, which is not currently
-    //  available on `ast`.
+    addParseSpanInfo(left, ast.nameSpan);
+    // TypeScript reports assignment errors on the entire lvalue expression. Annotate the lvalue of
+    // the assignment with the sourceSpan, which includes receivers, rather than nameSpan for
+    // consistency of the diagnostic location.
+    // a.b.c = 1
+    // ^^^^^^^^^ sourceSpan
+    //     ^     nameSpan
+    const leftWithPath = wrapForDiagnostics(left);
+    addParseSpanInfo(leftWithPath, ast.sourceSpan);
     const right = this.translate(ast.value);
-    const node = wrapForDiagnostics(ts.createBinary(left, ts.SyntaxKind.EqualsToken, right));
+    const node =
+        wrapForDiagnostics(ts.createBinary(leftWithPath, ts.SyntaxKind.EqualsToken, right));
     addParseSpanInfo(node, ast.sourceSpan);
     return node;
   }
@@ -235,15 +246,18 @@ class AstTranslator implements AstVisitor {
     if (this.config.strictSafeNavigationTypes) {
       // "a?.method(...)" becomes (null as any ? a!.method(...) : undefined)
       const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name);
+      addParseSpanInfo(method, ast.nameSpan);
       const call = ts.createCall(method, undefined, args);
       node = ts.createParen(ts.createConditional(NULL_AS_ANY, call, UNDEFINED));
     } else if (VeSafeLhsInferenceBugDetector.veWillInferAnyFor(ast)) {
       // "a?.method(...)" becomes (a as any).method(...)
       const method = ts.createPropertyAccess(tsCastToAny(receiver), ast.name);
+      addParseSpanInfo(method, ast.nameSpan);
       node = ts.createCall(method, undefined, args);
     } else {
       // "a?.method(...)" becomes (a!.method(...) as any)
       const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name);
+      addParseSpanInfo(method, ast.nameSpan);
       node = tsCastToAny(ts.createCall(method, undefined, args));
     }
     addParseSpanInfo(node, ast.sourceSpan);

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

@@ -1125,6 +1125,7 @@ class TcbExpressionTranslator {
       return result;
     } else if (ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver) {
       // Resolve the special `$any(expr)` syntax to insert a cast of the argument to type `any`.
+      // `$any(expr)` -> `expr as any`
       if (ast.name === '$any' && ast.args.length === 1) {
         const expr = this.translate(ast.args[0]);
         const exprAsAny =
@@ -1144,6 +1145,7 @@ class TcbExpressionTranslator {
       }
 
       const method = ts.createPropertyAccess(wrapForDiagnostics(receiver), ast.name);
+      addParseSpanInfo(method, ast.nameSpan);
       const args = ast.args.map(arg => this.translate(arg));
       const node = ts.createCall(method, undefined, args);
       addParseSpanInfo(node, ast.sourceSpan);
@@ -1435,7 +1437,7 @@ class TcbEventHandlerTranslator extends TcbExpressionTranslator {
     if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver &&
         ast.name === EVENT_PARAMETER) {
       const event = ts.createIdentifier(EVENT_PARAMETER);
-      addParseSpanInfo(event, ast.sourceSpan);
+      addParseSpanInfo(event, ast.nameSpan);
       return event;
     }
 

packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts

@@ -130,7 +130,7 @@ runInEachFileSystem(() => {
           [ngForDeclaration()], [ngForDts()]);
 
       expect(messages).toEqual([
-        `synthetic.html(1, 40): Property 'namme' does not exist on type '{ name: string; }'. Did you mean 'name'?`,
+        `synthetic.html(1, 47): Property 'namme' does not exist on type '{ name: string; }'. Did you mean 'name'?`,
       ]);
     });
 
@@ -329,7 +329,7 @@ runInEachFileSystem(() => {
           };
         }`);
 
-        expect(messages).toEqual([`synthetic.html(1, 26): Object is possibly 'undefined'.`]);
+        expect(messages).toEqual([`synthetic.html(1, 41): Object is possibly 'undefined'.`]);
       });
 
       it('does not produce diagnostic for checked property access', () => {
@@ -367,6 +367,85 @@ class TestComponent {
       ]);
     });
   });
+
+  describe('method call spans', () => {
+    it('reports invalid method name on method name span', () => {
+      const messages = diagnose(`{{ person.getNName() }}`, `
+        export class TestComponent {
+          person: {
+            getName(): string;
+          };
+        }`);
+
+      expect(messages).toEqual([
+        `synthetic.html(1, 11): Property 'getNName' does not exist on type '{ getName(): string; }'. Did you mean 'getName'?`
+      ]);
+    });
+
+    it('reports invalid method call signature on parameter span', () => {
+      const messages = diagnose(`{{ person.getName('abcd') }}`, `
+        export class TestComponent {
+          person: {
+            getName(): string;
+          };
+        }`);
+
+      expect(messages).toEqual([`synthetic.html(1, 19): Expected 0 arguments, but got 1.`]);
+    });
+  });
+
+  describe('safe method call spans', () => {
+    it('reports invalid method name on method name span', () => {
+      const messages = diagnose(`{{ person?.getNName() }}`, `
+        export class TestComponent {
+          person?: {
+            getName(): string;
+          };
+        }`);
+
+      expect(messages).toEqual([
+        `synthetic.html(1, 12): Property 'getNName' does not exist on type '{ getName(): string; }'. Did you mean 'getName'?`
+      ]);
+    });
+
+    it('reports invalid method call signature on parameter span', () => {
+      const messages = diagnose(`{{ person?.getName('abcd') }}`, `
+        export class TestComponent {
+          person?: {
+            getName(): string;
+          };
+        }`);
+
+      expect(messages).toEqual([`synthetic.html(1, 20): Expected 0 arguments, but got 1.`]);
+    });
+  });
+
+  describe('property write spans', () => {
+    it('reports invalid receiver property access on property access name span', () => {
+      const messages = diagnose(`<div (click)="person.nname = 'jacky'"></div>`, `
+        export class TestComponent {
+          person: {
+            name: string;
+          };
+        }`);
+
+      expect(messages).toEqual([
+        `synthetic.html(1, 22): Property 'nname' does not exist on type '{ name: string; }'. Did you mean 'name'?`
+      ]);
+    });
+
+    it('reports unassignable value on property write span', () => {
+      const messages = diagnose(`<div (click)="person.name = 2"></div>`, `
+        export class TestComponent {
+          person: {
+            name: string;
+          };
+        }`);
+
+      expect(messages).toEqual(
+          [`synthetic.html(1, 15): Type '2' is not assignable to type 'string'.`]);
+    });
+  });
 });
 
 function diagnose(

packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts

@@ -12,30 +12,33 @@ describe('type check blocks diagnostics', () => {
   describe('parse spans', () => {
     it('should annotate binary ops', () => {
       expect(tcbWithSpans('{{ a + b }}'))
-          .toContain('"" + (((ctx).a /*3,4*/) + ((ctx).b /*7,8*/) /*3,8*/);');
+          .toContain('(((ctx).a /*3,4*/) /*3,4*/) + (((ctx).b /*7,8*/) /*7,8*/) /*3,8*/');
     });
 
     it('should annotate conditions', () => {
       expect(tcbWithSpans('{{ a ? b : c }}'))
-          .toContain('((ctx).a /*3,4*/ ? (ctx).b /*7,8*/ : (ctx).c /*11,12*/) /*3,12*/;');
+          .toContain(
+              '(((ctx).a /*3,4*/) /*3,4*/ ? ((ctx).b /*7,8*/) /*7,8*/ : ((ctx).c /*11,12*/) /*11,12*/) /*3,12*/');
     });
 
     it('should annotate interpolations', () => {
       expect(tcbWithSpans('{{ hello }} {{ world }}'))
-          .toContain('"" + (ctx).hello /*3,8*/ + (ctx).world /*15,20*/;');
+          .toContain('"" + ((ctx).hello /*3,8*/) /*3,8*/ + ((ctx).world /*15,20*/) /*15,20*/');
     });
 
     it('should annotate literal map expressions', () => {
       // The additional method call is present to avoid that the object literal is emitted as
       // statement, which would wrap it into parenthesis that clutter the expected output.
       const TEMPLATE = '{{ m({foo: a, bar: b}) }}';
       expect(tcbWithSpans(TEMPLATE))
-          .toContain('m({ "foo": (ctx).a /*11,12*/, "bar": (ctx).b /*19,20*/ } /*5,21*/)');
+          .toContain(
+              '(ctx).m /*3,4*/({ "foo": ((ctx).a /*11,12*/) /*11,12*/, "bar": ((ctx).b /*19,20*/) /*19,20*/ } /*5,21*/) /*3,22*/');
     });
 
     it('should annotate literal array expressions', () => {
       const TEMPLATE = '{{ [a, b] }}';
-      expect(tcbWithSpans(TEMPLATE)).toContain('[(ctx).a /*4,5*/, (ctx).b /*7,8*/] /*3,9*/;');
+      expect(tcbWithSpans(TEMPLATE))
+          .toContain('[((ctx).a /*4,5*/) /*4,5*/, ((ctx).b /*7,8*/) /*7,8*/] /*3,9*/');
     });
 
     it('should annotate literals', () => {
@@ -45,77 +48,84 @@ describe('type check blocks diagnostics', () => {
 
     it('should annotate non-null assertions', () => {
       const TEMPLATE = `{{ a! }}`;
-      expect(tcbWithSpans(TEMPLATE)).toContain('(((ctx).a /*3,4*/)! /*3,5*/);');
+      expect(tcbWithSpans(TEMPLATE)).toContain('(((ctx).a /*3,4*/) /*3,4*/)! /*3,5*/');
     });
 
     it('should annotate prefix not', () => {
       const TEMPLATE = `{{ !a }}`;
-      expect(tcbWithSpans(TEMPLATE)).toContain('!((ctx).a /*4,5*/) /*3,5*/;');
+      expect(tcbWithSpans(TEMPLATE)).toContain('!(((ctx).a /*4,5*/) /*4,5*/) /*3,5*/;');
     });
 
     it('should annotate method calls', () => {
       const TEMPLATE = `{{ method(a, b) }}`;
       expect(tcbWithSpans(TEMPLATE))
-          .toContain('(ctx).method((ctx).a /*10,11*/, (ctx).b /*13,14*/) /*3,15*/;');
+          .toContain(
+              '(ctx).method /*3,9*/(((ctx).a /*10,11*/) /*10,11*/, ((ctx).b /*13,14*/) /*13,14*/) /*3,15*/');
     });
 
     it('should annotate method calls of variables', () => {
       const TEMPLATE = `<ng-template let-method>{{ method(a, b) }}</ng-template>`;
       expect(tcbWithSpans(TEMPLATE))
-          .toContain('(_t2 /*27,39*/).method((ctx).a /*34,35*/, (ctx).b /*37,38*/) /*27,39*/;');
+          .toContain(
+              '(_t2 /*27,39*/).method /*27,33*/(((ctx).a /*34,35*/) /*34,35*/, ((ctx).b /*37,38*/) /*37,38*/) /*27,39*/');
     });
 
     it('should annotate function calls', () => {
       const TEMPLATE = `{{ method(a)(b, c) }}`;
       expect(tcbWithSpans(TEMPLATE))
           .toContain(
-              '((ctx).method((ctx).a /*10,11*/) /*3,12*/)((ctx).b /*13,14*/, (ctx).c /*16,17*/) /*3,18*/;');
+              '((ctx).method /*3,9*/(((ctx).a /*10,11*/) /*10,11*/) /*3,12*/)(((ctx).b /*13,14*/) /*13,14*/, ((ctx).c /*16,17*/) /*16,17*/) /*3,18*/');
     });
 
     it('should annotate property access', () => {
       const TEMPLATE = `{{ a.b.c }}`;
-      expect(tcbWithSpans(TEMPLATE)).toContain('(((ctx).a /*3,4*/).b /*3,6*/).c /*3,8*/;');
+      expect(tcbWithSpans(TEMPLATE))
+          .toContain('((((((ctx).a /*3,4*/) /*3,4*/).b /*5,6*/) /*3,6*/).c /*7,8*/) /*3,8*/');
     });
 
     it('should annotate property writes', () => {
-      const TEMPLATE = `<div (click)="a.b.c = d"></div>`;
+      const TEMPLATE = `<div (click)='a.b.c = d'></div>`;
       expect(tcbWithSpans(TEMPLATE))
-          .toContain('((((ctx).a /*14,15*/).b /*14,17*/).c = (ctx).d /*22,23*/) /*14,23*/');
+          .toContain(
+              '(((((((ctx).a /*14,15*/) /*14,15*/).b /*16,17*/) /*14,17*/).c /*18,19*/) /*14,23*/ = ((ctx).d /*22,23*/) /*22,23*/) /*14,23*/');
     });
 
     it('should annotate keyed property access', () => {
       const TEMPLATE = `{{ a[b] }}`;
-      expect(tcbWithSpans(TEMPLATE)).toContain('((ctx).a /*3,4*/)[(ctx).b /*5,6*/] /*3,7*/;');
+      expect(tcbWithSpans(TEMPLATE))
+          .toContain('(((ctx).a /*3,4*/) /*3,4*/)[((ctx).b /*5,6*/) /*5,6*/] /*3,7*/');
     });
 
     it('should annotate keyed property writes', () => {
       const TEMPLATE = `<div (click)="a[b] = c"></div>`;
       expect(tcbWithSpans(TEMPLATE))
-          .toContain('(((ctx).a /*14,15*/)[(ctx).b /*16,17*/] = (ctx).c /*21,22*/) /*14,22*/');
+          .toContain(
+              '((((ctx).a /*14,15*/) /*14,15*/)[((ctx).b /*16,17*/) /*16,17*/] = ((ctx).c /*21,22*/) /*21,22*/) /*14,22*/');
     });
 
     it('should annotate safe property access', () => {
       const TEMPLATE = `{{ a?.b }}`;
       expect(tcbWithSpans(TEMPLATE))
-          .toContain('((null as any) ? ((ctx).a /*3,4*/)!.b : undefined) /*3,7*/');
+          .toContain('((null as any) ? (((ctx).a /*3,4*/) /*3,4*/)!.b : undefined) /*3,7*/');
     });
 
     it('should annotate safe method calls', () => {
       const TEMPLATE = `{{ a?.method(b) }}`;
       expect(tcbWithSpans(TEMPLATE))
           .toContain(
-              '((null as any) ? ((ctx).a /*3,4*/)!.method((ctx).b /*13,14*/) : undefined) /*3,15*/');
+              '((null as any) ? (((ctx).a /*3,4*/) /*3,4*/)!.method /*6,12*/(((ctx).b /*13,14*/) /*13,14*/) : undefined) /*3,15*/');
     });
 
     it('should annotate $any casts', () => {
       const TEMPLATE = `{{ $any(a) }}`;
-      expect(tcbWithSpans(TEMPLATE)).toContain('((ctx).a /*8,9*/ as any) /*3,10*/;');
+      expect(tcbWithSpans(TEMPLATE)).toContain('(((ctx).a /*8,9*/) /*8,9*/ as any) /*3,10*/');
     });
 
     it('should annotate chained expressions', () => {
-      const TEMPLATE = `<div (click)="a; b; c"></div>`;
+      const TEMPLATE = `<div (click)='a; b; c'></div>`;
       expect(tcbWithSpans(TEMPLATE))
-          .toContain('((ctx).a /*14,15*/, (ctx).b /*17,18*/, (ctx).c /*20,21*/) /*14,21*/');
+          .toContain(
+              '(((ctx).a /*14,15*/) /*14,15*/, ((ctx).b /*17,18*/) /*17,18*/, ((ctx).c /*20,21*/) /*20,21*/) /*14,21*/');
     });
 
     it('should annotate pipe usages', () => {
@@ -127,7 +137,7 @@ describe('type check blocks diagnostics', () => {
       }];
       const block = tcbWithSpans(TEMPLATE, PIPES);
       expect(block).toContain(
-          '(null as TestPipe).transform((ctx).a /*3,4*/, (ctx).b /*12,13*/) /*3,13*/;');
+          '(null as TestPipe).transform(((ctx).a /*3,4*/) /*3,4*/, ((ctx).b /*12,13*/) /*12,13*/) /*3,13*/;');
     });
 
     describe('attaching multiple comments for multiple references', () => {
@@ -136,7 +146,7 @@ describe('type check blocks diagnostics', () => {
         expect(tcbWithSpans(TEMPLATE)).toContain('((_t1 /*19,20*/) || (_t1 /*24,25*/) /*19,25*/);');
       });
       it('should be correct for template vars', () => {
-        const TEMPLATE = `<ng-template let-a="b">{{ a || a }}</ng-template>`;
+        const TEMPLATE = `<ng-template let-a='b'>{{ a || a }}</ng-template>`;
         expect(tcbWithSpans(TEMPLATE)).toContain('((_t2 /*26,27*/) || (_t2 /*31,32*/) /*26,32*/);');
       });
       it('should be correct for directive refs', () => {