From 1bf1b685d6ac2f2dfea5363b6af604cf48050820 Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Wed, 9 Dec 2020 09:39:33 -0800
Subject: [PATCH] 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
---
 packages/language-service/ivy/template_target.ts      |  9 +++++++++
 .../ivy/test/legacy/template_target_spec.ts           | 11 ++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/packages/language-service/ivy/template_target.ts b/packages/language-service/ivy/template_target.ts
index 3207cdcd42..4a18374a3b 100644
--- a/packages/language-service/ivy/template_target.ts
+++ b/packages/language-service/ivy/template_target.ts
@@ -100,6 +100,15 @@ class TemplateTargetVisitor implements t.Visitor {
   private constructor(private readonly position: number) {}
 
   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);
     if (isWithin(this.position, {start, end})) {
       this.path.push(node);
diff --git a/packages/language-service/ivy/test/legacy/template_target_spec.ts b/packages/language-service/ivy/test/legacy/template_target_spec.ts
index f54a374edc..a2ac29fa81 100644
--- a/packages/language-service/ivy/test/legacy/template_target_spec.ts
+++ b/packages/language-service/ivy/test/legacy/template_target_spec.ts
@@ -580,13 +580,10 @@ describe('findNodeAtPosition for microsyntax expression', () => {
   it('should locate bound attribute key when cursor is at the start', () => {
     const {errors, nodes, position} = parse(`<div *ngFor="let item ¦of items"></div>`);
     expect(errors).toBe(null);
-    // TODO(atscott): Fix this - we throw away the result because we match the variable node, after
-    // the attribute binding, then throw away the result because we aren't in the variable key
-    expect(getTargetAtPosition(nodes, position)).toBeNull();
-    // const {node} = getTargetAtPosition(nodes, position)!;
-    // expect(isTemplateNode(node!)).toBe(true);
-    // expect(node).toBeInstanceOf(t.BoundAttribute);
-    // expect((node as t.BoundAttribute).name).toBe('ngForOf');
+    const {node} = getTargetAtPosition(nodes, position)!;
+    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', () => {