fix(language-service): Prevent matching nodes after finding a keySpan (#40047)

If we've already identified that we are within a `keySpan` of a node, we
exit the visitor logic early. It can be the case that we have two nodes
which technically match a given location when the end span of one node
touches the start of the keySpan for the candidate node. Because
our `isWithin` logic is inclusive on both ends, we can match both nodes.
This change exits the visitor logic once we've identified a node where
the position is within its `keySpan`.

PR Close #40047
This commit is contained in:
Andrew Scott 2020-12-09 09:39:33 -08:00 committed by Alex Rickabaugh
parent 371a2c82ea
commit 1bf1b685d6
2 changed files with 13 additions and 7 deletions

View File

@ -100,6 +100,15 @@ class TemplateTargetVisitor implements t.Visitor {
private constructor(private readonly position: number) {} private constructor(private readonly position: number) {}
visit(node: t.Node) { visit(node: t.Node) {
const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
if (last && isTemplateNodeWithKeyAndValue(last) && isWithin(this.position, last.keySpan)) {
// We've already identified that we are within a `keySpan` of a node.
// We should stop processing nodes at this point to prevent matching
// any other nodes. This can happen when the end span of a different node
// touches the start of the keySpan for the candidate node. Because
// our `isWithin` logic is inclusive on both ends, we can match both nodes.
return;
}
const {start, end} = getSpanIncludingEndTag(node); const {start, end} = getSpanIncludingEndTag(node);
if (isWithin(this.position, {start, end})) { if (isWithin(this.position, {start, end})) {
this.path.push(node); this.path.push(node);

View File

@ -580,13 +580,10 @@ describe('findNodeAtPosition for microsyntax expression', () => {
it('should locate bound attribute key when cursor is at the start', () => { it('should locate bound attribute key when cursor is at the start', () => {
const {errors, nodes, position} = parse(`<div *ngFor="let item ¦of items"></div>`); const {errors, nodes, position} = parse(`<div *ngFor="let item ¦of items"></div>`);
expect(errors).toBe(null); expect(errors).toBe(null);
// TODO(atscott): Fix this - we throw away the result because we match the variable node, after const {node} = getTargetAtPosition(nodes, position)!;
// the attribute binding, then throw away the result because we aren't in the variable key expect(isTemplateNode(node!)).toBe(true);
expect(getTargetAtPosition(nodes, position)).toBeNull(); expect(node).toBeInstanceOf(t.BoundAttribute);
// const {node} = getTargetAtPosition(nodes, position)!; expect((node as t.BoundAttribute).name).toBe('ngForOf');
// expect(isTemplateNode(node!)).toBe(true);
// expect(node).toBeInstanceOf(t.BoundAttribute);
// expect((node as t.BoundAttribute).name).toBe('ngForOf');
}); });
it('should locate bound attribute key for trackBy', () => { it('should locate bound attribute key for trackBy', () => {