fix(language-service): Correctly parse inputs and selectors with dollar signs (angular#44268)

When we are going to the definition of an input, we find _both_ the
definition of the input _and_ also look for any directives which have
a selector that matches the input. For example:

```
@directive({
  selector: '[greeting]'
})
export class MyDir {
  @input() greeting!: string;
}
```

With this commit, we now correctly handle the case where inputs and/or
selectors have a dollar sign in them. The dollar sign has special
meaning in CSS, but when we encounter the dollar in a template, we need
to escape it when used as a selector so that it is taken as a dollar
literal rather than a character with special meaning.

Previously, we were not escaping the dollar sign and the CSS parsing
logic would throw an error. The change in this commit prevents that
error from happening, but a `try...catch` is still added in case there
is another unhandled use-case. If this happens, we do not want the
`goToDefinition` operation to completely fail.

Fixes angular/vscode-ng-language-service#1574

PR Close angular#44268

Author: atscott

packages/language-service/src/utils.ts

@@ -167,14 +167,19 @@ function getFirstComponentForTemplateFile(fileName: string, compiler: NgCompiler
 }
 
 /**
- * Given an attribute node, converts it to string form.
+ * Given an attribute node, converts it to string form for use as a CSS selector.
  */
-function toAttributeString(attribute: t.TextAttribute|t.BoundAttribute|t.BoundEvent): string {
+function toAttributeCssSelector(attribute: t.TextAttribute|t.BoundAttribute|t.BoundEvent): string {
+  let selector: string;
   if (attribute instanceof t.BoundEvent || attribute instanceof t.BoundAttribute) {
-    return `[${attribute.name}]`;
+    selector = `[${attribute.name}]`;
   } else {
-    return `[${attribute.name}=${attribute.valueSpan?.toString() ?? ''}]`;
+    selector = `[${attribute.name}=${attribute.valueSpan?.toString() ?? ''}]`;
   }
+  // Any dollar signs that appear in the attribute name and/or value need to be escaped because they
+  // need to be taken as literal characters rather than special selector behavior of dollar signs in
+  // CSS.
+  return selector.replace('$', '\\$');
 }
 
 function getNodeName(node: t.Template|t.Element): string {
@@ -222,7 +227,7 @@ function difference<T>(left: Set<T>, right: Set<T>): Set<T> {
 export function getDirectiveMatchesForElementTag(
     element: t.Template|t.Element, directives: DirectiveSymbol[]): Set<DirectiveSymbol> {
   const attributes = getAttributes(element);
-  const allAttrs = attributes.map(toAttributeString);
+  const allAttrs = attributes.map(toAttributeCssSelector);
   const allDirectiveMatches =
       getDirectiveMatchesForSelector(directives, getNodeName(element) + allAttrs.join(''));
   const matchesWithoutElement = getDirectiveMatchesForSelector(directives, allAttrs.join(''));
@@ -232,7 +237,7 @@ export function getDirectiveMatchesForElementTag(
 
 export function makeElementSelector(element: t.Element|t.Template): string {
   const attributes = getAttributes(element);
-  const allAttrs = attributes.map(toAttributeString);
+  const allAttrs = attributes.map(toAttributeCssSelector);
   return getNodeName(element) + allAttrs.join('');
 }
 
@@ -251,10 +256,10 @@ export function getDirectiveMatchesForAttribute(
     name: string, hostNode: t.Template|t.Element,
     directives: DirectiveSymbol[]): Set<DirectiveSymbol> {
   const attributes = getAttributes(hostNode);
-  const allAttrs = attributes.map(toAttributeString);
+  const allAttrs = attributes.map(toAttributeCssSelector);
   const allDirectiveMatches =
       getDirectiveMatchesForSelector(directives, getNodeName(hostNode) + allAttrs.join(''));
-  const attrsExcludingName = attributes.filter(a => a.name !== name).map(toAttributeString);
+  const attrsExcludingName = attributes.filter(a => a.name !== name).map(toAttributeCssSelector);
   const matchesWithoutAttr = getDirectiveMatchesForSelector(
       directives, getNodeName(hostNode) + attrsExcludingName.join(''));
   return difference(allDirectiveMatches, matchesWithoutAttr);
@@ -266,20 +271,26 @@ export function getDirectiveMatchesForAttribute(
  */
 function getDirectiveMatchesForSelector(
     directives: DirectiveSymbol[], selector: string): Set<DirectiveSymbol> {
-  const selectors = CssSelector.parse(selector);
-  if (selectors.length === 0) {
+  try {
+    const selectors = CssSelector.parse(selector);
+    if (selectors.length === 0) {
+      return new Set();
+    }
+    return new Set(directives.filter((dir: DirectiveSymbol) => {
+      if (dir.selector === null) {
+        return false;
+      }
+
+      const matcher = new SelectorMatcher();
+      matcher.addSelectables(CssSelector.parse(dir.selector));
+
+      return selectors.some(selector => matcher.match(selector, null));
+    }));
+  } catch {
+    // An invalid selector may throw an error. There would be no directive matches for an invalid
+    // selector.
     return new Set();
   }
-  return new Set(directives.filter((dir: DirectiveSymbol) => {
-    if (dir.selector === null) {
-      return false;
-    }
-
-    const matcher = new SelectorMatcher();
-    matcher.addSelectables(CssSelector.parse(dir.selector));
-
-    return selectors.some(selector => matcher.match(selector, null));
-  }));
 }
 
 /**

packages/language-service/test/definitions_spec.ts

@@ -0,0 +1,103 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
+
+import {assertFileNames, assertTextSpans, humanizeDocumentSpanLike, LanguageServiceTestEnv, Project} from '../testing';
+
+describe('definitions', () => {
+  let env: LanguageServiceTestEnv;
+
+  describe('when an input has a dollar sign', () => {
+    const files = {
+      'app.ts': `
+	 import {Component, NgModule, Input} from '@angular/core';
+
+	 @Component({selector: 'dollar-cmp', template: ''})
+	 export class DollarCmp {
+	   @Input() obs$!: string;
+	 }
+ 
+	 @Component({template: '<dollar-cmp [obs$]="greeting"></dollar-cmp>'})
+	 export class AppCmp {
+	   greeting = 'hello';
+	 }
+ 
+	 @NgModule({declarations: [AppCmp, DollarCmp]})
+	 export class AppModule {}
+       `,
+    };
+
+    beforeEach(() => {
+      initMockFileSystem('Native');
+      env = LanguageServiceTestEnv.setup();
+    });
+
+    it('can get definitions for input', () => {
+      const project = env.addProject('test', files, {strictTemplates: false});
+      const definitions = getDefinitionsAndAssertBoundSpan(project, 'app.ts', '[o¦bs$]="greeting"');
+      expect(definitions!.length).toEqual(1);
+
+      assertTextSpans(definitions, ['obs$']);
+      assertFileNames(definitions, ['app.ts']);
+    });
+
+    it('can get definitions for component', () => {
+      const project = env.addProject('test', files, {strictTemplates: false});
+      const definitions = getDefinitionsAndAssertBoundSpan(project, 'app.ts', '<dollar-cm¦p');
+      expect(definitions!.length).toEqual(1);
+
+      assertTextSpans(definitions, ['DollarCmp']);
+      assertFileNames(definitions, ['app.ts']);
+    });
+  });
+
+  describe('when a selector and input of a directive have a dollar sign', () => {
+    it('can get definitions', () => {
+      initMockFileSystem('Native');
+      env = LanguageServiceTestEnv.setup();
+      const files = {
+        'app.ts': `
+	 import {Component, Directive, NgModule, Input} from '@angular/core';
+
+	 @Directive({selector: '[dollar\\\\$]', template: ''})
+	 export class DollarDir {
+	   @Input() dollar$!: string;
+	 }
+ 
+	 @Component({template: '<div [dollar$]="greeting"></div>'})
+	 export class AppCmp {
+	   greeting = 'hello';
+	 }
+ 
+	 @NgModule({declarations: [AppCmp, DollarDir]})
+	 export class AppModule {}
+       `,
+      };
+      const project = env.addProject('test', files, {strictTemplates: false});
+      const definitions =
+          getDefinitionsAndAssertBoundSpan(project, 'app.ts', '[dollar¦$]="greeting"');
+      expect(definitions!.length).toEqual(2);
+
+      assertTextSpans(definitions, ['dollar$', 'DollarDir']);
+      assertFileNames(definitions, ['app.ts']);
+    });
+  });
+
+  function getDefinitionsAndAssertBoundSpan(project: Project, file: string, targetText: string) {
+    const template = project.openFile(file);
+    env.expectNoSourceDiagnostics();
+    project.expectNoTemplateDiagnostics('app.ts', 'AppCmp');
+
+    template.moveCursorToText(targetText);
+    const defAndBoundSpan = template.getDefinitionAndBoundSpan();
+    expect(defAndBoundSpan).toBeTruthy();
+    expect(defAndBoundSpan!.definitions).toBeTruthy();
+    return defAndBoundSpan!.definitions!.map(d => humanizeDocumentSpanLike(d, env));
+  }
+});