fix(extension): Allow requests to reach the language server when in component decorator (#2067)

This commit updates the early-return logic on the client-side to allow
requests to go to the server whenever the cursor is inside the component
decorator. Prior to this change, some requests would attempt to return
early if the position was not inside the inline template region.
However, we were never able to get the tokenizing working correctly and
this has led to issues whenever there are multiple template strings
(JS template strings with \`).

This approach should give us the needed benefits of early-returns in
non-Angular contexts (`@Component` decorator should be pretty rare) while
being more permissive and correct.

fixes #2064

Author: atscott

client/src/client.ts

@@ -15,7 +15,7 @@ import {OpenOutputChannel, ProjectLoadingFinish, ProjectLoadingStart, SuggestStr
 import {GetComponentsWithTemplateFile, GetTcbRequest, GetTemplateLocationForComponent, IsInAngularProject} from '../../common/requests';
 import {NodeModule, resolve} from '../../common/resolver';
 
-import {isInsideComponentDecorator, isInsideInlineTemplateRegion, isInsideStringLiteral} from './embedded_support';
+import {isInsideStringLiteral, isNotTypescriptOrInsideComponentDecorator} from './embedded_support';
 
 interface GetTcbResponse {
   uri: vscode.Uri;
@@ -67,8 +67,8 @@ export class AngularLanguageClient implements vscode.Disposable {
             document: vscode.TextDocument, range: vscode.Range, context: vscode.CodeActionContext,
             token: vscode.CancellationToken, next: lsp.ProvideCodeActionsSignature) => {
           if (await this.isInAngularProject(document) &&
-              isInsideInlineTemplateRegion(document, range.start) &&
-              isInsideInlineTemplateRegion(document, range.end)) {
+              isNotTypescriptOrInsideComponentDecorator(document, range.start) &&
+              isNotTypescriptOrInsideComponentDecorator(document, range.end)) {
             return next(document, range, context, token);
           }
         },
@@ -92,23 +92,23 @@ export class AngularLanguageClient implements vscode.Disposable {
             document: vscode.TextDocument, position: vscode.Position,
             token: vscode.CancellationToken, next: lsp.ProvideDefinitionSignature) => {
           if (await this.isInAngularProject(document) &&
-              isInsideComponentDecorator(document, position)) {
+              isNotTypescriptOrInsideComponentDecorator(document, position)) {
             return next(document, position, token);
           }
         },
         provideTypeDefinition: async (
             document: vscode.TextDocument, position: vscode.Position,
             token: vscode.CancellationToken, next) => {
           if (await this.isInAngularProject(document) &&
-              isInsideInlineTemplateRegion(document, position)) {
+              isNotTypescriptOrInsideComponentDecorator(document, position)) {
             return next(document, position, token);
           }
         },
         provideHover: async (
             document: vscode.TextDocument, position: vscode.Position,
             token: vscode.CancellationToken, next: lsp.ProvideHoverSignature) => {
           if (!(await this.isInAngularProject(document)) ||
-              !isInsideInlineTemplateRegion(document, position)) {
+              !isNotTypescriptOrInsideComponentDecorator(document, position)) {
             return;
           }
 
@@ -132,7 +132,7 @@ export class AngularLanguageClient implements vscode.Disposable {
             context: vscode.SignatureHelpContext, token: vscode.CancellationToken,
             next: lsp.ProvideSignatureHelpSignature) => {
           if (await this.isInAngularProject(document) &&
-              isInsideInlineTemplateRegion(document, position)) {
+              isNotTypescriptOrInsideComponentDecorator(document, position)) {
             return next(document, position, context, token);
           }
         },
@@ -142,7 +142,7 @@ export class AngularLanguageClient implements vscode.Disposable {
             next: lsp.ProvideCompletionItemsSignature) => {
           // If not in inline template, do not perform request forwarding
           if (!(await this.isInAngularProject(document)) ||
-              !isInsideInlineTemplateRegion(document, position)) {
+              !isNotTypescriptOrInsideComponentDecorator(document, position)) {
             return;
           }
           const angularCompletionsPromise = next(document, position, context, token) as

client/src/embedded_support.ts

@@ -8,18 +8,8 @@
 import * as ts from 'typescript';
 import * as vscode from 'vscode';
 
-/** Determines if the position is inside an inline template. */
-export function isInsideInlineTemplateRegion(
-    document: vscode.TextDocument, position: vscode.Position): boolean {
-  if (document.languageId !== 'typescript') {
-    return true;
-  }
-  return isPropertyAssignmentToStringOrStringInArray(
-      document.getText(), document.offsetAt(position), ['template']);
-}
-
 /** Determines if the position is inside an inline template, templateUrl, or string in styleUrls. */
-export function isInsideComponentDecorator(
+export function isNotTypescriptOrInsideComponentDecorator(
     document: vscode.TextDocument, position: vscode.Position): boolean {
   if (document.languageId !== 'typescript') {
     return true;

client/src/tests/embedded_support_spec.ts

@@ -8,96 +8,66 @@
 import * as vscode from 'vscode';
 import {DocumentUri, TextDocument} from 'vscode-languageserver-textdocument';
 
-import {isInsideComponentDecorator, isInsideInlineTemplateRegion} from '../embedded_support';
+import {isNotTypescriptOrInsideComponentDecorator} from '../embedded_support';
 
 describe('embedded language support', () => {
-  describe('isInsideInlineTemplateRegion', () => {
-    it('empty file', () => {
-      test('¦', isInsideInlineTemplateRegion, false);
-    });
-
-    it('just after template', () => {
-      test(`const foo = {template: '<div></div>'¦}`, isInsideInlineTemplateRegion, false);
-    });
-
-    it('just before template', () => {
-      // Note that while it seems that this should be `false`, we should still consider this inside
-      // the string because the visual mode of vim appears to have a position on top of the open
-      // quote while the cursor position is before it.
-      test(`const foo = {template: ¦'<div></div>'}`, isInsideInlineTemplateRegion, true);
-    });
-
-    it('two spaces before template', () => {
-      test(`const foo = {template:¦ '<div></div>'}`, isInsideInlineTemplateRegion, false);
-    });
-
-    it('at beginning of template', () => {
-      test(`const foo = {template: '¦<div></div>'}`, isInsideInlineTemplateRegion, true);
-    });
-
-    it('at end of template', () => {
-      test(`const foo = {template: '<div></div>¦'}`, isInsideInlineTemplateRegion, true);
-    });
-
-    xit('works for inline templates after a template string', () => {
-      test(
-          'const x = `${""}`;\n' +
-              'const foo = {template: `hello ¦world`}',
-          isInsideInlineTemplateRegion, true);
-    });
-
-    it('works for inline templates after a tagged template string inside tagged template string',
-       () => {
-         test(
-             'const x = `${`${""}`}`;\n' +
-                 'const foo = {template: `hello ¦world`}',
-             isInsideInlineTemplateRegion, true);
-       });
-  });
-
   describe('isInsideAngularContext', () => {
     it('empty file', () => {
-      test('¦', isInsideComponentDecorator, false);
+      test('¦', isNotTypescriptOrInsideComponentDecorator, false);
     });
 
     it('just after template', () => {
-      test(`const foo = {template: '<div></div>'¦}`, isInsideComponentDecorator, false);
+      test(
+          `const foo = {template: '<div></div>'¦}`, isNotTypescriptOrInsideComponentDecorator,
+          false);
     });
 
     it('inside template', () => {
-      test(`const foo = {template: '<div>¦</div>'}`, isInsideComponentDecorator, true);
+      test(
+          `const foo = {template: '<div>¦</div>'}`, isNotTypescriptOrInsideComponentDecorator,
+          true);
     });
 
     it('just after templateUrl', () => {
-      test(`const foo = {templateUrl: './abc.html'¦}`, isInsideComponentDecorator, false);
+      test(
+          `const foo = {templateUrl: './abc.html'¦}`, isNotTypescriptOrInsideComponentDecorator,
+          false);
     });
 
     it('inside templateUrl', () => {
-      test(`const foo = {templateUrl: './abc¦.html'}`, isInsideComponentDecorator, true);
+      test(
+          `const foo = {templateUrl: './abc¦.html'}`, isNotTypescriptOrInsideComponentDecorator,
+          true);
     });
 
     it('just after styleUrls', () => {
-      test(`const foo = {styleUrls: ['./abc.css']¦}`, isInsideComponentDecorator, false);
+      test(
+          `const foo = {styleUrls: ['./abc.css']¦}`, isNotTypescriptOrInsideComponentDecorator,
+          false);
     });
 
     it('inside first item of styleUrls', () => {
-      test(`const foo = {styleUrls: ['./abc.c¦ss', 'def.css']}`, isInsideComponentDecorator, true);
+      test(
+          `const foo = {styleUrls: ['./abc.c¦ss', 'def.css']}`,
+          isNotTypescriptOrInsideComponentDecorator, true);
     });
 
     it('inside second item of styleUrls', () => {
-      test(`const foo = {styleUrls: ['./abc.css', 'def¦.css']}`, isInsideComponentDecorator, true);
+      test(
+          `const foo = {styleUrls: ['./abc.css', 'def¦.css']}`,
+          isNotTypescriptOrInsideComponentDecorator, true);
     });
 
     it('inside second item of styleUrls, when first is complicated function', () => {
       test(
           `const foo = {styleUrls: [getCss({strict: true, dirs: ['apple', 'banana']}), 'def¦.css']}`,
-          isInsideComponentDecorator, true);
+          isNotTypescriptOrInsideComponentDecorator, true);
     });
 
     it('inside non-string item of styleUrls', () => {
       test(
           `const foo = {styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']}`,
-          isInsideComponentDecorator, false);
+          isNotTypescriptOrInsideComponentDecorator, false);
     });
   });
 });
@@ -129,4 +99,4 @@ function extractCursorInfo(textWithCursor: string): {cursor: number, text: strin
     cursor,
     text: textWithCursor.substr(0, cursor) + textWithCursor.substr(cursor + 1),
   };
-}
+}

integration/project/app/foo.component.html

@@ -1,5 +1,7 @@
 {{title | uppercase}}
 <span class="subtitle">
     subtitle
+    {{sig()}}
+    {{x.sig()}}
 </span>
 <lib-post></lib-post>

integration/project/app/foo.component.ts

@@ -1,8 +1,16 @@
-import { Component } from '@angular/core';
+import { Component, signal } from '@angular/core';
 
 @Component({
   templateUrl: 'foo.component.html',
 })
 export class FooComponent {
   title = 'Foo Component';
+  sig = signal(1);
+  x = {
+    sig: signal(1),
+  }
+  /** returns 1 */
+  method() {
+    return 1;
+  }
 }