fix(language-service): HTML path should include last node before cursor (#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 #34440
This commit is contained in:
Keen Yee Liau 2019-12-16 12:11:39 -08:00 committed by Kara Erickson
parent 28b4f4abce
commit 5df8a3ba95
5 changed files with 83 additions and 10 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 {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 {$$, $_, isAsciiLetter, isDigit} from '@angular/compiler/src/chars';
import {AstResult} from './common'; import {AstResult} from './common';
@ -15,7 +15,7 @@ import {getExpressionCompletions} from './expressions';
import {attributeNames, elementNames, eventNames, propertyNames} from './html_info'; import {attributeNames, elementNames, eventNames, propertyNames} from './html_info';
import {InlineTemplate} from './template'; import {InlineTemplate} from './template';
import * as ng from './types'; 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> = const HIDDEN_HTML_ELEMENTS: ReadonlySet<string> =
new Set(['html', 'script', 'noscript', 'base', 'body', 'title', 'head', 'link']); new Set(['html', 'script', 'noscript', 'base', 'body', 'title', 'head', 'link']);
@ -121,7 +121,7 @@ export function getTemplateCompletions(
const {htmlAst, template} = templateInfo; const {htmlAst, template} = templateInfo;
// The templateNode starts at the delimiter character so we add 1 to skip it. // The templateNode starts at the delimiter character so we add 1 to skip it.
const templatePosition = position - template.span.start; const templatePosition = position - template.span.start;
const path = findNode(htmlAst, templatePosition); const path = getPathToNodeAtPosition(htmlAst, templatePosition);
const mostSpecific = path.tail; const mostSpecific = path.tail;
if (path.empty || !mostSpecific) { if (path.empty || !mostSpecific) {
result = elementCompletions(templateInfo); result = elementCompletions(templateInfo);

View File

@ -6,12 +6,13 @@
* found in the LICENSE file at https://angular.io/license * 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 * as ts from 'typescript';
import {AstType, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type'; import {AstType, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type';
import {BuiltinType, Definition, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './symbols'; import {BuiltinType, Definition, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './symbols';
import {Diagnostic} from './types'; import {Diagnostic} from './types';
import {getPathToNodeAtPosition} from './utils';
export interface DiagnosticTemplateInfo { export interface DiagnosticTemplateInfo {
fileName?: string; fileName?: string;
@ -304,7 +305,7 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
} }
private attributeValueLocation(ast: TemplateAst) { 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; const last = path.tail;
if (last instanceof Attribute && last.valueSpan) { if (last instanceof Attribute && last.valueSpan) {
return last.valueSpan.start.offset; return last.valueSpan.start.offset;

View File

@ -6,13 +6,13 @@
* found in the LICENSE file at https://angular.io/license * 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 {AstResult} from './common';
import {getExpressionScope} from './expression_diagnostics'; import {getExpressionScope} from './expression_diagnostics';
import {getExpressionSymbol} from './expressions'; import {getExpressionSymbol} from './expressions';
import {Definition, DirectiveKind, Span, Symbol} from './types'; 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 { export interface SymbolInfo {
symbol: Symbol; symbol: Symbol;
@ -144,7 +144,7 @@ export function locateSymbol(info: AstResult, position: number): SymbolInfo|unde
function findAttribute(info: AstResult, position: number): Attribute|undefined { function findAttribute(info: AstResult, position: number): Attribute|undefined {
const templatePosition = position - info.template.span.start; const templatePosition = position - info.template.span.start;
const path = findNode(info.htmlAst, templatePosition); const path = getPathToNodeAtPosition(info.htmlAst, templatePosition);
return path.first(Attribute); return path.first(Attribute);
} }

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 {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 * as ts from 'typescript';
import {AstResult, SelectorInfo} from './common'; import {AstResult, SelectorInfo} from './common';
@ -225,3 +225,27 @@ export function findPropertyValueOfType<T extends ts.Node>(
} }
return startNode.forEachChild(c => findPropertyValueOfType(c, propName, predicate)); 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);
}

View File

@ -6,8 +6,10 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import * as ng from '@angular/compiler';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {getDirectiveClassLike} from '../src/utils';
import {getDirectiveClassLike, getPathToNodeAtPosition} from '../src/utils';
describe('getDirectiveClassLike()', () => { describe('getDirectiveClassLike()', () => {
it('should return a directive class', () => { it('should return a directive class', () => {
@ -32,3 +34,49 @@ describe('getDirectiveClassLike()', () => {
expect(classDecl.name !.text).toBe('AppModule'); 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);
});
});