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
This commit is contained in:
Ayaz Hafiz 2020-04-27 18:54:30 -07:00 committed by Misko Hevery
parent b37071c22f
commit ba1dfd0cf3
20 changed files with 496 additions and 199 deletions

View File

@ -10,7 +10,7 @@ import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional,
import * as ts from 'typescript'; import * as ts from 'typescript';
import {TypeCheckingConfig} from './api'; import {TypeCheckingConfig} from './api';
import {addParseSpanInfo, ignoreDiagnostics, wrapForDiagnostics} from './diagnostics'; import {addParseSpanInfo, wrapForDiagnostics} from './diagnostics';
import {tsCastToAny} from './ts_util'; import {tsCastToAny} from './ts_util';
export const NULL_AS_ANY = export const NULL_AS_ANY =
@ -179,6 +179,7 @@ class AstTranslator implements AstVisitor {
visitMethodCall(ast: MethodCall): ts.Expression { visitMethodCall(ast: MethodCall): ts.Expression {
const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const receiver = wrapForDiagnostics(this.translate(ast.receiver));
const method = ts.createPropertyAccess(receiver, ast.name); const method = ts.createPropertyAccess(receiver, ast.name);
addParseSpanInfo(method, ast.nameSpan);
const args = ast.args.map(expr => this.translate(expr)); const args = ast.args.map(expr => this.translate(expr));
const node = ts.createCall(method, undefined, args); const node = ts.createCall(method, undefined, args);
addParseSpanInfo(node, ast.sourceSpan); 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 // This is a normal property read - convert the receiver to an expression and emit the correct
// TypeScript expression to read the property. // TypeScript expression to read the property.
const receiver = wrapForDiagnostics(this.translate(ast.receiver)); 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); addParseSpanInfo(node, ast.sourceSpan);
return node; return node;
} }
@ -215,10 +218,18 @@ class AstTranslator implements AstVisitor {
visitPropertyWrite(ast: PropertyWrite): ts.Expression { visitPropertyWrite(ast: PropertyWrite): ts.Expression {
const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const receiver = wrapForDiagnostics(this.translate(ast.receiver));
const left = ts.createPropertyAccess(receiver, ast.name); const left = ts.createPropertyAccess(receiver, ast.name);
// TODO(joost): annotate `left` with the span of the property access, which is not currently addParseSpanInfo(left, ast.nameSpan);
// available on `ast`. // 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 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); addParseSpanInfo(node, ast.sourceSpan);
return node; return node;
} }
@ -235,15 +246,18 @@ class AstTranslator implements AstVisitor {
if (this.config.strictSafeNavigationTypes) { if (this.config.strictSafeNavigationTypes) {
// "a?.method(...)" becomes (null as any ? a!.method(...) : undefined) // "a?.method(...)" becomes (null as any ? a!.method(...) : undefined)
const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name); const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name);
addParseSpanInfo(method, ast.nameSpan);
const call = ts.createCall(method, undefined, args); const call = ts.createCall(method, undefined, args);
node = ts.createParen(ts.createConditional(NULL_AS_ANY, call, UNDEFINED)); node = ts.createParen(ts.createConditional(NULL_AS_ANY, call, UNDEFINED));
} else if (VeSafeLhsInferenceBugDetector.veWillInferAnyFor(ast)) { } else if (VeSafeLhsInferenceBugDetector.veWillInferAnyFor(ast)) {
// "a?.method(...)" becomes (a as any).method(...) // "a?.method(...)" becomes (a as any).method(...)
const method = ts.createPropertyAccess(tsCastToAny(receiver), ast.name); const method = ts.createPropertyAccess(tsCastToAny(receiver), ast.name);
addParseSpanInfo(method, ast.nameSpan);
node = ts.createCall(method, undefined, args); node = ts.createCall(method, undefined, args);
} else { } else {
// "a?.method(...)" becomes (a!.method(...) as any) // "a?.method(...)" becomes (a!.method(...) as any)
const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name); const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name);
addParseSpanInfo(method, ast.nameSpan);
node = tsCastToAny(ts.createCall(method, undefined, args)); node = tsCastToAny(ts.createCall(method, undefined, args));
} }
addParseSpanInfo(node, ast.sourceSpan); addParseSpanInfo(node, ast.sourceSpan);

View File

