fix(language-service): HTML path should include last node before curs…

…or (angular#34440)

Given the following HTML and cursor position:
```
<div c|></div>
      ^ cursor is here
```

Note that the cursor is **after** the attribute `c`.

Under the current implementation, only `Element` is included in the
path. Instead, it should be `Element -> Attribute`.

This bug occurs only for cases where the cursor is right after the Node,
and it is because the `end` position of the span is excluded from the search.
Instead, the `end` position should be included.

PR Close angular#34440

packages/language-service/src/completions.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, Element, ElementAst, ImplicitReceiver, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ParseSpan, PropertyRead, TagContentType, TemplateBinding, Text, findNode, getHtmlTagDefinition} from '@angular/compiler';
+import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, Element, ElementAst, ImplicitReceiver, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ParseSpan, PropertyRead, TagContentType, TemplateBinding, Text, getHtmlTagDefinition} from '@angular/compiler';
 import {$$, $_, isAsciiLetter, isDigit} from '@angular/compiler/src/chars';
 
 import {AstResult} from './common';
@@ -15,7 +15,7 @@ import {getExpressionCompletions} from './expressions';
 import {attributeNames, elementNames, eventNames, propertyNames} from './html_info';
 import {InlineTemplate} from './template';
 import * as ng from './types';
-import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getSelectors, hasTemplateReference, inSpan, spanOf} from './utils';
+import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getPathToNodeAtPosition, getSelectors, hasTemplateReference, inSpan, spanOf} from './utils';
 
 const HIDDEN_HTML_ELEMENTS: ReadonlySet<string> =
     new Set(['html', 'script', 'noscript', 'base', 'body', 'title', 'head', 'link']);
@@ -121,7 +121,7 @@ export function getTemplateCompletions(
   const {htmlAst, template} = templateInfo;
   // The templateNode starts at the delimiter character so we add 1 to skip it.
   const templatePosition = position - template.span.start;
-  const path = findNode(htmlAst, templatePosition);
+  const path = getPathToNodeAtPosition(htmlAst, templatePosition);
   const mostSpecific = path.tail;
   if (path.empty || !mostSpecific) {
     result = elementCompletions(templateInfo);

packages/language-service/src/expression_diagnostics.ts

@@ -6,12 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, CompileDirectiveSummary, CompileTypeMetadata, DirectiveAst, ElementAst, EmbeddedTemplateAst, Node, ParseSourceSpan, RecursiveTemplateAstVisitor, ReferenceAst, TemplateAst, TemplateAstPath, VariableAst, findNode, identifierName, templateVisitAll, tokenReference} from '@angular/compiler';
+import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, CompileDirectiveSummary, CompileTypeMetadata, DirectiveAst, ElementAst, EmbeddedTemplateAst, Node, ParseSourceSpan, RecursiveTemplateAstVisitor, ReferenceAst, TemplateAst, TemplateAstPath, VariableAst, identifierName, templateVisitAll, tokenReference} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {AstType, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type';
 import {BuiltinType, Definition, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './symbols';
 import {Diagnostic} from './types';
+import {getPathToNodeAtPosition} from './utils';
 
 export interface DiagnosticTemplateInfo {
   fileName?: string;
@@ -304,7 +305,7 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
   }
 
   private attributeValueLocation(ast: TemplateAst) {
-    const path = findNode(this.info.htmlAst, ast.sourceSpan.start.offset);
+    const path = getPathToNodeAtPosition(this.info.htmlAst, ast.sourceSpan.start.offset);
     const last = path.tail;
     if (last instanceof Attribute && last.valueSpan) {
       return last.valueSpan.start.offset;

packages/language-service/src/locate_symbol.ts

@@ -6,13 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AST, Attribute, BoundDirectivePropertyAst, BoundEventAst, CompileTypeSummary, CssSelector, DirectiveAst, ElementAst, SelectorMatcher, TemplateAstPath, findNode, tokenReference} from '@angular/compiler';
+import {AST, Attribute, BoundDirectivePropertyAst, BoundEventAst, CompileTypeSummary, CssSelector, DirectiveAst, ElementAst, SelectorMatcher, TemplateAstPath, tokenReference} from '@angular/compiler';
 
 import {AstResult} from './common';
 import {getExpressionScope} from './expression_diagnostics';
 import {getExpressionSymbol} from './expressions';
 import {Definition, DirectiveKind, Span, Symbol} from './types';
-import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, inSpan, offsetSpan, spanOf} from './utils';
+import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getPathToNodeAtPosition, inSpan, offsetSpan, spanOf} from './utils';
 
 export interface SymbolInfo {
   symbol: Symbol;
@@ -144,7 +144,7 @@ export function locateSymbol(info: AstResult, position: number): SymbolInfo|unde
 
 function findAttribute(info: AstResult, position: number): Attribute|undefined {
   const templatePosition = position - info.template.span.start;
-  const path = findNode(info.htmlAst, templatePosition);
+  const path = getPathToNodeAtPosition(info.htmlAst, templatePosition);
   return path.first(Attribute);
 }
 

packages/language-service/src/utils.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AstPath, CompileDirectiveSummary, CompileTypeMetadata, CssSelector, DirectiveAst, ElementAst, EmbeddedTemplateAst, ParseSourceSpan, RecursiveTemplateAstVisitor, TemplateAst, TemplateAstPath, identifierName, templateVisitAll} from '@angular/compiler';
+import {AstPath, CompileDirectiveSummary, CompileTypeMetadata, CssSelector, DirectiveAst, ElementAst, EmbeddedTemplateAst, HtmlAstPath, Node, ParseSourceSpan, RecursiveTemplateAstVisitor, RecursiveVisitor, TemplateAst, TemplateAstPath, identifierName, templateVisitAll, visitAll} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {AstResult, SelectorInfo} from './common';
@@ -225,3 +225,27 @@ export function findPropertyValueOfType<T extends ts.Node>(
   }
   return startNode.forEachChild(c => findPropertyValueOfType(c, propName, predicate));
 }
