refactor(language-service): Update hybrid visitor to use keySpan for bound attributes (#38955)

The keySpan in bound attributes provides more fine-grained location information and can be used
to disambiguate multiple bound attributes in a single microsyntax binding. Previously,
this case could not distinguish between the two different attributes because
the sourceSpans were identical and valueSpans would not match if the cursor
was located in a key.

PR Close #38955
This commit is contained in:
Andrew Scott 2020-09-23 08:36:38 -07:00 committed by Alex Rickabaugh
parent 8422633b8a
commit 4c8766573d
2 changed files with 39 additions and 8 deletions

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 {ParseSourceSpan} from '@angular/compiler'; import {AbsoluteSourceSpan, ParseSourceSpan} from '@angular/compiler';
import * as e from '@angular/compiler/src/expression_parser/ast'; // e for expression AST import * as e from '@angular/compiler/src/expression_parser/ast'; // e for expression AST
import * as t from '@angular/compiler/src/render3/r3_ast'; // t for template AST import * as t from '@angular/compiler/src/render3/r3_ast'; // t for template AST
@ -31,10 +31,13 @@ class R3Visitor implements t.Visitor {
constructor(private readonly position: number) {} constructor(private readonly position: number) {}
visit(node: t.Node) { visit(node: t.Node) {
if (node instanceof t.BoundAttribute) {
node.visit(this);
return;
}
const {start, end} = getSpanIncludingEndTag(node); const {start, end} = getSpanIncludingEndTag(node);
// Note both start and end are inclusive because we want to match conditions if (isWithin(this.position, {start, end})) {
// like ¦start and end¦ where ¦ is the cursor.
if (start <= this.position && this.position <= end) {
const length = end - start; const length = end - start;
const last: t.Node|e.AST|undefined = this.path[this.path.length - 1]; const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
if (last) { if (last) {
@ -97,8 +100,13 @@ class R3Visitor implements t.Visitor {
} }
visitBoundAttribute(attribute: t.BoundAttribute) { visitBoundAttribute(attribute: t.BoundAttribute) {
const visitor = new ExpressionVisitor(this.position); if (isWithin(this.position, attribute.keySpan)) {
visitor.visit(attribute.value, this.path); this.path.push(attribute);
} else if (attribute.valueSpan && isWithin(this.position, attribute.valueSpan)) {
this.path.push(attribute);
const visitor = new ExpressionVisitor(this.position);
visitor.visit(attribute.value, this.path);
}
} }
visitBoundEvent(attribute: t.BoundEvent) { visitBoundEvent(attribute: t.BoundEvent) {
@ -144,10 +152,9 @@ class ExpressionVisitor extends e.RecursiveAstVisitor {
// `ASTWithSource` and and underlying node that it wraps. // `ASTWithSource` and and underlying node that it wraps.
node = node.ast; node = node.ast;
} }
const {start, end} = node.sourceSpan;
// The third condition is to account for the implicit receiver, which should // The third condition is to account for the implicit receiver, which should
// not be visited. // not be visited.
if (start <= this.position && this.position <= end && !(node instanceof e.ImplicitReceiver)) { if (isWithin(this.position, node.sourceSpan) && !(node instanceof e.ImplicitReceiver)) {
path.push(node); path.push(node);
node.visit(this, path); node.visit(this, path);
} }
@ -178,3 +185,17 @@ function getSpanIncludingEndTag(ast: t.Node) {
} }
return result; return result;
} }
function isWithin(position: number, span: AbsoluteSourceSpan|ParseSourceSpan): boolean {
let start: number, end: number;
if (span instanceof ParseSourceSpan) {
start = span.start.offset;
end = span.end.offset;
} else {
start = span.start;
end = span.end;
}
// Note both start and end are inclusive because we want to match conditions
// like ¦start and end¦ where ¦ is the cursor.
return start <= position && position <= end;
}

View File

@ -518,6 +518,16 @@ describe('findNodeAtPosition for microsyntax expression', () => {
expect((node as t.BoundAttribute).name).toBe('ngForOf'); expect((node as t.BoundAttribute).name).toBe('ngForOf');
}); });
it('should locate bound attribute key for trackBy', () => {
const {errors, nodes, position} =
parse(`<div *ngFor="let item of items; trac¦kBy: trackByFn"></div>`);
expect(errors).toBe(null);
const node = findNodeAtPosition(nodes, position);
expect(isTemplateNode(node!)).toBe(true);
expect(node).toBeInstanceOf(t.BoundAttribute);
expect((node as t.BoundAttribute).name).toBe('ngForTrackBy');
});
it('should locate bound attribute value', () => { it('should locate bound attribute value', () => {
const {errors, nodes, position} = parse(`<div *ngFor="let item of it¦ems"></div>`); const {errors, nodes, position} = parse(`<div *ngFor="let item of it¦ems"></div>`);
expect(errors).toBe(null); expect(errors).toBe(null);