@ -1125,6 +1125,7 @@ class TcbExpressionTranslator {
return result; return result;
} else if (ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver) { } 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`. // 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) { if (ast.name === '$any' && ast.args.length === 1) {
const expr = this.translate(ast.args[0]); const expr = this.translate(ast.args[0]);
const exprAsAny = const exprAsAny =
@ -1144,6 +1145,7 @@ class TcbExpressionTranslator {
} }
const method = ts.createPropertyAccess(wrapForDiagnostics(receiver), ast.name); const method = ts.createPropertyAccess(wrapForDiagnostics(receiver), ast.name);
addParseSpanInfo(method, ast.nameSpan);
const args = ast.args.map(arg => this.translate(arg)); const args = ast.args.map(arg => this.translate(arg));
const node = ts.createCall(method, undefined, args); const node = ts.createCall(method, undefined, args);
addParseSpanInfo(node, ast.sourceSpan); addParseSpanInfo(node, ast.sourceSpan);
@ -1435,7 +1437,7 @@ class TcbEventHandlerTranslator extends TcbExpressionTranslator {
if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver && if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver &&
ast.name === EVENT_PARAMETER) { ast.name === EVENT_PARAMETER) {
const event = ts.createIdentifier(EVENT_PARAMETER); const event = ts.createIdentifier(EVENT_PARAMETER);
addParseSpanInfo(event, ast.sourceSpan); addParseSpanInfo(event, ast.nameSpan);
return event; return event;
} }

View File

@ -130,7 +130,7 @@ runInEachFileSystem(() => {
[ngForDeclaration()], [ngForDts()]); [ngForDeclaration()], [ngForDts()]);
expect(messages).toEqual([ 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', () => { 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( function diagnose(

View File

@ -12,17 +12,18 @@ describe('type check blocks diagnostics', () => {
describe('parse spans', () => { describe('parse spans', () => {
it('should annotate binary ops', () => { it('should annotate binary ops', () => {
expect(tcbWithSpans('{{ a + b }}')) 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', () => { it('should annotate conditions', () => {
expect(tcbWithSpans('{{ a ? b : c }}')) 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', () => { it('should annotate interpolations', () => {
expect(tcbWithSpans('{{ hello }} {{ world }}')) 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', () => { 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. // statement, which would wrap it into parenthesis that clutter the expected output.
const TEMPLATE = '{{ m({foo: a, bar: b}) }}'; const TEMPLATE = '{{ m({foo: a, bar: b}) }}';
expect(tcbWithSpans(TEMPLATE)) 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', () => { it('should annotate literal array expressions', () => {
const TEMPLATE = '{{ [a, b] }}'; 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', () => { it('should annotate literals', () => {
@ -45,77 +48,84 @@ describe('type check blocks diagnostics', () => {
it('should annotate non-null assertions', () => { it('should annotate non-null assertions', () => {
const TEMPLATE = `{{ a! }}`; 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', () => { it('should annotate prefix not', () => {
const TEMPLATE = `{{ !a }}`; 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', () => { it('should annotate method calls', () => {
const TEMPLATE = `{{ method(a, b) }}`; const TEMPLATE = `{{ method(a, b) }}`;
expect(tcbWithSpans(TEMPLATE)) 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', () => { it('should annotate method calls of variables', () => {
const TEMPLATE = `<ng-template let-method>{{ method(a, b) }}</ng-template>`; const TEMPLATE = `<ng-template let-method>{{ method(a, b) }}</ng-template>`;
expect(tcbWithSpans(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', () => { it('should annotate function calls', () => {
const TEMPLATE = `{{ method(a)(b, c) }}`; const TEMPLATE = `{{ method(a)(b, c) }}`;
expect(tcbWithSpans(TEMPLATE)) expect(tcbWithSpans(TEMPLATE))
.toContain( .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', () => { it('should annotate property access', () => {
const TEMPLATE = `{{ a.b.c }}`; 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', () => { 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)) 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', () => { it('should annotate keyed property access', () => {
const TEMPLATE = `{{ a[b] }}`; 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', () => { it('should annotate keyed property writes', () => {
const TEMPLATE = `<div (click)="a[b] = c"></div>`; const TEMPLATE = `<div (click)="a[b] = c"></div>`;
expect(tcbWithSpans(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', () => { it('should annotate safe property access', () => {
const TEMPLATE = `{{ a?.b }}`; const TEMPLATE = `{{ a?.b }}`;
expect(tcbWithSpans(TEMPLATE)) 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', () => { it('should annotate safe method calls', () => {
const TEMPLATE = `{{ a?.method(b) }}`; const TEMPLATE = `{{ a?.method(b) }}`;
expect(tcbWithSpans(TEMPLATE)) expect(tcbWithSpans(TEMPLATE))
.toContain( .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', () => { it('should annotate $any casts', () => {
const TEMPLATE = `{{ $any(a) }}`; 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', () => { it('should annotate chained expressions', () => {
const TEMPLATE = `<div (click)="a; b; c"></div>`; const TEMPLATE = `<div (click)='a; b; c'></div>`;
expect(tcbWithSpans(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', () => { it('should annotate pipe usages', () => {
@ -127,7 +137,7 @@ describe('type check blocks diagnostics', () => {
}]; }];
const block = tcbWithSpans(TEMPLATE, PIPES); const block = tcbWithSpans(TEMPLATE, PIPES);
expect(block).toContain( 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', () => { 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*/);'); expect(tcbWithSpans(TEMPLATE)).toContain('((_t1 /*19,20*/) || (_t1 /*24,25*/) /*19,25*/);');
}); });
it('should be correct for template vars', () => { 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*/);'); expect(tcbWithSpans(TEMPLATE)).toContain('((_t2 /*26,27*/) || (_t2 /*31,32*/) /*26,32*/);');
}); });
it('should be correct for directive refs', () => { it('should be correct for directive refs', () => {

View File

@ -13,32 +13,33 @@ import {ALL_ENABLED_CONFIG, tcb, TestDeclaration, TestDirective} from './test_ut
describe('type check blocks', () => { describe('type check blocks', () => {
it('should generate a basic block for a binding', () => { 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', () => { it('should generate literal map expressions', () => {
const TEMPLATE = '{{ method({foo: a, bar: b}) }}'; 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', () => { it('should generate literal array expressions', () => {
const TEMPLATE = '{{ method([a, b]) }}'; 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', () => { it('should handle non-null assertions', () => {
const TEMPLATE = `{{a!}}`; const TEMPLATE = `{{a!}}`;
expect(tcb(TEMPLATE)).toContain('(((ctx).a)!);'); expect(tcb(TEMPLATE)).toContain('((((ctx).a))!);');
}); });
it('should handle keyed property access', () => { it('should handle keyed property access', () => {
const TEMPLATE = `{{a[b]}}`; 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', () => { it('should handle nested ternary expressions', () => {
const TEMPLATE = `{{a ? b : c ? d : e}}`; 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', () => { it('should handle attribute values for directive inputs', () => {
@ -115,7 +116,8 @@ describe('type check blocks', () => {
}, },
}]; }];
expect(tcb(TEMPLATE, DIRECTIVES)) 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', () => { it('should generate a forward element reference correctly', () => {
@ -123,7 +125,8 @@ describe('type check blocks', () => {
{{ i.value }} {{ i.value }}
<input #i> <input #i>
`; `;
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', () => { it('should generate a forward directive reference correctly', () => {
@ -139,7 +142,7 @@ describe('type check blocks', () => {
}]; }];
expect(tcb(TEMPLATE, DIRECTIVES)) expect(tcb(TEMPLATE, DIRECTIVES))
.toContain( .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', () => { it('should handle style and class bindings specially', () => {
@ -147,7 +150,7 @@ describe('type check blocks', () => {
<div [style]="a" [class]="b"></div> <div [style]="a" [class]="b"></div>
`; `;
const block = tcb(TEMPLATE); 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. // There should be no assignments to the class or style properties.
expect(block).not.toContain('.class = '); expect(block).not.toContain('.class = ');
@ -218,7 +221,7 @@ describe('type check blocks', () => {
it('should handle $any casts', () => { it('should handle $any casts', () => {
const TEMPLATE = `{{$any(a)}}`; const TEMPLATE = `{{$any(a)}}`;
const block = tcb(TEMPLATE); 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', () => { describe('experimental DOM checking via lib.dom.d.ts', () => {
@ -244,7 +247,7 @@ describe('type check blocks', () => {
}]; }];
const TEMPLATE = `<div *ngIf="person"></div>`; const TEMPLATE = `<div *ngIf="person"></div>`;
const block = tcb(TEMPLATE, DIRECTIVES); 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', () => { it('should emit binding guards', () => {
@ -260,7 +263,7 @@ describe('type check blocks', () => {
}]; }];
const TEMPLATE = `<div *ngIf="person !== null"></div>`; const TEMPLATE = `<div *ngIf="person !== null"></div>`;
const block = tcb(TEMPLATE, DIRECTIVES); 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', () => { it('should descend into template bodies when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES); const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('(ctx).a;'); expect(block).toContain('((ctx).a);');
}); });
it('should not descend into template bodies when disabled', () => { it('should not descend into template bodies when disabled', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTemplateBodies: false}; const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTemplateBodies: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); 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', () => { it('should include null and undefined when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES); const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a) })'); expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)) })');
expect(block).toContain('(ctx).b;'); expect(block).toContain('((ctx).b);');
}); });
it('should use the non-null assertion operator when disabled', () => { it('should use the non-null assertion operator when disabled', () => {
const DISABLED_CONFIG: const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, strictNullInputBindings: false}; TypeCheckingConfig = {...BASE_CONFIG, strictNullInputBindings: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a!) })'); expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)!) })');
expect(block).toContain('(ctx).b!;'); expect(block).toContain('((ctx).b)!;');
}); });
}); });
@ -387,8 +390,8 @@ describe('type check blocks', () => {
it('should check types of bindings when enabled', () => { it('should check types of bindings when enabled', () => {
const TEMPLATE = `<div dir [dirInput]="a" [nonDirInput]="b"></div>`; const TEMPLATE = `<div dir [dirInput]="a" [nonDirInput]="b"></div>`;
const block = tcb(TEMPLATE, DIRECTIVES); const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a) })'); expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)) })');
expect(block).toContain('(ctx).b;'); expect(block).toContain('((ctx).b);');
}); });
it('should not check types of bindings when disabled', () => { it('should not check types of bindings when disabled', () => {
@ -396,8 +399,8 @@ describe('type check blocks', () => {
const DISABLED_CONFIG: const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a as any)) })'); expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((((ctx).a) as any)) })');
expect(block).toContain('((ctx).b as any);'); expect(block).toContain('(((ctx).b) as any);');
}); });
it('should wrap the cast to any in parentheses when required', () => { it('should wrap the cast to any in parentheses when required', () => {
@ -406,7 +409,7 @@ describe('type check blocks', () => {
TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain( 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', () => { it('should check types of pipes when enabled', () => {
const block = tcb(TEMPLATE, PIPES); 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', () => { it('should not check types of pipes when disabled', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfPipes: false}; const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfPipes: false};
const block = tcb(TEMPLATE, PIPES, DISABLED_CONFIG); 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', () => { it('should use undefined for safe navigation operations when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES); const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('((null as any) ? ((ctx).a)!.method() : undefined)'); 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))!.b : undefined)');
}); });
it('should use an \'any\' type for safe navigation operations when disabled', () => { it('should use an \'any\' type for safe navigation operations when disabled', () => {
const DISABLED_CONFIG: const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, strictSafeNavigationTypes: false}; TypeCheckingConfig = {...BASE_CONFIG, strictSafeNavigationTypes: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('(((ctx).a)!.method() as any)'); expect(block).toContain('((((ctx).a))!.method() as any)');
expect(block).toContain('(((ctx).a)!.b 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()}}`; const TEMPLATE = `{{a.method()?.b}} {{a()?.method()}}`;
it('should check the presence of a property/method on the receiver when enabled', () => { it('should check the presence of a property/method on the receiver when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES); 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)'); 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', () => { it('should not check the presence of a property/method on the receiver when disabled', () => {
const DISABLED_CONFIG: const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, strictSafeNavigationTypes: false}; TypeCheckingConfig = {...BASE_CONFIG, strictSafeNavigationTypes: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); 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()'); expect(block).toContain('(((ctx).a()) as any).method()');
}); });
}); });

