refactor(language-service): adjust hybrid visitor to not throw away v…

…alid results (angular#40047)

The visitor has a check in it with the goal of preventing the structural directive
parent elements from matching when we have already found the candidate we want.
However, this code did not check to ensure that it was looking at the correct
type of node for this case and was evaluating this logic in places it shouldn't.
This special check can be more easily done by simply not traversing the
template children if we've already found a candidate on the template
node itself.

PR Close angular#40047

packages/language-service/ivy/template_target.ts

@@ -102,19 +102,6 @@ class TemplateTargetVisitor implements t.Visitor {
   visit(node: t.Node) {
     const {start, end} = getSpanIncludingEndTag(node);
     if (isWithin(this.position, {start, end})) {
-      const length = end - start;
-      const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
-      if (last) {
-        const {start, end} = isTemplateNode(last) ? getSpanIncludingEndTag(last) : last.sourceSpan;
-        const lastLength = end - start;
-        if (length > lastLength) {
-          // The current node has a span that is larger than the last node found
-          // so we do not descend into it. This typically means we have found
-          // a candidate in one of the root nodes so we do not need to visit
-          // other root nodes.
-          return;
-        }
-      }
       this.path.push(node);
       node.visit(this);
     }
@@ -125,7 +112,12 @@ class TemplateTargetVisitor implements t.Visitor {
     this.visitAll(element.inputs);
     this.visitAll(element.outputs);
     this.visitAll(element.references);
-    this.visitAll(element.children);
+    const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
+    // If we get here and have not found a candidate node on the element itself, proceed with
+    // looking for a more specific node on the element children.
+    if (last === element) {
+      this.visitAll(element.children);
+    }
   }
 
   visitTemplate(template: t.Template) {
@@ -135,7 +127,12 @@ class TemplateTargetVisitor implements t.Visitor {
     this.visitAll(template.templateAttrs);
     this.visitAll(template.references);
     this.visitAll(template.variables);
-    this.visitAll(template.children);
+    const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
+    // If we get here and have not found a candidate node on the template itself, proceed with
+    // looking for a more specific node on the template children.
+    if (last === template) {
+      this.visitAll(template.children);
+    }
   }
 
   visitContent(content: t.Content) {

packages/language-service/ivy/test/legacy/template_target_spec.ts

@@ -577,6 +577,18 @@ describe('findNodeAtPosition for microsyntax expression', () => {
     expect((node as t.BoundAttribute).name).toBe('ngForOf');
   });
 
+  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');
+  });
+
   it('should locate bound attribute key for trackBy', () => {
     const {errors, nodes, position} =
         parse(`<div *ngFor="let item of items; trac¦kBy: trackByFn"></div>`);