From eb34aa551a89c5c2ccf8ac27d130e974902bcc4e Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Mon, 27 Apr 2020 18:54:30 -0700 Subject: [PATCH] feat(compiler): add name spans for property reads and method calls (#36826) ASTs for property read and method calls contain information about the entire span of the expression, including its receiver. Use cases like a language service and compile error messages may be more interested in the span of the direct identifier for which the expression is constructed (i.e. an accessed property). To support this, this commit adds a `nameSpan` property on - `PropertyRead`s - `SafePropertyRead`s - `PropertyWrite`s - `MethodCall`s - `SafeMethodCall`s The `nameSpan` property already existed for `BindingPipe`s. This commit also updates usages of these expressions' `sourceSpan`s in Ngtsc and the langauge service to use `nameSpan`s where appropriate. PR Close #36826 --- .../src/ngtsc/typecheck/src/expression.ts | 24 ++++-- .../ngtsc/typecheck/src/type_check_block.ts | 4 +- .../ngtsc/typecheck/test/diagnostics_spec.ts | 83 ++++++++++++++++++- .../typecheck/test/span_comments_spec.ts | 54 +++++++----- .../typecheck/test/type_check_block_spec.ts | 67 ++++++++------- .../test/ngtsc/template_typecheck_spec.ts | 12 ++- .../src/compiler_util/expression_converter.ts | 8 +- .../compiler/src/expression_parser/ast.ts | 76 ++++++++++------- .../compiler/src/expression_parser/parser.ts | 50 ++++++++--- .../compiler/src/render3/view/template.ts | 2 +- .../test/expression_parser/parser_spec.ts | 54 +++++++++++- .../test/expression_parser/utils/unparser.ts | 33 +++++++- .../test/render3/r3_ast_absolute_span_spec.ts | 21 ++++- .../template_parser_absolute_span_spec.ts | 22 ++++- packages/language-service/src/completions.ts | 9 +- .../language-service/src/expression_type.ts | 59 ++++++++----- packages/language-service/src/expressions.ts | 39 ++++----- .../language-service/test/definitions_spec.ts | 4 +- .../language-service/test/diagnostics_spec.ts | 38 ++++----- packages/language-service/test/hover_spec.ts | 36 +++++++- 20 files changed, 496 insertions(+), 199 deletions(-) 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 = `{{ method(a, b) }}`; 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 = `
`; + const TEMPLATE = `
`; 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 = `
`; 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 = `
`; + const TEMPLATE = `
`; 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 = `{{ a || a }}`; + const TEMPLATE = `{{ a || a }}`; expect(tcbWithSpans(TEMPLATE)).toContain('((_t2 /*26,27*/) || (_t2 /*31,32*/) /*26,32*/);'); }); it('should be correct for directive refs', () => { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index f69af3763b..6c239c14e1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -13,32 +13,33 @@ import {ALL_ENABLED_CONFIG, tcb, TestDeclaration, TestDirective} from './test_ut describe('type check blocks', () => { it('should generate a basic block for a binding', () => { - expect(tcb('{{hello}} {{world}}')).toContain('"" + (ctx).hello + (ctx).world;'); + expect(tcb('{{hello}} {{world}}')).toContain('"" + ((ctx).hello) + ((ctx).world);'); }); it('should generate literal map expressions', () => { const TEMPLATE = '{{ method({foo: a, bar: b}) }}'; - expect(tcb(TEMPLATE)).toContain('(ctx).method({ "foo": (ctx).a, "bar": (ctx).b });'); + expect(tcb(TEMPLATE)).toContain('(ctx).method({ "foo": ((ctx).a), "bar": ((ctx).b) });'); }); it('should generate literal array expressions', () => { const TEMPLATE = '{{ method([a, b]) }}'; - expect(tcb(TEMPLATE)).toContain('(ctx).method([(ctx).a, (ctx).b]);'); + expect(tcb(TEMPLATE)).toContain('(ctx).method([((ctx).a), ((ctx).b)]);'); }); it('should handle non-null assertions', () => { const TEMPLATE = `{{a!}}`; - expect(tcb(TEMPLATE)).toContain('(((ctx).a)!);'); + expect(tcb(TEMPLATE)).toContain('((((ctx).a))!);'); }); it('should handle keyed property access', () => { const TEMPLATE = `{{a[b]}}`; - expect(tcb(TEMPLATE)).toContain('((ctx).a)[(ctx).b];'); + expect(tcb(TEMPLATE)).toContain('(((ctx).a))[((ctx).b)];'); }); it('should handle nested ternary expressions', () => { const TEMPLATE = `{{a ? b : c ? d : e}}`; - expect(tcb(TEMPLATE)).toContain('((ctx).a ? (ctx).b : ((ctx).c ? (ctx).d : (ctx).e))'); + expect(tcb(TEMPLATE)) + .toContain('(((ctx).a) ? ((ctx).b) : (((ctx).c) ? ((ctx).d) : ((ctx).e)))'); }); it('should handle attribute values for directive inputs', () => { @@ -115,7 +116,8 @@ describe('type check blocks', () => { }, }]; expect(tcb(TEMPLATE, DIRECTIVES)) - .toContain('var _t2 = Dir.ngTypeCtor({ "fieldA": ((ctx).foo), "fieldB": (null as any) });'); + .toContain( + 'var _t2 = Dir.ngTypeCtor({ "fieldA": (((ctx).foo)), "fieldB": (null as any) });'); }); it('should generate a forward element reference correctly', () => { @@ -123,7 +125,8 @@ describe('type check blocks', () => { {{ i.value }} `; - expect(tcb(TEMPLATE)).toContain('var _t1 = document.createElement("input"); "" + (_t1).value;'); + expect(tcb(TEMPLATE)) + .toContain('var _t1 = document.createElement("input"); "" + ((_t1).value);'); }); it('should generate a forward directive reference correctly', () => { @@ -139,7 +142,7 @@ describe('type check blocks', () => { }]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t1 = Dir.ngTypeCtor({}); "" + (_t1).value; var _t2 = document.createElement("div");'); + 'var _t1 = Dir.ngTypeCtor({}); "" + ((_t1).value); var _t2 = document.createElement("div");'); }); it('should handle style and class bindings specially', () => { @@ -147,7 +150,7 @@ describe('type check blocks', () => {
`; const block = tcb(TEMPLATE); - expect(block).toContain('(ctx).a; (ctx).b;'); + expect(block).toContain('((ctx).a); ((ctx).b);'); // There should be no assignments to the class or style properties. expect(block).not.toContain('.class = '); @@ -218,7 +221,7 @@ describe('type check blocks', () => { it('should handle $any casts', () => { const TEMPLATE = `{{$any(a)}}`; const block = tcb(TEMPLATE); - expect(block).toContain('((ctx).a as any);'); + expect(block).toContain('(((ctx).a) as any);'); }); describe('experimental DOM checking via lib.dom.d.ts', () => { @@ -244,7 +247,7 @@ describe('type check blocks', () => { }]; const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('if (NgIf.ngTemplateGuard_ngIf(_t1, (ctx).person))'); + expect(block).toContain('if (NgIf.ngTemplateGuard_ngIf(_t1, ((ctx).person)))'); }); it('should emit binding guards', () => { @@ -260,7 +263,7 @@ describe('type check blocks', () => { }]; const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('if (((ctx).person) !== (null))'); + expect(block).toContain('if ((((ctx).person)) !== (null))'); }); }); @@ -357,12 +360,12 @@ describe('type check blocks', () => { it('should descend into template bodies when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('(ctx).a;'); + expect(block).toContain('((ctx).a);'); }); it('should not descend into template bodies when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTemplateBodies: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).not.toContain('(ctx).a;'); + expect(block).not.toContain('((ctx).a);'); }); }); @@ -371,15 +374,15 @@ describe('type check blocks', () => { it('should include null and undefined when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a) })'); - expect(block).toContain('(ctx).b;'); + expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)) })'); + expect(block).toContain('((ctx).b);'); }); it('should use the non-null assertion operator when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, strictNullInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a!) })'); - expect(block).toContain('(ctx).b!;'); + expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)!) })'); + expect(block).toContain('((ctx).b)!;'); }); }); @@ -387,8 +390,8 @@ describe('type check blocks', () => { it('should check types of bindings when enabled', () => { const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a) })'); - expect(block).toContain('(ctx).b;'); + expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)) })'); + expect(block).toContain('((ctx).b);'); }); it('should not check types of bindings when disabled', () => { @@ -396,8 +399,8 @@ describe('type check blocks', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a as any)) })'); - expect(block).toContain('((ctx).b as any);'); + expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((((ctx).a) as any)) })'); + expect(block).toContain('(((ctx).b) as any);'); }); it('should wrap the cast to any in parentheses when required', () => { @@ -406,7 +409,7 @@ describe('type check blocks', () => { TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).toContain( - 'Dir.ngTypeCtor({ "dirInput": (((((ctx).a) === ((ctx).b)) as any)) })'); + 'Dir.ngTypeCtor({ "dirInput": ((((((ctx).a)) === (((ctx).b))) as any)) })'); }); }); @@ -550,12 +553,12 @@ describe('type check blocks', () => { it('should check types of pipes when enabled', () => { const block = tcb(TEMPLATE, PIPES); - expect(block).toContain('(null as TestPipe).transform((ctx).a, (ctx).b, (ctx).c);'); + expect(block).toContain('(null as TestPipe).transform(((ctx).a), ((ctx).b), ((ctx).c));'); }); it('should not check types of pipes when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfPipes: false}; const block = tcb(TEMPLATE, PIPES, DISABLED_CONFIG); - expect(block).toContain('(null as any).transform((ctx).a, (ctx).b, (ctx).c);'); + expect(block).toContain('(null as any).transform(((ctx).a), ((ctx).b), ((ctx).c));'); }); }); @@ -564,15 +567,15 @@ describe('type check blocks', () => { it('should use undefined for safe navigation operations when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('((null as any) ? ((ctx).a)!.method() : undefined)'); - expect(block).toContain('((null as any) ? ((ctx).a)!.b : undefined)'); + expect(block).toContain('((null as any) ? (((ctx).a))!.method() : undefined)'); + expect(block).toContain('((null as any) ? (((ctx).a))!.b : undefined)'); }); it('should use an \'any\' type for safe navigation operations when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, strictSafeNavigationTypes: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('(((ctx).a)!.method() as any)'); - expect(block).toContain('(((ctx).a)!.b as any)'); + expect(block).toContain('((((ctx).a))!.method() as any)'); + expect(block).toContain('((((ctx).a))!.b as any)'); }); }); @@ -580,14 +583,14 @@ describe('type check blocks', () => { const TEMPLATE = `{{a.method()?.b}} {{a()?.method()}}`; it('should check the presence of a property/method on the receiver when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('((null as any) ? (((ctx).a).method())!.b : undefined)'); + expect(block).toContain('((null as any) ? ((((ctx).a)).method())!.b : undefined)'); expect(block).toContain('((null as any) ? ((ctx).a())!.method() : undefined)'); }); it('should not check the presence of a property/method on the receiver when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, strictSafeNavigationTypes: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('((((ctx).a).method()) as any).b'); + expect(block).toContain('(((((ctx).a)).method()) as any).b'); expect(block).toContain('(((ctx).a()) as any).method()'); }); }); diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index ef40bab07e..e7bdb4effc 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -902,8 +902,7 @@ export declare class AnimationEvent { expect(diags.length).toBe(1); expect(diags[0].messageText) .toEqual(`Property 'does_not_exist' does not exist on type '{ name: string; }'.`); - expect(diags[0].start).toBe(199); - expect(diags[0].length).toBe(19); + expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist'); }); it('should accept an NgFor iteration over an any-typed value', () => { @@ -1126,8 +1125,7 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags.length).toBe(1); expect(diags[0].messageText).toEqual(`Property 'does_not_exist' does not exist on type 'T'.`); - expect(diags[0].start).toBe(206); - expect(diags[0].length).toBe(19); + expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist'); }); describe('microsyntax variables', () => { @@ -1752,7 +1750,7 @@ export declare class AnimationEvent { const diags = await driveDiagnostics(); expect(diags.length).toBe(1); expect(diags[0].file!.fileName).toBe(_('/test.ts')); - expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist'); + expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist'); }); it('should be correct for indirect templates', async () => { @@ -1774,7 +1772,7 @@ export declare class AnimationEvent { const diags = await driveDiagnostics(); expect(diags.length).toBe(1); expect(diags[0].file!.fileName).toBe(_('/test.ts') + ' (TestCmp template)'); - expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist'); + expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist'); expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])).toBe('TEMPLATE'); }); @@ -1797,7 +1795,7 @@ export declare class AnimationEvent { const diags = await driveDiagnostics(); expect(diags.length).toBe(1); expect(diags[0].file!.fileName).toBe(_('/template.html')); - expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist'); + expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist'); expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])) .toBe(`'./template.html'`); }); diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index dd7c741d80..97cb089c35 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -665,14 +665,14 @@ class _AstToIrVisitor implements cdAst.AstVisitor { this._nodeMap.set( leftMostSafe, new cdAst.MethodCall( - leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.receiver, leftMostSafe.name, - leftMostSafe.args)); + leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan, + leftMostSafe.receiver, leftMostSafe.name, leftMostSafe.args)); } else { this._nodeMap.set( leftMostSafe, new cdAst.PropertyRead( - leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.receiver, - leftMostSafe.name)); + leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan, + leftMostSafe.receiver, leftMostSafe.name)); } // Recursively convert the node now without the guarded member access. diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 756831b97b..d0dec2050d 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -39,6 +39,13 @@ export class AST { } } +export abstract class ASTWithName extends AST { + constructor( + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public nameSpan: AbsoluteSourceSpan) { + super(span, sourceSpan); + } +} + /** * Represents a quoted expression of the form: * @@ -101,31 +108,33 @@ export class Conditional extends AST { } } -export class PropertyRead extends AST { +export class PropertyRead extends ASTWithName { constructor( - span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string) { - super(span, sourceSpan); + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan, + public receiver: AST, public name: string) { + super(span, sourceSpan, nameSpan); } visit(visitor: AstVisitor, context: any = null): any { return visitor.visitPropertyRead(this, context); } } -export class PropertyWrite extends AST { +export class PropertyWrite extends ASTWithName { constructor( - span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string, - public value: AST) { - super(span, sourceSpan); + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan, + public receiver: AST, public name: string, public value: AST) { + super(span, sourceSpan, nameSpan); } visit(visitor: AstVisitor, context: any = null): any { return visitor.visitPropertyWrite(this, context); } } -export class SafePropertyRead extends AST { +export class SafePropertyRead extends ASTWithName { constructor( - span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string) { - super(span, sourceSpan); + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan, + public receiver: AST, public name: string) { + super(span, sourceSpan, nameSpan); } visit(visitor: AstVisitor, context: any = null): any { return visitor.visitSafePropertyRead(this, context); @@ -152,11 +161,11 @@ export class KeyedWrite extends AST { } } -export class BindingPipe extends AST { +export class BindingPipe extends ASTWithName { constructor( span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public exp: AST, public name: string, - public args: any[], public nameSpan: AbsoluteSourceSpan) { - super(span, sourceSpan); + public args: any[], nameSpan: AbsoluteSourceSpan) { + super(span, sourceSpan, nameSpan); } visit(visitor: AstVisitor, context: any = null): any { return visitor.visitPipe(this, context); @@ -236,22 +245,22 @@ export class NonNullAssert extends AST { } } -export class MethodCall extends AST { +export class MethodCall extends ASTWithName { constructor( - span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string, - public args: any[]) { - super(span, sourceSpan); + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan, + public receiver: AST, public name: string, public args: any[]) { + super(span, sourceSpan, nameSpan); } visit(visitor: AstVisitor, context: any = null): any { return visitor.visitMethodCall(this, context); } } -export class SafeMethodCall extends AST { +export class SafeMethodCall extends ASTWithName { constructor( - span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string, - public args: any[]) { - super(span, sourceSpan); + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan, + public receiver: AST, public name: string, public args: any[]) { + super(span, sourceSpan, nameSpan); } visit(visitor: AstVisitor, context: any = null): any { return visitor.visitSafeMethodCall(this, context); @@ -478,26 +487,31 @@ export class AstTransformer implements AstVisitor { } visitPropertyRead(ast: PropertyRead, context: any): AST { - return new PropertyRead(ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name); + return new PropertyRead( + ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name); } visitPropertyWrite(ast: PropertyWrite, context: any): AST { return new PropertyWrite( - ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name, ast.value.visit(this)); + ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name, + ast.value.visit(this)); } visitSafePropertyRead(ast: SafePropertyRead, context: any): AST { - return new SafePropertyRead(ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name); + return new SafePropertyRead( + ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name); } visitMethodCall(ast: MethodCall, context: any): AST { return new MethodCall( - ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name, this.visitAll(ast.args)); + ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name, + this.visitAll(ast.args)); } visitSafeMethodCall(ast: SafeMethodCall, context: any): AST { return new SafeMethodCall( - ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name, this.visitAll(ast.args)); + ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name, + this.visitAll(ast.args)); } visitFunctionCall(ast: FunctionCall, context: any): AST { @@ -586,7 +600,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor { visitPropertyRead(ast: PropertyRead, context: any): AST { const receiver = ast.receiver.visit(this); if (receiver !== ast.receiver) { - return new PropertyRead(ast.span, ast.sourceSpan, receiver, ast.name); + return new PropertyRead(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name); } return ast; } @@ -595,7 +609,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor { const receiver = ast.receiver.visit(this); const value = ast.value.visit(this); if (receiver !== ast.receiver || value !== ast.value) { - return new PropertyWrite(ast.span, ast.sourceSpan, receiver, ast.name, value); + return new PropertyWrite(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, value); } return ast; } @@ -603,7 +617,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor { visitSafePropertyRead(ast: SafePropertyRead, context: any): AST { const receiver = ast.receiver.visit(this); if (receiver !== ast.receiver) { - return new SafePropertyRead(ast.span, ast.sourceSpan, receiver, ast.name); + return new SafePropertyRead(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name); } return ast; } @@ -612,7 +626,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor { const receiver = ast.receiver.visit(this); const args = this.visitAll(ast.args); if (receiver !== ast.receiver || args !== ast.args) { - return new MethodCall(ast.span, ast.sourceSpan, receiver, ast.name, args); + return new MethodCall(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args); } return ast; } @@ -621,7 +635,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor { const receiver = ast.receiver.visit(this); const args = this.visitAll(ast.args); if (receiver !== ast.receiver || args !== ast.args) { - return new SafeMethodCall(ast.span, ast.sourceSpan, receiver, ast.name, args); + return new SafeMethodCall(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args); } return ast; } diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index d003edff14..38860de4dd 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -305,9 +305,34 @@ export class _ParseAST { return this.peek(0); } + /** Whether all the parser input has been processed. */ + get atEOF(): boolean { + return this.index >= this.tokens.length; + } + + /** + * Index of the next token to be processed, or the end of the last token if all have been + * processed. + */ get inputIndex(): number { - return (this.index < this.tokens.length) ? this.next.index + this.offset : - this.inputLength + this.offset; + return this.atEOF ? this.currentEndIndex : this.next.index + this.offset; + } + + /** + * End index of the last processed token, or the start of the first token if none have been + * processed. + */ + get currentEndIndex(): number { + if (this.index > 0) { + const curToken = this.peek(-1); + return curToken.end + this.offset; + } + // No tokens have been processed yet; return the next token's start or the length of the input + // if there is no token. + if (this.tokens.length === 0) { + return this.inputLength + this.offset; + } + return this.next.index + this.offset; } /** @@ -318,12 +343,7 @@ export class _ParseAST { } span(start: number) { - // `end` is either the - // - end index of the current token - // - start of the first token (this can happen e.g. when creating an implicit receiver) - const curToken = this.peek(-1); - const end = this.index > 0 ? curToken.end + this.offset : this.inputIndex; - return new ParseSpan(start, end); + return new ParseSpan(start, this.currentEndIndex); } sourceSpan(start: number): AbsoluteSourceSpan { @@ -730,7 +750,9 @@ export class _ParseAST { parseAccessMemberOrMethodCall(receiver: AST, isSafe: boolean = false): AST { const start = receiver.span.start; + const nameStart = this.inputIndex; const id = this.expectIdentifierOrKeyword(); + const nameSpan = this.sourceSpan(nameStart); if (this.consumeOptionalCharacter(chars.$LPAREN)) { this.rparensExpected++; @@ -739,8 +761,8 @@ export class _ParseAST { this.rparensExpected--; const span = this.span(start); const sourceSpan = this.sourceSpan(start); - return isSafe ? new SafeMethodCall(span, sourceSpan, receiver, id, args) : - new MethodCall(span, sourceSpan, receiver, id, args); + return isSafe ? new SafeMethodCall(span, sourceSpan, nameSpan, receiver, id, args) : + new MethodCall(span, sourceSpan, nameSpan, receiver, id, args); } else { if (isSafe) { @@ -748,7 +770,8 @@ export class _ParseAST { this.error('The \'?.\' operator cannot be used in the assignment'); return new EmptyExpr(this.span(start), this.sourceSpan(start)); } else { - return new SafePropertyRead(this.span(start), this.sourceSpan(start), receiver, id); + return new SafePropertyRead( + this.span(start), this.sourceSpan(start), nameSpan, receiver, id); } } else { if (this.consumeOptionalOperator('=')) { @@ -758,9 +781,10 @@ export class _ParseAST { } const value = this.parseConditional(); - return new PropertyWrite(this.span(start), this.sourceSpan(start), receiver, id, value); + return new PropertyWrite( + this.span(start), this.sourceSpan(start), nameSpan, receiver, id, value); } else { - return new PropertyRead(this.span(start), this.sourceSpan(start), receiver, id); + return new PropertyRead(this.span(start), this.sourceSpan(start), nameSpan, receiver, id); } } } diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 6d33831324..67e1b045e5 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -1428,7 +1428,7 @@ export class ValueConverter extends AstMemoryEfficientTransformer { // Allocate one slot for the result plus one slot per pipe argument const pureFunctionSlot = this.allocatePureFunctionSlots(2 + pipe.args.length); const target = new PropertyRead( - pipe.span, pipe.sourceSpan, new ImplicitReceiver(pipe.span, pipe.sourceSpan), + pipe.span, pipe.sourceSpan, pipe.nameSpan, new ImplicitReceiver(pipe.span, pipe.sourceSpan), slotPseudoLocal); const {identifier, isVarLength} = pipeBindingCallInfo(pipe.args); this.definePipe(pipe.name, slotPseudoLocal, slot, o.importExpr(identifier)); diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts index f25aab0e40..5f7d047c3b 100644 --- a/packages/compiler/test/expression_parser/parser_spec.ts +++ b/packages/compiler/test/expression_parser/parser_spec.ts @@ -12,7 +12,7 @@ import {Parser, SplitInterpolation} from '@angular/compiler/src/expression_parse import {expect} from '@angular/platform-browser/testing/src/matchers'; -import {unparse} from './utils/unparser'; +import {unparse, unparseWithSpan} from './utils/unparser'; import {validate} from './utils/validator'; describe('parser', () => { @@ -198,6 +198,56 @@ describe('parser', () => { }); }); + describe('parse spans', () => { + it('should record property read span', () => { + const ast = parseAction('foo'); + expect(unparseWithSpan(ast)).toContain(['foo', 'foo']); + expect(unparseWithSpan(ast)).toContain(['foo', '[nameSpan] foo']); + }); + + it('should record accessed property read span', () => { + const ast = parseAction('foo.bar'); + expect(unparseWithSpan(ast)).toContain(['foo.bar', 'foo.bar']); + expect(unparseWithSpan(ast)).toContain(['foo.bar', '[nameSpan] bar']); + }); + + it('should record safe property read span', () => { + const ast = parseAction('foo?.bar'); + expect(unparseWithSpan(ast)).toContain(['foo?.bar', 'foo?.bar']); + expect(unparseWithSpan(ast)).toContain(['foo?.bar', '[nameSpan] bar']); + }); + + it('should record method call span', () => { + const ast = parseAction('foo()'); + expect(unparseWithSpan(ast)).toContain(['foo()', 'foo()']); + expect(unparseWithSpan(ast)).toContain(['foo()', '[nameSpan] foo']); + }); + + it('should record accessed method call span', () => { + const ast = parseAction('foo.bar()'); + expect(unparseWithSpan(ast)).toContain(['foo.bar()', 'foo.bar()']); + expect(unparseWithSpan(ast)).toContain(['foo.bar()', '[nameSpan] bar']); + }); + + it('should record safe method call span', () => { + const ast = parseAction('foo?.bar()'); + expect(unparseWithSpan(ast)).toContain(['foo?.bar()', 'foo?.bar()']); + expect(unparseWithSpan(ast)).toContain(['foo?.bar()', '[nameSpan] bar']); + }); + + it('should record property write span', () => { + const ast = parseAction('a = b'); + expect(unparseWithSpan(ast)).toContain(['a = b', 'a = b']); + expect(unparseWithSpan(ast)).toContain(['a = b', '[nameSpan] a']); + }); + + it('should record accessed property write span', () => { + const ast = parseAction('a.b = c'); + expect(unparseWithSpan(ast)).toContain(['a.b = c', 'a.b = c']); + expect(unparseWithSpan(ast)).toContain(['a.b = c', '[nameSpan] b']); + }); + }); + describe('general error handling', () => { it('should report an unexpected token', () => { expectActionError('[1,2] trac', 'Unexpected token \'trac\''); @@ -589,7 +639,7 @@ describe('parser', () => { ['of: [1,2,3] | pipe ', 'of', '[1,2,3] | pipe'], ['of: [1,2,3] | pipe as items; ', 'items', 'of'], ['let i=index, ', 'i', 'index'], - ['count as len, ', 'len', 'count'], + ['count as len,', 'len', 'count'], ]); }); }); diff --git a/packages/compiler/test/expression_parser/utils/unparser.ts b/packages/compiler/test/expression_parser/utils/unparser.ts index fd2574e388..78592a45e6 100644 --- a/packages/compiler/test/expression_parser/utils/unparser.ts +++ b/packages/compiler/test/expression_parser/utils/unparser.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstVisitor, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '../../../src/expression_parser/ast'; +import {AbsoluteSourceSpan, AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead} from '../../../src/expression_parser/ast'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/interpolation_config'; class Unparser implements AstVisitor { @@ -197,3 +197,34 @@ export function unparse( ast: AST, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): string { return sharedUnparser.unparse(ast, interpolationConfig); } + +// [unparsed AST, original source code of AST] +type UnparsedWithSpan = [string, string]; + +export function unparseWithSpan( + ast: ASTWithSource, + interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): UnparsedWithSpan[] { + const unparsed: UnparsedWithSpan[] = []; + const source = ast.source!; + const recursiveSpanUnparser = new class extends RecursiveAstVisitor { + private recordUnparsed(ast: any, spanKey: string, unparsedList: UnparsedWithSpan[]) { + const span = ast[spanKey]; + const prefix = spanKey === 'span' ? '' : `[${spanKey}] `; + const src = source.substring(span.start, span.end); + unparsedList.push([ + unparse(ast, interpolationConfig), + prefix + src, + ]); + } + + visit(ast: AST, unparsedList: UnparsedWithSpan[]) { + this.recordUnparsed(ast, 'span', unparsedList); + if (ast.hasOwnProperty('nameSpan')) { + this.recordUnparsed(ast, 'nameSpan', unparsedList); + } + ast.visit(this, unparsedList); + } + }; + recursiveSpanUnparser.visitAll([ast.ast], unparsed); + return unparsed; +} diff --git a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts index e9c2a7a248..3330d4c5af 100644 --- a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts +++ b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts @@ -245,10 +245,18 @@ describe('expression AST absolute source spans', () => { }); }); - it('should provide absolute offsets of a property read', () => { - expect(humanizeExpressionSource(parse('
{{prop}}
').nodes)).toContain([ - 'prop', new AbsoluteSourceSpan(7, 11) - ]); + describe('property read', () => { + it('should provide absolute offsets of a property read', () => { + expect(humanizeExpressionSource(parse('
{{prop.obj}}
').nodes)).toContain([ + 'prop.obj', new AbsoluteSourceSpan(7, 15) + ]); + }); + + it('should provide absolute offsets of expressions in a property read', () => { + expect(humanizeExpressionSource(parse('
{{prop.obj}}
').nodes)).toContain([ + 'prop', new AbsoluteSourceSpan(7, 11) + ]); + }); }); describe('property write', () => { @@ -258,6 +266,11 @@ describe('expression AST absolute source spans', () => { ]); }); + it('should provide absolute offsets of an accessed property write', () => { + expect(humanizeExpressionSource(parse('
').nodes)) + .toContain(['prop.inner = 0', new AbsoluteSourceSpan(14, 28)]); + }); + it('should provide absolute offsets of expressions in a property write', () => { expect(humanizeExpressionSource(parse('
').nodes)).toContain([ '0', new AbsoluteSourceSpan(21, 22) diff --git a/packages/compiler/test/template_parser/template_parser_absolute_span_spec.ts b/packages/compiler/test/template_parser/template_parser_absolute_span_spec.ts index 7547490e5b..95478e7b8b 100644 --- a/packages/compiler/test/template_parser/template_parser_absolute_span_spec.ts +++ b/packages/compiler/test/template_parser/template_parser_absolute_span_spec.ts @@ -283,10 +283,18 @@ describe('expression AST absolute source spans', () => { }); }); - it('should provide absolute offsets of a property read', () => { - expect(humanizeExpressionSource(parse('
{{prop}}
'))).toContain([ - 'prop', new AbsoluteSourceSpan(7, 11) - ]); + describe('property read', () => { + it('should provide absolute offsets of a property read', () => { + expect(humanizeExpressionSource(parse('
{{prop.obj}}
'))).toContain([ + 'prop.obj', new AbsoluteSourceSpan(7, 15) + ]); + }); + + it('should provide absolute offsets of expressions in a property read', () => { + expect(humanizeExpressionSource(parse('
{{prop.obj}}
'))).toContain([ + 'prop', new AbsoluteSourceSpan(7, 11) + ]); + }); }); describe('property write', () => { @@ -296,6 +304,12 @@ describe('expression AST absolute source spans', () => { ]); }); + it('should provide absolute offsets of an accessed property write', () => { + expect(humanizeExpressionSource(parse('
'))).toContain([ + 'prop.inner = 0', new AbsoluteSourceSpan(14, 28) + ]); + }); + it('should provide absolute offsets of expressions in a property write', () => { expect(humanizeExpressionSource(parse('
'))).toContain([ '0', new AbsoluteSourceSpan(21, 22) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index a637eba1da..a354ed3ae7 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -457,8 +457,13 @@ class ExpressionVisitor extends NullTemplateVisitor { const absValueOffset = ast.sourceSpan.start.offset; const {templateBindings} = this.info.expressionParser.parseTemplateBindings( templateKey, templateValue, templateUrl, absKeyOffset, absValueOffset); - // Find the template binding that contains the position. - const templateBinding = templateBindings.find(b => inSpan(this.position, b.sourceSpan)); + // Find the nearest template binding to the position. + const lastBindingEnd = templateBindings.length > 0 && + templateBindings[templateBindings.length - 1].sourceSpan.end; + const normalizedPositionToBinding = + lastBindingEnd && this.position > lastBindingEnd ? lastBindingEnd : this.position; + const templateBinding = + templateBindings.find(b => inSpan(normalizedPositionToBinding, b.sourceSpan)); if (!templateBinding) { return; diff --git a/packages/language-service/src/expression_type.ts b/packages/language-service/src/expression_type.ts index 732fe97910..73912a85f0 100644 --- a/packages/language-service/src/expression_type.ts +++ b/packages/language-service/src/expression_type.ts @@ -6,11 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstVisitor, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '@angular/compiler'; +import {AST, AstVisitor, ASTWithName, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '@angular/compiler'; import {createDiagnostic, Diagnostic} from './diagnostic_messages'; import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols'; import * as ng from './types'; +import {offsetSpan} from './utils'; interface ExpressionDiagnosticsContext { inEvent?: boolean; @@ -32,7 +33,7 @@ export class AstType implements AstVisitor { const type: Symbol = ast.visit(this); if (this.context.inEvent && type.callable) { this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.callable_expression_expected_method_call)); + createDiagnostic(refinedSpan(ast), Diagnostic.callable_expression_expected_method_call)); } return this.diagnostics; } @@ -51,7 +52,8 @@ export class AstType implements AstVisitor { // Nullable allowed. break; default: - this.diagnostics.push(createDiagnostic(ast.span, Diagnostic.expression_might_be_null)); + this.diagnostics.push( + createDiagnostic(refinedSpan(ast), Diagnostic.expression_might_be_null)); break; } } @@ -130,7 +132,7 @@ export class AstType implements AstVisitor { return this.anyType; default: this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.expected_a_string_or_number_type)); + createDiagnostic(refinedSpan(ast), Diagnostic.expected_a_string_or_number_type)); return this.anyType; } case '>': @@ -146,8 +148,8 @@ export class AstType implements AstVisitor { // Two values are comparable only if // - they have some type overlap, or // - at least one is not defined - this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.expected_operands_of_comparable_types_or_any)); + this.diagnostics.push(createDiagnostic( + refinedSpan(ast), Diagnostic.expected_operands_of_comparable_types_or_any)); } return this.query.getBuiltinType(BuiltinType.Boolean); case '&&': @@ -157,7 +159,7 @@ export class AstType implements AstVisitor { } this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.unrecognized_operator, ast.operation)); + createDiagnostic(refinedSpan(ast), Diagnostic.unrecognized_operator, ast.operation)); return this.anyType; } @@ -187,7 +189,8 @@ export class AstType implements AstVisitor { const target = this.getType(ast.target!); if (!target || !target.callable) { this.diagnostics.push(createDiagnostic( - ast.span, Diagnostic.call_target_not_callable, this.sourceOf(ast.target!), target.name)); + refinedSpan(ast), Diagnostic.call_target_not_callable, this.sourceOf(ast.target!), + target.name)); return this.anyType; } const signature = target.selectSignature(args); @@ -197,7 +200,7 @@ export class AstType implements AstVisitor { // TODO: Consider a better error message here. See `typescript_symbols#selectSignature` for more // details. this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.unable_to_resolve_compatible_call_signature)); + createDiagnostic(refinedSpan(ast), Diagnostic.unable_to_resolve_compatible_call_signature)); return this.anyType; } @@ -291,8 +294,8 @@ export class AstType implements AstVisitor { case 'number': return this.query.getBuiltinType(BuiltinType.Number); default: - this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.unrecognized_primitive, typeof ast.value)); + this.diagnostics.push(createDiagnostic( + refinedSpan(ast), Diagnostic.unrecognized_primitive, typeof ast.value)); return this.anyType; } } @@ -307,7 +310,7 @@ export class AstType implements AstVisitor { // by getPipes() is expected to contain symbols with the corresponding transform method type. const pipe = this.query.getPipes().get(ast.name); if (!pipe) { - this.diagnostics.push(createDiagnostic(ast.span, Diagnostic.no_pipe_found, ast.name)); + this.diagnostics.push(createDiagnostic(refinedSpan(ast), Diagnostic.no_pipe_found, ast.name)); return this.anyType; } const expType = this.getType(ast.exp); @@ -315,7 +318,7 @@ export class AstType implements AstVisitor { pipe.selectSignature([expType].concat(ast.args.map(arg => this.getType(arg)))); if (!signature) { this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.unable_to_resolve_signature, ast.name)); + createDiagnostic(refinedSpan(ast), Diagnostic.unable_to_resolve_signature, ast.name)); return this.anyType; } return signature.result; @@ -389,7 +392,7 @@ export class AstType implements AstVisitor { const methodType = this.resolvePropertyRead(receiverType, ast); if (!methodType) { this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.could_not_resolve_type, ast.name)); + createDiagnostic(refinedSpan(ast), Diagnostic.could_not_resolve_type, ast.name)); return this.anyType; } if (this.isAny(methodType)) { @@ -397,13 +400,13 @@ export class AstType implements AstVisitor { } if (!methodType.callable) { this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.identifier_not_callable, ast.name)); + createDiagnostic(refinedSpan(ast), Diagnostic.identifier_not_callable, ast.name)); return this.anyType; } const signature = methodType.selectSignature(ast.args.map(arg => this.getType(arg))); if (!signature) { this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.unable_to_resolve_signature, ast.name)); + createDiagnostic(refinedSpan(ast), Diagnostic.unable_to_resolve_signature, ast.name)); return this.anyType; } return signature.result; @@ -417,24 +420,25 @@ export class AstType implements AstVisitor { const member = receiverType.members().get(ast.name); if (!member) { if (receiverType.name === '$implicit') { - this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.identifier_not_defined_in_app_context, ast.name)); + this.diagnostics.push(createDiagnostic( + refinedSpan(ast), Diagnostic.identifier_not_defined_in_app_context, ast.name)); } else if (receiverType.nullable && ast.receiver instanceof PropertyRead) { const receiver = ast.receiver.name; this.diagnostics.push(createDiagnostic( - ast.span, Diagnostic.identifier_possibly_undefined, receiver, + refinedSpan(ast), Diagnostic.identifier_possibly_undefined, receiver, `${receiver}?.${ast.name}`, `${receiver}!.${ast.name}`)); } else { this.diagnostics.push(createDiagnostic( - ast.span, Diagnostic.identifier_not_defined_on_receiver, ast.name, receiverType.name)); + refinedSpan(ast), Diagnostic.identifier_not_defined_on_receiver, ast.name, + receiverType.name)); } return this.anyType; } if (!member.public) { const container = receiverType.name === '$implicit' ? 'the component' : `'${receiverType.name}'`; - this.diagnostics.push( - createDiagnostic(ast.span, Diagnostic.identifier_is_private, ast.name, container)); + this.diagnostics.push(createDiagnostic( + refinedSpan(ast), Diagnostic.identifier_is_private, ast.name, container)); } return member.type; } @@ -444,3 +448,14 @@ export class AstType implements AstVisitor { (!!symbol.type && this.isAny(symbol.type)); } } + +function refinedSpan(ast: AST): ng.Span { + // nameSpan is an absolute span, but the spans returned by the expression visitor are expected to + // be relative to the start of the expression. + // TODO: migrate to only using absolute spans + const absoluteOffset = ast.sourceSpan.start - ast.span.start; + if (ast instanceof ASTWithName) { + return offsetSpan(ast.nameSpan, -absoluteOffset); + } + return offsetSpan(ast.sourceSpan, -absoluteOffset); +} diff --git a/packages/language-service/src/expressions.ts b/packages/language-service/src/expressions.ts index 1d27b209ab..40f9024a6f 100644 --- a/packages/language-service/src/expressions.ts +++ b/packages/language-service/src/expressions.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstPath as AstPathBase, ASTWithSource, RecursiveAstVisitor} from '@angular/compiler'; +import {AST, AstPath as AstPathBase, ASTWithName, ASTWithSource, RecursiveAstVisitor} from '@angular/compiler'; import {AstType} from './expression_type'; import {BuiltinType, Span, Symbol, SymbolTable, TemplateSource} from './types'; @@ -120,6 +120,17 @@ export function getExpressionSymbol( return new AstType(scope, templateInfo.query, {}, templateInfo.source).getType(ast); } + function spanFromName(ast: ASTWithName): Span { + // `nameSpan` is an absolute span, but the span expected by the result of this method is + // relative to the start of the expression. + // TODO(ayazhafiz): migrate to only using absolute spans + const offset = ast.sourceSpan.start - ast.span.start; + return { + start: ast.nameSpan.start - offset, + end: ast.nameSpan.end - offset, + }; + } + let symbol: Symbol|undefined = undefined; let span: Span|undefined = undefined; @@ -141,22 +152,14 @@ export function getExpressionSymbol( visitMethodCall(ast) { const receiverType = getType(ast.receiver); symbol = receiverType && receiverType.members().get(ast.name); - span = ast.span; + span = spanFromName(ast); }, visitPipe(ast) { if (inSpan(position, ast.nameSpan, /* exclusive */ true)) { // We are in a position a pipe name is expected. const pipes = templateInfo.query.getPipes(); symbol = pipes.get(ast.name); - - // `nameSpan` is an absolute span, but the span expected by the result of this method is - // relative to the start of the expression. - // TODO(ayazhafiz): migrate to only using absolute spans - const offset = ast.sourceSpan.start - ast.span.start; - span = { - start: ast.nameSpan.start - offset, - end: ast.nameSpan.end - offset, - }; + span = spanFromName(ast); } }, visitPrefixNot(_ast) {}, @@ -164,29 +167,23 @@ export function getExpressionSymbol( visitPropertyRead(ast) { const receiverType = getType(ast.receiver); symbol = receiverType && receiverType.members().get(ast.name); - span = ast.span; + span = spanFromName(ast); }, visitPropertyWrite(ast) { const receiverType = getType(ast.receiver); - const {start} = ast.span; symbol = receiverType && receiverType.members().get(ast.name); - // A PropertyWrite span includes both the LHS (name) and the RHS (value) of the write. In this - // visit, only the name is relevant. - // prop=$event - // ^^^^ name - // ^^^^^^ value; visited separately as a nested AST - span = {start, end: start + ast.name.length}; + span = spanFromName(ast); }, visitQuote(_ast) {}, visitSafeMethodCall(ast) { const receiverType = getType(ast.receiver); symbol = receiverType && receiverType.members().get(ast.name); - span = ast.span; + span = spanFromName(ast); }, visitSafePropertyRead(ast) { const receiverType = getType(ast.receiver); symbol = receiverType && receiverType.members().get(ast.name); - span = ast.span; + span = spanFromName(ast); }, }); diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index dcf5f25bea..31baaddb37 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -77,7 +77,7 @@ describe('definitions', () => { it('should be able to find a method from a call', () => { const fileName = mockHost.addCode(` @Component({ - template: '
' + template: '
' }) export class MyComponent { «ᐱmyClickᐱ() { }» @@ -88,7 +88,7 @@ describe('definitions', () => { expect(result).toBeDefined(); const {textSpan, definitions} = result!; - expect(textSpan).toEqual(mockHost.getLocationMarkerFor(fileName, 'my')); + expect(textSpan).toEqual(marker); expect(definitions).toBeDefined(); expect(definitions!.length).toBe(1); const def = definitions![0]; diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index 22a1e937c7..9a9054c1e0 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -182,7 +182,7 @@ describe('diagnostics', () => { mockHost.override(TEST_TEMPLATE, `
'titleProxy' is a string - {{~{start-err}titleProxy.notAProperty~{end-err}}} + {{titleProxy.~{start-err}notAProperty~{end-err}}}
`); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); @@ -200,7 +200,7 @@ describe('diagnostics', () => { mockHost.override(TEST_TEMPLATE, `
'titleProxy' is a string - {{~{start-err}titleProxy.notAProperty~{end-err}}} + {{titleProxy.~{start-err}notAProperty~{end-err}}}
`); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); @@ -364,7 +364,7 @@ describe('diagnostics', () => { it('report an unknown field in $implicit context', () => { mockHost.override(TEST_TEMPLATE, `
- {{ ~{start-emb}myVar.missingField~{end-emb} }} + {{ myVar.~{start-emb}missingField~{end-emb} }}
`); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); @@ -383,7 +383,7 @@ describe('diagnostics', () => { it('report an unknown field in non implicit context', () => { mockHost.override(TEST_TEMPLATE, `
- {{ ~{start-emb}myVar.missingField~{end-emb} }} + {{ myVar.~{start-emb}missingField~{end-emb} }}
`); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); @@ -421,8 +421,7 @@ describe('diagnostics', () => { const {messageText, start, length} = diagnostics[0]; expect(messageText) .toBe(`Identifier 'xyz' is not defined. 'Hero' does not contain such a member`); - expect(start).toBe(content.indexOf('member.xyz')); - expect(length).toBe('member.xyz'.length); + expect(content.substring(start!, start! + length!)).toBe('xyz'); }); describe('with $event', () => { @@ -454,8 +453,7 @@ describe('diagnostics', () => { expect(messageText) .toBe( `Identifier 'notSubstring' is not defined. 'string' does not contain such a member`); - expect(start).toBe(content.indexOf('$event')); - expect(length).toBe('$event.notSubstring()'.length); + expect(content.substring(start!, start! + length!)).toBe('notSubstring'); }); }); @@ -990,7 +988,7 @@ describe('diagnostics', () => { `Consider using the safe navigation operator (optional?.toLowerCase) ` + `or non-null assertion operator (optional!.toLowerCase).`); expect(category).toBe(ts.DiagnosticCategory.Suggestion); - expect(content.substring(start!, start! + length!)).toBe('optional.toLowerCase()'); + expect(content.substring(start!, start! + length!)).toBe('toLowerCase'); }); it('should suggest ? or ! operator if property receiver is nullable', () => { @@ -1004,40 +1002,40 @@ describe('diagnostics', () => { `Consider using the safe navigation operator (optional?.length) ` + `or non-null assertion operator (optional!.length).`); expect(category).toBe(ts.DiagnosticCategory.Suggestion); - expect(content.substring(start!, start! + length!)).toBe('optional.length'); + expect(content.substring(start!, start! + length!)).toBe('length'); }); - it('should report error if method is not found on non-nullable receiver', () => { + it('should report error if method is not found on non-nullable receivers', () => { const expressions = [ - 'optional?.someMethod()', - 'optional!.someMethod()', + 'optional?', + 'optional!', ]; for (const expression of expressions) { - const content = mockHost.override(TEST_TEMPLATE, `{{${expression}}}`); + const content = mockHost.override(TEST_TEMPLATE, `{{ ${expression}.someMethod() }}`); const ngDiags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); expect(ngDiags.length).toBe(1); const {start, length, messageText, category} = ngDiags[0]; expect(messageText) .toBe(`Identifier 'someMethod' is not defined. 'string' does not contain such a member`); expect(category).toBe(ts.DiagnosticCategory.Error); - expect(content.substring(start!, start! + length!)).toBe(expression); + expect(content.substring(start!, start! + length!)).toBe('someMethod'); } }); - it('should report error if property is not found on non-nullable receiver', () => { + it('should report error if property is not found on non-nullable receivers', () => { const expressions = [ - 'optional?.someProp', - 'optional!.someProp', + 'optional?', + 'optional!', ]; for (const expression of expressions) { - const content = mockHost.override(TEST_TEMPLATE, `{{${expression}}}`); + const content = mockHost.override(TEST_TEMPLATE, `{{ ${expression}.someProp }}`); const ngDiags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); expect(ngDiags.length).toBe(1); const {start, length, messageText, category} = ngDiags[0]; expect(messageText) .toBe(`Identifier 'someProp' is not defined. 'string' does not contain such a member`); expect(category).toBe(ts.DiagnosticCategory.Error); - expect(content.substring(start!, start! + length!)).toBe(expression); + expect(content.substring(start!, start! + length!)).toBe('someProp'); } }); diff --git a/packages/language-service/test/hover_spec.ts b/packages/language-service/test/hover_spec.ts index 425d9078f4..b6d3f8578e 100644 --- a/packages/language-service/test/hover_spec.ts +++ b/packages/language-service/test/hover_spec.ts @@ -109,6 +109,36 @@ describe('hover', () => { expect(toText(displayParts)).toBe('(property) TemplateReference.title: string'); }); + it('should work for accessed property reads', () => { + mockHost.override(TEST_TEMPLATE, `
{{title.«length»}}
`); + const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'length'); + const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start); + expect(quickInfo).toBeTruthy(); + const {textSpan, displayParts} = quickInfo!; + expect(textSpan).toEqual(marker); + expect(toText(displayParts)).toBe('(property) String.length: number'); + }); + + it('should work for properties in writes', () => { + mockHost.override(TEST_TEMPLATE, `
`); + const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'title'); + const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start); + expect(quickInfo).toBeTruthy(); + const {textSpan, displayParts} = quickInfo!; + expect(textSpan).toEqual(marker); + expect(toText(displayParts)).toBe('(property) TemplateReference.title: string'); + }); + + it('should work for accessed properties in writes', () => { + mockHost.override(TEST_TEMPLATE, `
`); + const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'id'); + const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start); + expect(quickInfo).toBeTruthy(); + const {textSpan, displayParts} = quickInfo!; + expect(textSpan).toEqual(marker); + expect(toText(displayParts)).toBe('(property) Hero.id: number'); + }); + it('should work for array members', () => { mockHost.override(TEST_TEMPLATE, `
{{«hero»}}
`); const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'hero'); @@ -142,8 +172,8 @@ describe('hover', () => { }); it('should work for method calls', () => { - mockHost.override(TEST_TEMPLATE, `
`); - const marker = mockHost.getDefinitionMarkerFor(TEST_TEMPLATE, 'myClick'); + mockHost.override(TEST_TEMPLATE, `
`); + const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'myClick'); const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start); expect(quickInfo).toBeTruthy(); const {textSpan, displayParts} = quickInfo!; @@ -192,7 +222,7 @@ describe('hover', () => { const {textSpan, displayParts} = quickInfo!; expect(textSpan).toEqual({ start: position, - length: '$any(title)'.length, + length: '$any'.length, }); expect(toText(displayParts)).toBe('(method) $any: $any'); });