View File

@ -902,8 +902,7 @@ export declare class AnimationEvent {
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].messageText) expect(diags[0].messageText)
.toEqual(`Property 'does_not_exist' does not exist on type '{ name: string; }'.`); .toEqual(`Property 'does_not_exist' does not exist on type '{ name: string; }'.`);
expect(diags[0].start).toBe(199); expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist');
expect(diags[0].length).toBe(19);
}); });
it('should accept an NgFor iteration over an any-typed value', () => { it('should accept an NgFor iteration over an any-typed value', () => {
@ -1126,8 +1125,7 @@ export declare class AnimationEvent {
const diags = env.driveDiagnostics(); const diags = env.driveDiagnostics();
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].messageText).toEqual(`Property 'does_not_exist' does not exist on type 'T'.`); expect(diags[0].messageText).toEqual(`Property 'does_not_exist' does not exist on type 'T'.`);
expect(diags[0].start).toBe(206); expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist');
expect(diags[0].length).toBe(19);
}); });
describe('microsyntax variables', () => { describe('microsyntax variables', () => {
@ -1752,7 +1750,7 @@ export declare class AnimationEvent {
const diags = await driveDiagnostics(); const diags = await driveDiagnostics();
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].file!.fileName).toBe(_('/test.ts')); 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 () => { it('should be correct for indirect templates', async () => {
@ -1774,7 +1772,7 @@ export declare class AnimationEvent {
const diags = await driveDiagnostics(); const diags = await driveDiagnostics();
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].file!.fileName).toBe(_('/test.ts') + ' (TestCmp template)'); 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'); expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])).toBe('TEMPLATE');
}); });
@ -1797,7 +1795,7 @@ export declare class AnimationEvent {
const diags = await driveDiagnostics(); const diags = await driveDiagnostics();
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].file!.fileName).toBe(_('/template.html')); 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])) expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0]))
.toBe(`'./template.html'`); .toBe(`'./template.html'`);
}); });

View File

