fix(language-service): Use last child end span for parent without close tag (#42554)
Unclosed element tags are not assigned an `endSourceSpan` by the parser. As a result, the visitor which determines the target node at a position for the language service was unable to determine that a given position was inside an unclosed parent. This happens because we update the `endSourceSpan` of template/element nodes to be the end tag (and there is not one for unclosed tags). Consequently, the visitor then cannot match a position to any child node location. This change updates the visitor logic to check if there are any `children` of a template/element node and updates the end span to be the end span of the last child. This allows our `isWithin` logic to identify that a child position is within the unclosed parent. Addresses one of the issues found during investigation of https://github.com/angular/vscode-ng-language-service/issues/1399 PR Close #42554
This commit is contained in:
parent
8c1e0e6ad0
commit
228beeabd1
|
@ -286,7 +286,7 @@ class TemplateTargetVisitor implements t.Visitor {
|
|||
|
||||
visit(node: t.Node) {
|
||||
const {start, end} = getSpanIncludingEndTag(node);
|
||||
if (!isWithin(this.position, {start, end})) {
|
||||
if (end !== null && !isWithin(this.position, {start, end})) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
@ -441,8 +441,16 @@ function getSpanIncludingEndTag(ast: t.Node) {
|
|||
// the end of the closing tag. Otherwise, for situation like
|
||||
// <my-component></my-comp¦onent> where the cursor is in the closing tag
|
||||
// we will not be able to return any information.
|
||||
if ((ast instanceof t.Element || ast instanceof t.Template) && ast.endSourceSpan) {
|
||||
result.end = ast.endSourceSpan.end.offset;
|
||||
if (ast instanceof t.Element || ast instanceof t.Template) {
|
||||
if (ast.endSourceSpan) {
|
||||
result.end = ast.endSourceSpan.end.offset;
|
||||
} else if (ast.children.length > 0) {
|
||||
// If the AST has children but no end source span, then it is an unclosed element with an end
|
||||
// that should be the end of the last child.
|
||||
result.end = getSpanIncludingEndTag(ast.children[ast.children.length - 1]).end;
|
||||
} else {
|
||||
// This is likely a self-closing tag with no children so the `sourceSpan.end` is correct.
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
|
|
@ -797,3 +797,39 @@ describe('findNodeAtPosition for microsyntax expression', () => {
|
|||
expect((context as SingleNodeTarget).node).toBeInstanceOf(t.Element);
|
||||
});
|
||||
});
|
||||
|
||||
describe('unclosed elements', () => {
|
||||
it('should locate children of unclosed elements', () => {
|
||||
const {errors, nodes, position} = parse(`<div> {{b¦ar}}`);
|
||||
expect(errors).toBe(null);
|
||||
const {context} = getTargetAtPosition(nodes, position)!;
|
||||
const {node} = context as SingleNodeTarget;
|
||||
expect(isExpressionNode(node!)).toBe(true);
|
||||
expect(node).toBeInstanceOf(e.PropertyRead);
|
||||
});
|
||||
|
||||
it('should locate children of outside of unclosed when parent is closed elements', () => {
|
||||
const {nodes, position} = parse(`<li><div></li> {{b¦ar}}`);
|
||||
const {context} = getTargetAtPosition(nodes, position)!;
|
||||
const {node} = context as SingleNodeTarget;
|
||||
expect(isExpressionNode(node!)).toBe(true);
|
||||
expect(node).toBeInstanceOf(e.PropertyRead);
|
||||
});
|
||||
|
||||
it('should locate nodes before unclosed element', () => {
|
||||
const {nodes, position} = parse(`<li>{{b¦ar}}<div></li>`);
|
||||
const {context} = getTargetAtPosition(nodes, position)!;
|
||||
const {node} = context as SingleNodeTarget;
|
||||
expect(isExpressionNode(node!)).toBe(true);
|
||||
expect(node).toBeInstanceOf(e.PropertyRead);
|
||||
});
|
||||
|
||||
it('should be correct for end tag of parent node with unclosed child', () => {
|
||||
const {nodes, position} = parse(`<li><div><div>{{bar}}</l¦i>`);
|
||||
const {context} = getTargetAtPosition(nodes, position)!;
|
||||
const {node} = context as SingleNodeTarget;
|
||||
expect(isTemplateNode(node!)).toBe(true);
|
||||
expect(node).toBeInstanceOf(t.Element);
|
||||
expect((node as t.Element).name).toBe('li');
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue