diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index 0d7901332c..3d201c8f73 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/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,7 +208,9 @@ 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; } @@ -215,10 +218,18 @@ class AstTranslator implements AstVisitor { 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); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 7911baacbd..cc58543a50 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/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; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index c691989bb9..66554b5277 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/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(`
`, ` + 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(``, ` + export class TestComponent { + person: { + name: string; + }; + }`); + + expect(messages).toEqual( + [`synthetic.html(1, 15): Type '2' is not assignable to type 'string'.`]); + }); + }); }); function diagnose( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts index 81279b37bb..0bc32c44da 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts @@ -12,17 +12,18 @@ 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', () => { @@ -30,12 +31,14 @@ describe('type check blocks diagnostics', () => { // 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 = `