@ -665,14 +665,14 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
this._nodeMap.set( this._nodeMap.set(
leftMostSafe, leftMostSafe,
new cdAst.MethodCall( new cdAst.MethodCall(
leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.receiver, leftMostSafe.name, leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan,
leftMostSafe.args)); leftMostSafe.receiver, leftMostSafe.name, leftMostSafe.args));
} else { } else {
this._nodeMap.set( this._nodeMap.set(
leftMostSafe, leftMostSafe,
new cdAst.PropertyRead( new cdAst.PropertyRead(
leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.receiver, leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan,
leftMostSafe.name)); leftMostSafe.receiver, leftMostSafe.name));
} }
// Recursively convert the node now without the guarded member access. // Recursively convert the node now without the guarded member access.

View File

@ -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: * 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( constructor(
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string) { span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
super(span, sourceSpan); public receiver: AST, public name: string) {
super(span, sourceSpan, nameSpan);
} }
visit(visitor: AstVisitor, context: any = null): any { visit(visitor: AstVisitor, context: any = null): any {
return visitor.visitPropertyRead(this, context); return visitor.visitPropertyRead(this, context);
} }
} }
export class PropertyWrite extends AST { export class PropertyWrite extends ASTWithName {
constructor( constructor(
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string, span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
public value: AST) { public receiver: AST, public name: string, public value: AST) {
super(span, sourceSpan); super(span, sourceSpan, nameSpan);
} }
visit(visitor: AstVisitor, context: any = null): any { visit(visitor: AstVisitor, context: any = null): any {
return visitor.visitPropertyWrite(this, context); return visitor.visitPropertyWrite(this, context);
} }
} }
export class SafePropertyRead extends AST { export class SafePropertyRead extends ASTWithName {
constructor( constructor(
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string) { span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
super(span, sourceSpan); public receiver: AST, public name: string) {
super(span, sourceSpan, nameSpan);
} }
visit(visitor: AstVisitor, context: any = null): any { visit(visitor: AstVisitor, context: any = null): any {
return visitor.visitSafePropertyRead(this, context); 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( constructor(
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public exp: AST, public name: string, span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public exp: AST, public name: string,
public args: any[], public nameSpan: AbsoluteSourceSpan) { public args: any[], nameSpan: AbsoluteSourceSpan) {
super(span, sourceSpan); super(span, sourceSpan, nameSpan);
} }
visit(visitor: AstVisitor, context: any = null): any { visit(visitor: AstVisitor, context: any = null): any {
return visitor.visitPipe(this, context); 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( constructor(
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string, span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
public args: any[]) { public receiver: AST, public name: string, public args: any[]) {
super(span, sourceSpan); super(span, sourceSpan, nameSpan);
} }
visit(visitor: AstVisitor, context: any = null): any { visit(visitor: AstVisitor, context: any = null): any {
return visitor.visitMethodCall(this, context); return visitor.visitMethodCall(this, context);
} }
} }
export class SafeMethodCall extends AST { export class SafeMethodCall extends ASTWithName {
constructor( constructor(
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string, span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
public args: any[]) { public receiver: AST, public name: string, public args: any[]) {
super(span, sourceSpan); super(span, sourceSpan, nameSpan);
} }
visit(visitor: AstVisitor, context: any = null): any { visit(visitor: AstVisitor, context: any = null): any {
return visitor.visitSafeMethodCall(this, context); return visitor.visitSafeMethodCall(this, context);
@ -478,26 +487,31 @@ export class AstTransformer implements AstVisitor {
} }
visitPropertyRead(ast: PropertyRead, context: any): AST { 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 { visitPropertyWrite(ast: PropertyWrite, context: any): AST {
return new PropertyWrite( 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 { 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 { visitMethodCall(ast: MethodCall, context: any): AST {
return new MethodCall( 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 { visitSafeMethodCall(ast: SafeMethodCall, context: any): AST {
return new SafeMethodCall( 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 { visitFunctionCall(ast: FunctionCall, context: any): AST {
@ -586,7 +600,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
visitPropertyRead(ast: PropertyRead, context: any): AST { visitPropertyRead(ast: PropertyRead, context: any): AST {
const receiver = ast.receiver.visit(this); const receiver = ast.receiver.visit(this);
if (receiver !== ast.receiver) { 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; return ast;
} }
@ -595,7 +609,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
const receiver = ast.receiver.visit(this); const receiver = ast.receiver.visit(this);
const value = ast.value.visit(this); const value = ast.value.visit(this);
if (receiver !== ast.receiver || value !== ast.value) { 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; return ast;
} }
@ -603,7 +617,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
visitSafePropertyRead(ast: SafePropertyRead, context: any): AST { visitSafePropertyRead(ast: SafePropertyRead, context: any): AST {
const receiver = ast.receiver.visit(this); const receiver = ast.receiver.visit(this);
if (receiver !== ast.receiver) { 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; return ast;
} }
@ -612,7 +626,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
const receiver = ast.receiver.visit(this); const receiver = ast.receiver.visit(this);
const args = this.visitAll(ast.args); const args = this.visitAll(ast.args);
if (receiver !== ast.receiver || args !== 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; return ast;
} }
@ -621,7 +635,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
const receiver = ast.receiver.visit(this); const receiver = ast.receiver.visit(this);
const args = this.visitAll(ast.args); const args = this.visitAll(ast.args);
if (receiver !== ast.receiver || args !== 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; return ast;
} }

View File

@ -305,9 +305,34 @@ export class _ParseAST {
return this.peek(0); 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 { get inputIndex(): number {
return (this.index < this.tokens.length) ? this.next.index + this.offset : return this.atEOF ? this.currentEndIndex : this.next.index + this.offset;
this.inputLength + 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) { span(start: number) {
// `end` is either the return new ParseSpan(start, this.currentEndIndex);
// - 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);
} }
sourceSpan(start: number): AbsoluteSourceSpan { sourceSpan(start: number): AbsoluteSourceSpan {
@ -730,7 +750,9 @@ export class _ParseAST {
parseAccessMemberOrMethodCall(receiver: AST, isSafe: boolean = false): AST { parseAccessMemberOrMethodCall(receiver: AST, isSafe: boolean = false): AST {
const start = receiver.span.start; const start = receiver.span.start;
const nameStart = this.inputIndex;
const id = this.expectIdentifierOrKeyword(); const id = this.expectIdentifierOrKeyword();
const nameSpan = this.sourceSpan(nameStart);
if (this.consumeOptionalCharacter(chars.$LPAREN)) { if (this.consumeOptionalCharacter(chars.$LPAREN)) {
this.rparensExpected++; this.rparensExpected++;
@ -739,8 +761,8 @@ export class _ParseAST {
this.rparensExpected--; this.rparensExpected--;
const span = this.span(start); const span = this.span(start);
const sourceSpan = this.sourceSpan(start); const sourceSpan = this.sourceSpan(start);
return isSafe ? new SafeMethodCall(span, sourceSpan, receiver, id, args) : return isSafe ? new SafeMethodCall(span, sourceSpan, nameSpan, receiver, id, args) :
new MethodCall(span, sourceSpan, receiver, id, args); new MethodCall(span, sourceSpan, nameSpan, receiver, id, args);
} else { } else {
if (isSafe) { if (isSafe) {
@ -748,7 +770,8 @@ export class _ParseAST {
this.error('The \'?.\' operator cannot be used in the assignment'); this.error('The \'?.\' operator cannot be used in the assignment');
return new EmptyExpr(this.span(start), this.sourceSpan(start)); return new EmptyExpr(this.span(start), this.sourceSpan(start));
} else { } 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 { } else {
if (this.consumeOptionalOperator('=')) { if (this.consumeOptionalOperator('=')) {
@ -758,9 +781,10 @@ export class _ParseAST {
} }
const value = this.parseConditional(); 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 { } else {
return new PropertyRead(this.span(start), this.sourceSpan(start), receiver, id); return new PropertyRead(this.span(start), this.sourceSpan(start), nameSpan, receiver, id);
} }
} }
} }

View File

@ -1428,7 +1428,7 @@ export class ValueConverter extends AstMemoryEfficientTransformer {
// Allocate one slot for the result plus one slot per pipe argument // Allocate one slot for the result plus one slot per pipe argument
const pureFunctionSlot = this.allocatePureFunctionSlots(2 + pipe.args.length); const pureFunctionSlot = this.allocatePureFunctionSlots(2 + pipe.args.length);
const target = new PropertyRead( 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); slotPseudoLocal);
const {identifier, isVarLength} = pipeBindingCallInfo(pipe.args); const {identifier, isVarLength} = pipeBindingCallInfo(pipe.args);
this.definePipe(pipe.name, slotPseudoLocal, slot, o.importExpr(identifier)); this.definePipe(pipe.name, slotPseudoLocal, slot, o.importExpr(identifier));

View File

@ -12,7 +12,7 @@ import {Parser, SplitInterpolation} from '@angular/compiler/src/expression_parse
import {expect} from '@angular/platform-browser/testing/src/matchers'; 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'; import {validate} from './utils/validator';
describe('parser', () => { 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', () => { describe('general error handling', () => {
it('should report an unexpected token', () => { it('should report an unexpected token', () => {
expectActionError('[1,2] trac', 'Unexpected token \'trac\''); 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 ', 'of', '[1,2,3] | pipe'],
['of: [1,2,3] | pipe as items; ', 'items', 'of'], ['of: [1,2,3] | pipe as items; ', 'items', 'of'],
['let i=index, ', 'i', 'index'], ['let i=index, ', 'i', 'index'],
['count as len, ', 'len', 'count'], ['count as len,', 'len', 'count'],
]); ]);
}); });
}); });

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * 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'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/interpolation_config';
class Unparser implements AstVisitor { class Unparser implements AstVisitor {
@ -197,3 +197,34 @@ export function unparse(
ast: AST, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): string { ast: AST, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): string {
return sharedUnparser.unparse(ast, interpolationConfig); 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;
}

View File

@ -245,10 +245,18 @@ describe('expression AST absolute source spans', () => {
}); });
}); });
it('should provide absolute offsets of a property read', () => { describe('property read', () => {
expect(humanizeExpressionSource(parse('<div>{{prop}}</div>').nodes)).toContain([ it('should provide absolute offsets of a property read', () => {
'prop', new AbsoluteSourceSpan(7, 11) expect(humanizeExpressionSource(parse('<div>{{prop.obj}}<div>').nodes)).toContain([
]); 'prop.obj', new AbsoluteSourceSpan(7, 15)
]);
});
it('should provide absolute offsets of expressions in a property read', () => {
expect(humanizeExpressionSource(parse('<div>{{prop.obj}}<div>').nodes)).toContain([
'prop', new AbsoluteSourceSpan(7, 11)
]);
});
}); });
describe('property write', () => { 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('<div (click)="prop.inner = 0"></div>').nodes))
.toContain(['prop.inner = 0', new AbsoluteSourceSpan(14, 28)]);
});
it('should provide absolute offsets of expressions in a property write', () => { it('should provide absolute offsets of expressions in a property write', () => {
expect(humanizeExpressionSource(parse('<div (click)="prop = 0"></div>').nodes)).toContain([ expect(humanizeExpressionSource(parse('<div (click)="prop = 0"></div>').nodes)).toContain([
'0', new AbsoluteSourceSpan(21, 22) '0', new AbsoluteSourceSpan(21, 22)

View File

@ -283,10 +283,18 @@ describe('expression AST absolute source spans', () => {
}); });
}); });
it('should provide absolute offsets of a property read', () => { describe('property read', () => {
expect(humanizeExpressionSource(parse('<div>{{prop}}</div>'))).toContain([ it('should provide absolute offsets of a property read', () => {
'prop', new AbsoluteSourceSpan(7, 11) expect(humanizeExpressionSource(parse('<div>{{prop.obj}}<div>'))).toContain([
]); 'prop.obj', new AbsoluteSourceSpan(7, 15)
]);
});
it('should provide absolute offsets of expressions in a property read', () => {
expect(humanizeExpressionSource(parse('<div>{{prop.obj}}<div>'))).toContain([
'prop', new AbsoluteSourceSpan(7, 11)
]);
});
}); });
describe('property write', () => { 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('<div (click)="prop.inner = 0"></div>'))).toContain([
'prop.inner = 0', new AbsoluteSourceSpan(14, 28)
]);
});
it('should provide absolute offsets of expressions in a property write', () => { it('should provide absolute offsets of expressions in a property write', () => {
expect(humanizeExpressionSource(parse('<div (click)="prop = 0"></div>'))).toContain([ expect(humanizeExpressionSource(parse('<div (click)="prop = 0"></div>'))).toContain([
'0', new AbsoluteSourceSpan(21, 22) '0', new AbsoluteSourceSpan(21, 22)

View File

@ -457,8 +457,13 @@ class ExpressionVisitor extends NullTemplateVisitor {
const absValueOffset = ast.sourceSpan.start.offset; const absValueOffset = ast.sourceSpan.start.offset;
const {templateBindings} = this.info.expressionParser.parseTemplateBindings( const {templateBindings} = this.info.expressionParser.parseTemplateBindings(
templateKey, templateValue, templateUrl, absKeyOffset, absValueOffset); templateKey, templateValue, templateUrl, absKeyOffset, absValueOffset);
// Find the template binding that contains the position. // Find the nearest template binding to the position.
const templateBinding = templateBindings.find(b => inSpan(this.position, b.sourceSpan)); 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) { if (!templateBinding) {
return; return;

View File

@ -6,11 +6,12 @@
* found in the LICENSE file at https://angular.io/license * 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 {createDiagnostic, Diagnostic} from './diagnostic_messages';
import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols'; import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols';
import * as ng from './types'; import * as ng from './types';
import {offsetSpan} from './utils';
interface ExpressionDiagnosticsContext { interface ExpressionDiagnosticsContext {
inEvent?: boolean; inEvent?: boolean;
@ -32,7 +33,7 @@ export class AstType implements AstVisitor {
const type: Symbol = ast.visit(this); const type: Symbol = ast.visit(this);
if (this.context.inEvent && type.callable) { if (this.context.inEvent && type.callable) {
this.diagnostics.push( this.diagnostics.push(
createDiagnostic(ast.span, Diagnostic.callable_expression_expected_method_call)); createDiagnostic(refinedSpan(ast), Diagnostic.callable_expression_expected_method_call));
} }
return this.diagnostics; return this.diagnostics;
} }
@ -51,7 +52,8 @@ export class AstType implements AstVisitor {
// Nullable allowed. // Nullable allowed.
break; break;
default: default:
this.diagnostics.push(createDiagnostic(ast.span, Diagnostic.expression_might_be_null)); this.diagnostics.push(
createDiagnostic(refinedSpan(ast), Diagnostic.expression_might_be_null));
break; break;
} }
} }
@ -130,7 +132,7 @@ export class AstType implements AstVisitor {
return this.anyType; return this.anyType;
default: default:
this.diagnostics.push( 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; return this.anyType;
} }
case '>': case '>':
@ -146,8 +148,8 @@ export class AstType implements AstVisitor {
// Two values are comparable only if // Two values are comparable only if
// - they have some type overlap, or // - they have some type overlap, or
// - at least one is not defined // - at least one is not defined
this.diagnostics.push( this.diagnostics.push(createDiagnostic(
createDiagnostic(ast.span, Diagnostic.expected_operands_of_comparable_types_or_any)); refinedSpan(ast), Diagnostic.expected_operands_of_comparable_types_or_any));
} }
return this.query.getBuiltinType(BuiltinType.Boolean); return this.query.getBuiltinType(BuiltinType.Boolean);
case '&&': case '&&':
@ -157,7 +159,7 @@ export class AstType implements AstVisitor {
} }
this.diagnostics.push( this.diagnostics.push(
createDiagnostic(ast.span, Diagnostic.unrecognized_operator, ast.operation)); createDiagnostic(refinedSpan(ast), Diagnostic.unrecognized_operator, ast.operation));
return this.anyType; return this.anyType;
} }
@ -187,7 +189,8 @@ export class AstType implements AstVisitor {
const target = this.getType(ast.target!); const target = this.getType(ast.target!);
if (!target || !target.callable) { if (!target || !target.callable) {
this.diagnostics.push(createDiagnostic( 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; return this.anyType;
} }
const signature = target.selectSignature(args); 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 // TODO: Consider a better error message here. See `typescript_symbols#selectSignature` for more
// details. // details.
this.diagnostics.push( 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; return this.anyType;
} }
@ -291,8 +294,8 @@ export class AstType implements AstVisitor {
case 'number': case 'number':
return this.query.getBuiltinType(BuiltinType.Number); return this.query.getBuiltinType(BuiltinType.Number);
default: default:
this.diagnostics.push( this.diagnostics.push(createDiagnostic(
createDiagnostic(ast.span, Diagnostic.unrecognized_primitive, typeof ast.value)); refinedSpan(ast), Diagnostic.unrecognized_primitive, typeof ast.value));
return this.anyType; 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. // by getPipes() is expected to contain symbols with the corresponding transform method type.
const pipe = this.query.getPipes().get(ast.name); const pipe = this.query.getPipes().get(ast.name);
if (!pipe) { 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; return this.anyType;
} }
const expType = this.getType(ast.exp); 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)))); pipe.selectSignature([expType].concat(ast.args.map(arg => this.getType(arg))));
if (!signature) { if (!signature) {
this.diagnostics.push( 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 this.anyType;
} }
return signature.result; return signature.result;
@ -389,7 +392,7 @@ export class AstType implements AstVisitor {
const methodType = this.resolvePropertyRead(receiverType, ast); const methodType = this.resolvePropertyRead(receiverType, ast);
if (!methodType) { if (!methodType) {
this.diagnostics.push( 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; return this.anyType;
} }
if (this.isAny(methodType)) { if (this.isAny(methodType)) {
@ -397,13 +400,13 @@ export class AstType implements AstVisitor {
} }
if (!methodType.callable) { if (!methodType.callable) {
this.diagnostics.push( this.diagnostics.push(
createDiagnostic(ast.span, Diagnostic.identifier_not_callable, ast.name)); createDiagnostic(refinedSpan(ast), Diagnostic.identifier_not_callable, ast.name));
return this.anyType; return this.anyType;
} }
const signature = methodType.selectSignature(ast.args.map(arg => this.getType(arg))); const signature = methodType.selectSignature(ast.args.map(arg => this.getType(arg)));
if (!signature) { if (!signature) {
this.diagnostics.push( 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 this.anyType;
} }
return signature.result; return signature.result;
@ -417,24 +420,25 @@ export class AstType implements AstVisitor {
const member = receiverType.members().get(ast.name); const member = receiverType.members().get(ast.name);
if (!member) { if (!member) {
if (receiverType.name === '$implicit') { if (receiverType.name === '$implicit') {
this.diagnostics.push( this.diagnostics.push(createDiagnostic(
createDiagnostic(ast.span, Diagnostic.identifier_not_defined_in_app_context, ast.name)); refinedSpan(ast), Diagnostic.identifier_not_defined_in_app_context, ast.name));
} else if (receiverType.nullable && ast.receiver instanceof PropertyRead) { } else if (receiverType.nullable && ast.receiver instanceof PropertyRead) {
const receiver = ast.receiver.name; const receiver = ast.receiver.name;
this.diagnostics.push(createDiagnostic( this.diagnostics.push(createDiagnostic(
ast.span, Diagnostic.identifier_possibly_undefined, receiver, refinedSpan(ast), Diagnostic.identifier_possibly_undefined, receiver,
`${receiver}?.${ast.name}`, `${receiver}!.${ast.name}`)); `${receiver}?.${ast.name}`, `${receiver}!.${ast.name}`));
} else { } else {
this.diagnostics.push(createDiagnostic( 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; return this.anyType;
} }
if (!member.public) { if (!member.public) {
const container = const container =
receiverType.name === '$implicit' ? 'the component' : `'${receiverType.name}'`; receiverType.name === '$implicit' ? 'the component' : `'${receiverType.name}'`;
this.diagnostics.push( this.diagnostics.push(createDiagnostic(
createDiagnostic(ast.span, Diagnostic.identifier_is_private, ast.name, container)); refinedSpan(ast), Diagnostic.identifier_is_private, ast.name, container));
} }
return member.type; return member.type;
} }
@ -444,3 +448,14 @@ export class AstType implements AstVisitor {
(!!symbol.type && this.isAny(symbol.type)); (!!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);
}

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * 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 {AstType} from './expression_type';
import {BuiltinType, Span, Symbol, SymbolTable, TemplateSource} from './types'; 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); 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 symbol: Symbol|undefined = undefined;
let span: Span|undefined = undefined; let span: Span|undefined = undefined;
@ -141,22 +152,14 @@ export function getExpressionSymbol(
visitMethodCall(ast) { visitMethodCall(ast) {
const receiverType = getType(ast.receiver); const receiverType = getType(ast.receiver);
symbol = receiverType && receiverType.members().get(ast.name); symbol = receiverType && receiverType.members().get(ast.name);
span = ast.span; span = spanFromName(ast);
}, },
visitPipe(ast) { visitPipe(ast) {
if (inSpan(position, ast.nameSpan, /* exclusive */ true)) { if (inSpan(position, ast.nameSpan, /* exclusive */ true)) {
// We are in a position a pipe name is expected. // We are in a position a pipe name is expected.
const pipes = templateInfo.query.getPipes(); const pipes = templateInfo.query.getPipes();
symbol = pipes.get(ast.name); symbol = pipes.get(ast.name);
span = spanFromName(ast);
// `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,
};
} }
}, },
visitPrefixNot(_ast) {}, visitPrefixNot(_ast) {},
@ -164,29 +167,23 @@ export function getExpressionSymbol(
visitPropertyRead(ast) { visitPropertyRead(ast) {
const receiverType = getType(ast.receiver); const receiverType = getType(ast.receiver);
symbol = receiverType && receiverType.members().get(ast.name); symbol = receiverType && receiverType.members().get(ast.name);
span = ast.span; span = spanFromName(ast);
}, },
visitPropertyWrite(ast) { visitPropertyWrite(ast) {
const receiverType = getType(ast.receiver); const receiverType = getType(ast.receiver);
const {start} = ast.span;
symbol = receiverType && receiverType.members().get(ast.name); symbol = receiverType && receiverType.members().get(ast.name);
// A PropertyWrite span includes both the LHS (name) and the RHS (value) of the write. In this span = spanFromName(ast);
// visit, only the name is relevant.
// prop=$event
// ^^^^ name
// ^^^^^^ value; visited separately as a nested AST
span = {start, end: start + ast.name.length};
}, },
visitQuote(_ast) {}, visitQuote(_ast) {},
visitSafeMethodCall(ast) { visitSafeMethodCall(ast) {
const receiverType = getType(ast.receiver); const receiverType = getType(ast.receiver);
symbol = receiverType && receiverType.members().get(ast.name); symbol = receiverType && receiverType.members().get(ast.name);
span = ast.span; span = spanFromName(ast);
}, },
visitSafePropertyRead(ast) { visitSafePropertyRead(ast) {
const receiverType = getType(ast.receiver); const receiverType = getType(ast.receiver);
symbol = receiverType && receiverType.members().get(ast.name); symbol = receiverType && receiverType.members().get(ast.name);
span = ast.span; span = spanFromName(ast);
}, },
}); });

View File

@ -77,7 +77,7 @@ describe('definitions', () => {
it('should be able to find a method from a call', () => { it('should be able to find a method from a call', () => {
const fileName = mockHost.addCode(` const fileName = mockHost.addCode(`
@Component({ @Component({
template: '<div (click)="~{start-my}«myClick»()~{end-my};"></div>' template: '<div (click)="«myClick»();"></div>'
}) })
export class MyComponent { export class MyComponent {
«myClickᐱ() { }» «myClickᐱ() { }»
@ -88,7 +88,7 @@ describe('definitions', () => {
expect(result).toBeDefined(); expect(result).toBeDefined();
const {textSpan, definitions} = result!; const {textSpan, definitions} = result!;
expect(textSpan).toEqual(mockHost.getLocationMarkerFor(fileName, 'my')); expect(textSpan).toEqual(marker);
expect(definitions).toBeDefined(); expect(definitions).toBeDefined();
expect(definitions!.length).toBe(1); expect(definitions!.length).toBe(1);
const def = definitions![0]; const def = definitions![0];

View File

@ -182,7 +182,7 @@ describe('diagnostics', () => {
mockHost.override(TEST_TEMPLATE, ` mockHost.override(TEST_TEMPLATE, `
<div *ngIf="title; let titleProxy;"> <div *ngIf="title; let titleProxy;">
'titleProxy' is a string 'titleProxy' is a string
{{~{start-err}titleProxy.notAProperty~{end-err}}} {{titleProxy.~{start-err}notAProperty~{end-err}}}
</div> </div>
`); `);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
@ -200,7 +200,7 @@ describe('diagnostics', () => {
mockHost.override(TEST_TEMPLATE, ` mockHost.override(TEST_TEMPLATE, `
<div *ngIf="title as titleProxy"> <div *ngIf="title as titleProxy">
'titleProxy' is a string 'titleProxy' is a string
{{~{start-err}titleProxy.notAProperty~{end-err}}} {{titleProxy.~{start-err}notAProperty~{end-err}}}
</div> </div>
`); `);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
@ -364,7 +364,7 @@ describe('diagnostics', () => {
it('report an unknown field in $implicit context', () => { it('report an unknown field in $implicit context', () => {
mockHost.override(TEST_TEMPLATE, ` mockHost.override(TEST_TEMPLATE, `
<div *withContext="let myVar"> <div *withContext="let myVar">
{{ ~{start-emb}myVar.missingField~{end-emb} }} {{ myVar.~{start-emb}missingField~{end-emb} }}
</div> </div>
`); `);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
@ -383,7 +383,7 @@ describe('diagnostics', () => {
it('report an unknown field in non implicit context', () => { it('report an unknown field in non implicit context', () => {
mockHost.override(TEST_TEMPLATE, ` mockHost.override(TEST_TEMPLATE, `
<div *withContext="let myVar = nonImplicitPerson"> <div *withContext="let myVar = nonImplicitPerson">
{{ ~{start-emb}myVar.missingField~{end-emb} }} {{ myVar.~{start-emb}missingField~{end-emb} }}
</div> </div>
`); `);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
@ -421,8 +421,7 @@ describe('diagnostics', () => {
const {messageText, start, length} = diagnostics[0]; const {messageText, start, length} = diagnostics[0];
expect(messageText) expect(messageText)
.toBe(`Identifier 'xyz' is not defined. 'Hero' does not contain such a member`); .toBe(`Identifier 'xyz' is not defined. 'Hero' does not contain such a member`);
expect(start).toBe(content.indexOf('member.xyz')); expect(content.substring(start!, start! + length!)).toBe('xyz');
expect(length).toBe('member.xyz'.length);
}); });
describe('with $event', () => { describe('with $event', () => {
@ -454,8 +453,7 @@ describe('diagnostics', () => {
expect(messageText) expect(messageText)
.toBe( .toBe(
`Identifier 'notSubstring' is not defined. 'string' does not contain such a member`); `Identifier 'notSubstring' is not defined. 'string' does not contain such a member`);
expect(start).toBe(content.indexOf('$event')); expect(content.substring(start!, start! + length!)).toBe('notSubstring');
expect(length).toBe('$event.notSubstring()'.length);
}); });
}); });
@ -990,7 +988,7 @@ describe('diagnostics', () => {
`Consider using the safe navigation operator (optional?.toLowerCase) ` + `Consider using the safe navigation operator (optional?.toLowerCase) ` +
`or non-null assertion operator (optional!.toLowerCase).`); `or non-null assertion operator (optional!.toLowerCase).`);
expect(category).toBe(ts.DiagnosticCategory.Suggestion); 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', () => { it('should suggest ? or ! operator if property receiver is nullable', () => {
@ -1004,40 +1002,40 @@ describe('diagnostics', () => {
`Consider using the safe navigation operator (optional?.length) ` + `Consider using the safe navigation operator (optional?.length) ` +
`or non-null assertion operator (optional!.length).`); `or non-null assertion operator (optional!.length).`);
expect(category).toBe(ts.DiagnosticCategory.Suggestion); 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 = [ const expressions = [
'optional?.someMethod()', 'optional?',
'optional!.someMethod()', 'optional!',
]; ];
for (const expression of expressions) { 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); const ngDiags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
expect(ngDiags.length).toBe(1); expect(ngDiags.length).toBe(1);
const {start, length, messageText, category} = ngDiags[0]; const {start, length, messageText, category} = ngDiags[0];
expect(messageText) expect(messageText)
.toBe(`Identifier 'someMethod' is not defined. 'string' does not contain such a member`); .toBe(`Identifier 'someMethod' is not defined. 'string' does not contain such a member`);
expect(category).toBe(ts.DiagnosticCategory.Error); 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 = [ const expressions = [
'optional?.someProp', 'optional?',
'optional!.someProp', 'optional!',
]; ];
for (const expression of expressions) { 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); const ngDiags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
expect(ngDiags.length).toBe(1); expect(ngDiags.length).toBe(1);
const {start, length, messageText, category} = ngDiags[0]; const {start, length, messageText, category} = ngDiags[0];
expect(messageText) expect(messageText)
.toBe(`Identifier 'someProp' is not defined. 'string' does not contain such a member`); .toBe(`Identifier 'someProp' is not defined. 'string' does not contain such a member`);
expect(category).toBe(ts.DiagnosticCategory.Error); expect(category).toBe(ts.DiagnosticCategory.Error);
expect(content.substring(start!, start! + length!)).toBe(expression); expect(content.substring(start!, start! + length!)).toBe('someProp');
} }
}); });

View File

@ -109,6 +109,36 @@ describe('hover', () => {
expect(toText(displayParts)).toBe('(property) TemplateReference.title: string'); expect(toText(displayParts)).toBe('(property) TemplateReference.title: string');
}); });
it('should work for accessed property reads', () => {
mockHost.override(TEST_TEMPLATE, `<div>{{title.«length»}}</div>`);
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, `<div (click)="«title» = 't'"></div>`);
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, `<div (click)="hero.«id» = 2"></div>`);
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', () => { it('should work for array members', () => {
mockHost.override(TEST_TEMPLATE, `<div *ngFor="let hero of heroes">{{«hero»}}</div>`); mockHost.override(TEST_TEMPLATE, `<div *ngFor="let hero of heroes">{{«hero»}}</div>`);
const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'hero'); const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'hero');
@ -142,8 +172,8 @@ describe('hover', () => {
}); });
it('should work for method calls', () => { it('should work for method calls', () => {
mockHost.override(TEST_TEMPLATE, `<div (click)="«ᐱmyClickᐱ($event)»"></div>`); mockHost.override(TEST_TEMPLATE, `<div (click)="«myClick»($event)"></div>`);
const marker = mockHost.getDefinitionMarkerFor(TEST_TEMPLATE, 'myClick'); const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'myClick');
const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start); const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start);
expect(quickInfo).toBeTruthy(); expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo!; const {textSpan, displayParts} = quickInfo!;
@ -192,7 +222,7 @@ describe('hover', () => {
const {textSpan, displayParts} = quickInfo!; const {textSpan, displayParts} = quickInfo!;
expect(textSpan).toEqual({ expect(textSpan).toEqual({
start: position, start: position,
length: '$any(title)'.length, length: '$any'.length,
}); });
expect(toText(displayParts)).toBe('(method) $any: $any'); expect(toText(displayParts)).toBe('(method) $any: $any');
}); });