From 228beeabd1542941cbfff46ad78d52c1df3d283a Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 10 Jun 2021 15:59:44 -0700 Subject: [PATCH] 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 --- .../language-service/ivy/template_target.ts | 14 ++++++-- .../ivy/test/legacy/template_target_spec.ts | 36 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/packages/language-service/ivy/template_target.ts b/packages/language-service/ivy/template_target.ts index f4d760c63954e..ea14bfccc6f57 100644 --- a/packages/language-service/ivy/template_target.ts +++ b/packages/language-service/ivy/template_target.ts @@ -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 // 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; } 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 6b8e911f5758e..b33535ccadec4 100644 --- a/packages/language-service/ivy/test/legacy/template_target_spec.ts +++ b/packages/language-service/ivy/test/legacy/template_target_spec.ts @@ -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(`
{{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(`
  • {{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(`
  • {{b¦ar}}
  • `); + 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(`
  • {{bar}}`); + 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'); + }); +});