+
+/**
+ * Find the tightest node at the specified `position` from the AST `nodes`, and
+ * return the path to the node.
+ * @param nodes HTML AST nodes
+ * @param position
+ */
+export function getPathToNodeAtPosition(nodes: Node[], position: number): HtmlAstPath {
+  const path: Node[] = [];
+  const visitor = new class extends RecursiveVisitor {
+    visit(ast: Node) {
+      const span = spanOf(ast);
+      if (inSpan(position, span)) {
+        path.push(ast);
+      } else {
+        // Returning a truthy value here will skip all children and terminate
+        // the visit.
+        return true;
+      }
+    }
+  };
+  visitAll(visitor, nodes);
+  return new AstPath<Node>(path, position);
+}

packages/language-service/test/utils_spec.ts

@@ -6,8 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import * as ng from '@angular/compiler';
 import * as ts from 'typescript';
-import {getDirectiveClassLike} from '../src/utils';
+
+import {getDirectiveClassLike, getPathToNodeAtPosition} from '../src/utils';
 
 describe('getDirectiveClassLike()', () => {
   it('should return a directive class', () => {
@@ -32,3 +34,49 @@ describe('getDirectiveClassLike()', () => {
     expect(classDecl.name !.text).toBe('AppModule');
   });
 });
+
+describe('getPathToNodeAtPosition', () => {
+  const html = '<div c></div>';
+  const nodes: ng.Node[] = [];
+
+  beforeAll(() => {
+    const parser = new ng.HtmlParser();
+    const {rootNodes, errors} = parser.parse(html, 'url');
+    expect(errors.length).toBe(0);
+    nodes.push(...rootNodes);
+  });
+
+  it('must capture element', () => {
+    // First, try to get a Path to the Element
+    // <|div c></div>
+    //  ^ cursor is here
+    const position = html.indexOf('div');
+    const path = getPathToNodeAtPosition(nodes, position);
+    // There should be just 1 node in the path, the Element node
+    expect(path.empty).toBe(false);
+    expect(path.head instanceof ng.Element).toBe(true);
+    expect(path.head).toBe(path.tail);
+  });
+
+  it('must capture attribute', () => {
+    // Then, try to get a Path to the Attribute
+    // <div |c></div>
+    //      ^ cusor is here, before the attribute
+    const position = html.indexOf('c');
+    const path = getPathToNodeAtPosition(nodes, position);
+    expect(path.empty).toBe(false);
+    expect(path.head instanceof ng.Element).toBe(true);
+    expect(path.tail instanceof ng.Attribute).toBe(true);
+  });
+
+  it('must capture attribute before cursor', () => {
+    // Finally, try to get a Path to the attribute after the 'c' text
+    // <div c|></div>
+    //       ^ cursor is here, after the attribute
+    const position = html.indexOf('c') + 1;
+    const path = getPathToNodeAtPosition(nodes, position);
+    expect(path.empty).toBe(false);
+    expect(path.head instanceof ng.Element).toBe(true);
+    expect(path.tail instanceof ng.Attribute).toBe(true);
+  });
+});