fix(language-service): fix calculation of pipe spans

packages/language-service/src/expressions.ts

@@ -96,6 +96,14 @@ export function getExpressionCompletions(
   return result && result.values();
 }
 
+/**
+ * Retrieves the expression symbol at a particular position in a template.
+ *
+ * @param scope symbols in scope of the template
+ * @param ast template AST
+ * @param position absolute location in template to retrieve symbol at
+ * @param query type symbol query for the template scope
+ */
 export function getExpressionSymbol(
     scope: SymbolTable, ast: AST, position: number,
     query: SymbolQuery): {symbol: Symbol, span: Span}|undefined {
@@ -129,14 +137,19 @@ export function getExpressionSymbol(
       span = ast.span;
     },
     visitPipe(ast) {
-      if (position >= ast.exp.span.end &&
-          (!ast.args || !ast.args.length || position < (<AST>ast.args[0]).span.start)) {
+      if (inSpan(position, ast.nameSpan, /* exclusive */ true)) {
         // We are in a position a pipe name is expected.
         const pipes = query.getPipes();
-        if (pipes) {
-          symbol = pipes.get(ast.name);
-          span = ast.span;
-        }
+        symbol = pipes.get(ast.name);
+
+        // `nameSpan` is an absolute span, but the span expected by the result of this method is
+        // relative to the start of the expression.
+        // TODO(ayazhafiz): migrate to only using absolute spans
+        const offset = ast.sourceSpan.start - ast.span.start;
+        span = {
+          start: ast.nameSpan.start - offset,
+          end: ast.nameSpan.end - offset,
+        };
       }
     },
     visitPrefixNot(ast) {},

packages/language-service/test/definitions_spec.ts

@@ -234,22 +234,19 @@ describe('definitions', () => {
   it('should be able to find a pipe', () => {
     const fileName = mockHost.addCode(`
       @Component({
-        template: '<div *ngIf="~{start-my}input | «async»~{end-my}"></div>'
+        template: '<div *ngIf="input | «async»"></div>'
       })
       export class MyComponent {
         input: EventEmitter;
       }`);
 
-    // Get the marker for «test» in the code added above.
+    // Get the marker for «async» in the code added above.
     const marker = mockHost.getReferenceMarkerFor(fileName, 'async');
-
     const result = ngService.getDefinitionAndBoundSpan(fileName, marker.start);
+
     expect(result).toBeDefined();
     const {textSpan, definitions} = result !;
-
-    // Get the marker for bounded text in the code added above
-    const boundedText = mockHost.getLocationMarkerFor(fileName, 'my');
-    expect(textSpan).toEqual(boundedText);
+    expect(textSpan).toEqual(marker);
 
     expect(definitions).toBeDefined();
     expect(definitions !.length).toBe(4);
@@ -263,6 +260,28 @@ describe('definitions', () => {
     }
   });
 
+  // https://github.com/angular/vscode-ng-language-service/issues/677
+  it('should be able to find a pipe with arguments', () => {
+    mockHost.override(TEST_TEMPLATE, `{{birthday | «date»: "MM/dd/yy"}}`);
+
+    const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'date');
+    const result = ngService.getDefinitionAndBoundSpan(TEST_TEMPLATE, marker.start);
+
+    expect(result).toBeDefined();
+    const {textSpan, definitions} = result !;
+    expect(textSpan).toEqual(marker);
+
+    expect(definitions).toBeDefined();
+    expect(definitions !.length).toBe(1);
+
+    const refFileName = '/node_modules/@angular/common/common.d.ts';
+    for (const def of definitions !) {
+      expect(def.fileName).toBe(refFileName);
+      expect(def.name).toBe('date');
+      expect(def.kind).toBe('pipe');
+    }
+  });
+
   describe('in structural directive', () => {
     it('should be able to find the directive', () => {
       mockHost.override(

packages/language-service/test/hover_spec.ts

@@ -138,6 +138,19 @@ describe('hover', () => {
       expect(toText(displayParts)).toBe('(property) TemplateReference.heroes: Hero[]');
     });
 
+    it('should work for pipes', () => {
+      mockHost.override(TEST_TEMPLATE, `
+        <p>The hero's birthday is {{birthday | «date»: "MM/dd/yy"}}</p>`);
+      const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'date');
+      const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start);
+      expect(quickInfo).toBeTruthy();
+      const {textSpan, displayParts} = quickInfo !;
+      expect(textSpan).toEqual(marker);
+      expect(toText(displayParts))
+          .toBe(
+              '(pipe) date: (value: any, format?: string | undefined, timezone?: string | undefined, locale?: string | undefined) => string | null');
+    });
+
     it('should work for the $any() cast function', () => {
       const content = mockHost.override(TEST_TEMPLATE, '<div>{{$any(title)}}</div>');
       const position = content.indexOf('$any');

packages/language-service/test/project/app/parsing-cases.ts

@@ -171,6 +171,7 @@ export class TemplateReference {
   anyValue: any;
   optional?: string;
   myClick(event: any) {}
+  birthday = new Date();
 }
 
 @Component({