refactor(client): use SourceFile to detect the Angular context in t…

…he client (#2027) When using the typescript's scanner to parse the template literals which includes an expression, knowing the right `CloseBraceToken` of the expression to invoke the `reScanTemplateToken` function is a little complex. If don't do this, the rest of the file is temporarily "poisoned". There is an explanation for why typescript's fast-acting lexical classifications don't work for template sting across lines. microsoft/TypeScript#1477 (comment) https://github.com/microsoft/TypeScript/blob/a4d12a46c8b413c65a730c4ad0323459f1fc44ce/src/services/classifier.ts#L114 So this PR uses the `SourceFile` to detect the Angular context in the client.
angular · Apr 29, 2024 · 13d9776 · 13d9776
1 parent 51d907e
commit 13d9776
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 97 deletions.
diff --git a/client/src/embedded_support.ts b/client/src/embedded_support.ts
@@ -14,8 +14,13 @@ export function isInsideInlineTemplateRegion(
   if (document.languageId !== 'typescript') {
     return true;
   }
-  return isPropertyAssignmentToStringOrStringInArray(
-      document.getText(), document.offsetAt(position), ['template']);
+  const node = getNodeAtDocumentPosition(document, position);
+
+  if (!node) {
+    return false;
+  }
+
+  return getPropertyAssignmentFromValue(node, 'template') !== null;
 }
 
 /** Determines if the position is inside an inline template, templateUrl, or string in styleUrls. */
@@ -24,102 +29,94 @@ export function isInsideComponentDecorator(
   if (document.languageId !== 'typescript') {
     return true;
   }
-  return isPropertyAssignmentToStringOrStringInArray(
-      document.getText(), document.offsetAt(position),
-      ['template', 'templateUrl', 'styleUrls', 'styleUrl']);
+
+  const node = getNodeAtDocumentPosition(document, position);
+  if (!node) {
+    return false;
+  }
+  const assignment = getPropertyAssignmentFromValue(node, 'template') ??
+      getPropertyAssignmentFromValue(node, 'templateUrl') ??
+      // `node.parent` is used because the string is a child of an array element and we want to get
+      // the property name
+      getPropertyAssignmentFromValue(node.parent, 'styleUrls') ??
+      getPropertyAssignmentFromValue(node, 'styleUrl');
+  return assignment !== null;
 }
 
 /**
- * Determines if the position is inside a string literal. Returns `true` if the document language is
- * not TypeScript.
+ * Determines if the position is inside a string literal. Returns `true` if the document language
+ * is not TypeScript.
  */
 export function isInsideStringLiteral(
     document: vscode.TextDocument, position: vscode.Position): boolean {
   if (document.languageId !== 'typescript') {
     return true;
   }
-  const offset = document.offsetAt(position);
-  const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
-  scanner.setText(document.getText());
-
-  let token: ts.SyntaxKind = scanner.scan();
-  while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
-    const isStringToken = token === ts.SyntaxKind.StringLiteral ||
-        token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
-    const isCursorInToken = scanner.getStartPos() <= offset &&
-        scanner.getStartPos() + scanner.getTokenText().length >= offset;
-    if (isCursorInToken && isStringToken) {
-      return true;
-    }
-    token = scanner.scan();
+  const node = getNodeAtDocumentPosition(document, position);
+
+  if (!node) {
+    return false;
   }
-  return false;
+
+  return ts.isStringLiteralLike(node);
 }
 
 /**
- * Basic scanner to determine if we're inside a string of a property with one of the given names.
- *
- * This scanner is not currently robust or perfect but provides us with an accurate answer _most_ of
- * the time.
- *
- * False positives are OK here. Though this will give some false positives for determining if a
- * position is within an Angular context, i.e. an object like `{template: ''}` that is not inside an
- * `@Component` or `{styleUrls: [someFunction('stringL¦iteral')]}`, the @angular/language-service
- * will always give us the correct answer. This helper gives us a quick win for optimizing the
- * number of requests we send to the server.
- *
- * TODO(atscott): tagged templates don't work: #1872 /
- * https://github.com/Microsoft/TypeScript/issues/20055
+ * Return the node that most tightly encompasses the specified `position`.
+ * @param node The starting node to start the top-down search.
+ * @param position The target position within the `node`.
  */
-function isPropertyAssignmentToStringOrStringInArray(
-    documentText: string, offset: number, propertyAssignmentNames: string[]): boolean {
-  const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
-  scanner.setText(documentText);
-
-  let token: ts.SyntaxKind = scanner.scan();
-  let lastToken: ts.SyntaxKind|undefined;
-  let lastTokenText: string|undefined;
-  let unclosedBraces = 0;
-  let unclosedBrackets = 0;
-  let propertyAssignmentContext = false;
-  while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
-    if (lastToken === ts.SyntaxKind.Identifier && lastTokenText !== undefined &&
-        propertyAssignmentNames.includes(lastTokenText) && token === ts.SyntaxKind.ColonToken) {
-      propertyAssignmentContext = true;
-      token = scanner.scan();
-      continue;
-    }
-    if (unclosedBraces === 0 && unclosedBrackets === 0 && isPropertyAssignmentTerminator(token)) {
-      propertyAssignmentContext = false;
-    }
-
-    if (token === ts.SyntaxKind.OpenBracketToken) {
-      unclosedBrackets++;
-    } else if (token === ts.SyntaxKind.OpenBraceToken) {
-      unclosedBraces++;
-    } else if (token === ts.SyntaxKind.CloseBracketToken) {
-      unclosedBrackets--;
-    } else if (token === ts.SyntaxKind.CloseBraceToken) {
-      unclosedBraces--;
-    }
-
-    const isStringToken = token === ts.SyntaxKind.StringLiteral ||
-        token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
-    const isCursorInToken = scanner.getStartPos() <= offset &&
-        scanner.getStartPos() + scanner.getTokenText().length >= offset;
-    if (propertyAssignmentContext && isCursorInToken && isStringToken) {
-      return true;
-    }
-
-    lastTokenText = scanner.getTokenText();
-    lastToken = token;
-    token = scanner.scan();
+function findTightestNodeAtPosition(node: ts.Node, position: number): ts.Node|undefined {
+  if (node.getStart() <= position && position < node.getEnd()) {
+    return node.forEachChild(c => findTightestNodeAtPosition(c, position)) ?? node;
   }
+  return undefined;
+}
 
-  return false;
+/**
+ * Returns a property assignment from the assignment value if the property name
+ * matches the specified `key`, or `null` if there is no match.
+ */
+function getPropertyAssignmentFromValue(value: ts.Node, key: string): ts.PropertyAssignment|null {
+  const propAssignment = value.parent;
+  if (!propAssignment || !ts.isPropertyAssignment(propAssignment) ||
+      propAssignment.name.getText() !== key) {
+    return null;
+  }
+  return propAssignment;
 }
 
-function isPropertyAssignmentTerminator(token: ts.SyntaxKind) {
-  return token === ts.SyntaxKind.EndOfFileToken || token === ts.SyntaxKind.CommaToken ||
-      token === ts.SyntaxKind.SemicolonToken || token === ts.SyntaxKind.CloseBraceToken;
+type NgLSClientSourceFile = ts.SourceFile&{[NgLSClientSourceFileVersion]: number};
+
+/**
+ * The `TextDocument` is not extensible, so the `WeakMap` is used here.
+ */
+const ngLSClientSourceFileMap = new WeakMap<vscode.TextDocument, NgLSClientSourceFile>();
+const NgLSClientSourceFileVersion = Symbol('NgLSClientSourceFileVersion');
+
+/**
+ *
+ * Parse the document to `SourceFile` and return the node at the document position.
+ */
+function getNodeAtDocumentPosition(
+    document: vscode.TextDocument, position: vscode.Position): ts.Node|undefined {
+  const offset = document.offsetAt(position);
+
+  let sourceFile = ngLSClientSourceFileMap.get(document);
+  if (!sourceFile || sourceFile[NgLSClientSourceFileVersion] !== document.version) {
+    sourceFile =
+        ts.createSourceFile(
+            document.fileName, document.getText(), {
+              languageVersion: ts.ScriptTarget.ESNext,
+              jsDocParsingMode: ts.JSDocParsingMode.ParseNone,
+            },
+            /** setParentNodes */
+            true /** If not set, the `findTightestNodeAtPosition` will throw an error */) as
+        NgLSClientSourceFile;
+    sourceFile[NgLSClientSourceFileVersion] = document.version;
+
+    ngLSClientSourceFileMap.set(document, sourceFile);
+  }
+
+  return findTightestNodeAtPosition(sourceFile, offset);
 }
diff --git a/client/src/tests/embedded_support_spec.ts b/client/src/tests/embedded_support_spec.ts
@@ -17,27 +17,42 @@ describe('embedded language support', () => {
     });
 
     it('just after template', () => {
-      test(`template: '<div></div>'¦`, isInsideInlineTemplateRegion, false);
+      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(`template: ¦'<div></div>'`, isInsideInlineTemplateRegion, true);
+      test(`const foo = {template: ¦'<div></div>'}`, isInsideInlineTemplateRegion, true);
     });
 
     it('two spaces before template', () => {
-      test(`template:¦ '<div></div>'`, isInsideInlineTemplateRegion, false);
+      test(`const foo = {template:¦ '<div></div>'}`, isInsideInlineTemplateRegion, false);
     });
 
     it('at beginning of template', () => {
-      test(`template: '¦<div></div>'`, isInsideInlineTemplateRegion, true);
+      test(`const foo = {template: '¦<div></div>'}`, isInsideInlineTemplateRegion, true);
     });
 
     it('at end of template', () => {
-      test(`template: '<div></div>¦'`, isInsideInlineTemplateRegion, true);
+      test(`const foo = {template: '<div></div>¦'}`, isInsideInlineTemplateRegion, true);
     });
+
+    it('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', () => {
@@ -46,42 +61,42 @@ describe('embedded language support', () => {
     });
 
     it('just after template', () => {
-      test(`template: '<div></div>'¦`, isInsideComponentDecorator, false);
+      test(`const foo = {template: '<div></div>'¦}`, isInsideComponentDecorator, false);
     });
 
     it('inside template', () => {
-      test(`template: '<div>¦</div>'`, isInsideComponentDecorator, true);
+      test(`const foo = {template: '<div>¦</div>'}`, isInsideComponentDecorator, true);
     });
 
     it('just after templateUrl', () => {
-      test(`templateUrl: './abc.html'¦`, isInsideComponentDecorator, false);
+      test(`const foo = {templateUrl: './abc.html'¦}`, isInsideComponentDecorator, false);
     });
 
     it('inside templateUrl', () => {
-      test(`templateUrl: './abc¦.html'`, isInsideComponentDecorator, true);
+      test(`const foo = {templateUrl: './abc¦.html'}`, isInsideComponentDecorator, true);
     });
 
     it('just after styleUrls', () => {
-      test(`styleUrls: ['./abc.css']¦`, isInsideComponentDecorator, false);
+      test(`const foo = {styleUrls: ['./abc.css']¦}`, isInsideComponentDecorator, false);
     });
 
     it('inside first item of styleUrls', () => {
-      test(`styleUrls: ['./abc.c¦ss', 'def.css']`, isInsideComponentDecorator, true);
+      test(`const foo = {styleUrls: ['./abc.c¦ss', 'def.css']}`, isInsideComponentDecorator, true);
     });
 
     it('inside second item of styleUrls', () => {
-      test(`styleUrls: ['./abc.css', 'def¦.css']`, isInsideComponentDecorator, true);
+      test(`const foo = {styleUrls: ['./abc.css', 'def¦.css']}`, isInsideComponentDecorator, true);
     });
 
     it('inside second item of styleUrls, when first is complicated function', () => {
       test(
-          `styleUrls: [getCss({strict: true, dirs: ['apple', 'banana']}), 'def¦.css']`,
+          `const foo = {styleUrls: [getCss({strict: true, dirs: ['apple', 'banana']}), 'def¦.css']}`,
           isInsideComponentDecorator, true);
     });
 
     it('inside non-string item of styleUrls', () => {
       test(
-          `styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']`,
+          `const foo = {styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']}`,
           isInsideComponentDecorator, false);
     });
   });
@@ -92,8 +107,10 @@ function test(
     testFn: (doc: vscode.TextDocument, position: vscode.Position) => boolean,
     expectation: boolean): void {
   const {cursor, text} = extractCursorInfo(fileWithCursor);
-  const vdoc = TextDocument.create('test' as DocumentUri, 'typescript', 0, text) as {} as
+
+  const vdoc = TextDocument.create('test.ts' as DocumentUri, 'typescript', 0, text) as {} as
       vscode.TextDocument;
+  (vdoc as any).fileName = 'test.ts';
   const actual = testFn(vdoc, vdoc.positionAt(cursor));
   expect(actual).toBe(expectation);
 }