fix(language-service): Autocomplete block keywords in more cases (#52198)

dylhunn · dylhunn · commit 9d565cd6d682 · 2023-10-23T12:01:52.000-07:00
Previously, autocompletions were not available in two main cases. We correct them. 1. Autocompletions immediately after `@` were usually not working, for example `foo @|`. We fix this by causing the lexer to not consider the `@` part of the text node. 2. Autocompletions such as `@\nfoo`, where a newline follows a bare `@`, were not working because the language service visitor considered us inside the subsequent text node. We fix this by adding a block name span for the block keyword, and special-case whether we are completing inside the name span. If we are, we don't continue to the following text node. PR Close #52198
diff --git a/packages/compiler/src/ml_parser/ast.ts b/packages/compiler/src/ml_parser/ast.ts
@@ -89,8 +89,8 @@ export class Comment implements BaseNode {
 export class Block implements BaseNode {
   constructor(
       public name: string, public parameters: BlockParameter[], public children: Node[],
-      public sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan,
-      public endSourceSpan: ParseSourceSpan|null = null) {}
+      public sourceSpan: ParseSourceSpan, public nameSpan: ParseSourceSpan,
+      public startSourceSpan: ParseSourceSpan, public endSourceSpan: ParseSourceSpan|null = null) {}
 
   visit(visitor: Visitor, context: any) {
     return visitor.visitBlock(this, context);
diff --git a/packages/compiler/src/ml_parser/html_whitespaces.ts b/packages/compiler/src/ml_parser/html_whitespaces.ts
@@ -101,7 +101,7 @@ export class WhitespaceVisitor implements html.Visitor {
   visitBlock(block: html.Block, context: any): any {
     return new html.Block(
         block.name, block.parameters, visitAllWithSiblings(this, block.children), block.sourceSpan,
-        block.startSourceSpan, block.endSourceSpan);
+        block.nameSpan, block.startSourceSpan, block.endSourceSpan);
   }
 
   visitBlockParameter(parameter: html.BlockParameter, context: any) {
diff --git a/packages/compiler/src/ml_parser/icu_ast_expander.ts b/packages/compiler/src/ml_parser/icu_ast_expander.ts
@@ -91,7 +91,7 @@ class _Expander implements html.Visitor {
   visitBlock(block: html.Block, context: any) {
     return new html.Block(
         block.name, block.parameters, html.visitAll(this, block.children), block.sourceSpan,
-        block.startSourceSpan, block.endSourceSpan);
+        block.nameSpan, block.startSourceSpan, block.endSourceSpan);
   }
 
   visitBlockParameter(parameter: html.BlockParameter, context: any) {
diff --git a/packages/compiler/src/ml_parser/lexer.ts b/packages/compiler/src/ml_parser/lexer.ts
@@ -918,7 +918,7 @@ class _Tokenizer {
     }
 
     if (this._tokenizeBlocks && !this._inInterpolation && !this._isInExpansion() &&
-        (this._isBlockStart() || this._cursor.peek() === chars.$RBRACE)) {
+        (this._cursor.peek() === chars.$AT || this._cursor.peek() === chars.$RBRACE)) {
       return true;
     }
 
@@ -944,19 +944,6 @@ class _Tokenizer {
     return false;
   }
 
-  private _isBlockStart(): boolean {
-    if (this._tokenizeBlocks && this._cursor.peek() === chars.$AT) {
-      const tmp = this._cursor.clone();
-
-      // If it is, also verify that the next character is a valid block identifier.
-      tmp.advance();
-      if (isBlockNameChar(tmp.peek())) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   private _readUntil(char: number): string {
     const start = this._cursor.clone();
     this._attemptUntilChar(char);
diff --git a/packages/compiler/src/ml_parser/parser.ts b/packages/compiler/src/ml_parser/parser.ts
@@ -474,7 +474,7 @@ class _TreeBuilder {
     const span = new ParseSourceSpan(token.sourceSpan.start, end, token.sourceSpan.fullStart);
     // Create a separate `startSpan` because `span` will be modified when there is an `end` span.
     const startSpan = new ParseSourceSpan(token.sourceSpan.start, end, token.sourceSpan.fullStart);
-    const block = new html.Block(token.parts[0], parameters, [], span, startSpan);
+    const block = new html.Block(token.parts[0], parameters, [], span, token.sourceSpan, startSpan);
     this._pushContainer(block, false);
   }
 
@@ -500,7 +500,7 @@ class _TreeBuilder {
     const span = new ParseSourceSpan(token.sourceSpan.start, end, token.sourceSpan.fullStart);
     // Create a separate `startSpan` because `span` will be modified when there is an `end` span.
     const startSpan = new ParseSourceSpan(token.sourceSpan.start, end, token.sourceSpan.fullStart);
-    const block = new html.Block(token.parts[0], parameters, [], span, startSpan);
+    const block = new html.Block(token.parts[0], parameters, [], span, token.sourceSpan, startSpan);
     this._pushContainer(block, false);
 
     // Incomplete blocks don't have children so we close them immediately and report an error.
diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts
@@ -315,7 +315,8 @@ export class IfBlockBranch implements Node {
 }
 
 export class UnknownBlock implements Node {
-  constructor(public name: string, public sourceSpan: ParseSourceSpan) {}
+  constructor(
+      public name: string, public sourceSpan: ParseSourceSpan, public nameSpan: ParseSourceSpan) {}
 
   visit<Result>(visitor: Visitor<Result>): Result {
     return visitor.visitUnknownBlock(this);
diff --git a/packages/compiler/src/render3/r3_control_flow.ts b/packages/compiler/src/render3/r3_control_flow.ts
@@ -163,7 +163,7 @@ export function createSwitchBlock(
     }
 
     if ((node.name !== 'case' || node.parameters.length === 0) && node.name !== 'default') {
-      unknownBlocks.push(new t.UnknownBlock(node.name, node.sourceSpan));
+      unknownBlocks.push(new t.UnknownBlock(node.name, node.sourceSpan, node.nameSpan));
       continue;
     }
 
diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts
@@ -383,7 +383,7 @@ class HtmlAstToIvyAst implements html.Visitor {
         }
 
         result = {
-          node: new t.UnknownBlock(block.name, block.sourceSpan),
+          node: new t.UnknownBlock(block.name, block.sourceSpan, block.nameSpan),
           errors: [new ParseError(block.sourceSpan, errorMessage)],
         };
         break;
diff --git a/packages/compiler/test/ml_parser/lexer_spec.ts b/packages/compiler/test/ml_parser/lexer_spec.ts
diff --git a/packages/language-service/src/template_target.ts b/packages/language-service/src/template_target.ts
diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ export class WhitespaceVisitor implements html.Visitor {`
`101`	`101`	`visitBlock(block: html.Block, context: any): any {`
`102`	`102`	`return new html.Block(`
`103`	`103`	`block.name, block.parameters, visitAllWithSiblings(this, block.children), block.sourceSpan,`
`104`		`- block.startSourceSpan, block.endSourceSpan);`
	`104`	`+ block.nameSpan, block.startSourceSpan, block.endSourceSpan);`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`visitBlockParameter(parameter: html.BlockParameter, context: any) {`
Original file line number	Diff line number	Diff line change
`@@ -91,7 +91,7 @@ class _Expander implements html.Visitor {`
`91`	`91`	`visitBlock(block: html.Block, context: any) {`
`92`	`92`	`return new html.Block(`
`93`	`93`	`block.name, block.parameters, html.visitAll(this, block.children), block.sourceSpan,`
`94`		`- block.startSourceSpan, block.endSourceSpan);`
	`94`	`+ block.nameSpan, block.startSourceSpan, block.endSourceSpan);`
`95`	`95`	`}`
`96`	`96`
`97`	`97`	`visitBlockParameter(parameter: html.BlockParameter, context: any) {`
Original file line number	Diff line number	Diff line change
`@@ -315,7 +315,8 @@ export class IfBlockBranch implements Node {`
`315`	`315`	`}`
`316`	`316`
`317`	`317`	`export class UnknownBlock implements Node {`
`318`		`- constructor(public name: string, public sourceSpan: ParseSourceSpan) {}`
	`318`	`+ constructor(`
	`319`	`+ public name: string, public sourceSpan: ParseSourceSpan, public nameSpan: ParseSourceSpan) {}`
`319`	`320`
`320`	`321`	`visit<Result>(visitor: Visitor<Result>): Result {`
`321`	`322`	`return visitor.visitUnknownBlock(this);`
Original file line number	Diff line number	Diff line change
`@@ -163,7 +163,7 @@ export function createSwitchBlock(`
`163`	`163`	`}`
`164`	`164`
`165`	`165`	`if ((node.name !== 'case' \|\| node.parameters.length === 0) && node.name !== 'default') {`
`166`		`- unknownBlocks.push(new t.UnknownBlock(node.name, node.sourceSpan));`
	`166`	`+ unknownBlocks.push(new t.UnknownBlock(node.name, node.sourceSpan, node.nameSpan));`
`167`	`167`	`continue;`
`168`	`168`	`}`
`169`	`169`
Original file line number	Diff line number	Diff line change
`@@ -383,7 +383,7 @@ class HtmlAstToIvyAst implements html.Visitor {`
`383`	`383`	`}`
`384`	`384`
`385`	`385`	`result = {`
`386`		`- node: new t.UnknownBlock(block.name, block.sourceSpan),`
	`386`	`+ node: new t.UnknownBlock(block.name, block.sourceSpan, block.nameSpan),`
`387`	`387`	`errors: [new ParseError(block.sourceSpan, errorMessage)],`
`388`	`388`	`};`
`389`	`389`	`break;`