Implement signature help for the Language Service

packages/compiler-cli/src/ngtsc/perf/src/api.ts

@@ -144,6 +144,11 @@ export enum PerfPhase {
    */
   LsComponentLocations,
 
+  /**
+   * Time spent by the Angular Language Service calculating signature help.
+   */
+  LsSignatureHelp,
+
   /**
    * Tracks the number of `PerfPhase`s, and must appear at the end of the list.
    */

packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts

@@ -69,7 +69,9 @@ class AstTranslator implements AstVisitor {
 
     // The `EmptyExpr` doesn't have a dedicated method on `AstVisitor`, so it's special cased here.
     if (ast instanceof EmptyExpr) {
-      return UNDEFINED;
+      const res = ts.factory.createIdentifier('undefined');
+      addParseSpanInfo(res, ast.sourceSpan);
+      return res;
     }
 
     // First attempt to let any custom resolution logic provide a translation for the given node.

packages/compiler/src/compiler_util/expression_converter.ts

@@ -705,7 +705,8 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
           leftMostSafe,
           new cdAst.MethodCall(
               leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan,
-              leftMostSafe.receiver, leftMostSafe.name, leftMostSafe.args));
+              leftMostSafe.receiver, leftMostSafe.name, leftMostSafe.args,
+              leftMostSafe.argumentSpan));
     } else {
       this._nodeMap.set(
           leftMostSafe,

packages/compiler/src/expression_parser/ast.ts

@@ -308,7 +308,8 @@ export class NonNullAssert extends AST {
 export class MethodCall extends ASTWithName {
   constructor(
       span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
-      public receiver: AST, public name: string, public args: any[]) {
+      public receiver: AST, public name: string, public args: any[],
+      public argumentSpan: AbsoluteSourceSpan) {
     super(span, sourceSpan, nameSpan);
   }
   visit(visitor: AstVisitor, context: any = null): any {
@@ -319,7 +320,8 @@ export class MethodCall extends ASTWithName {
 export class SafeMethodCall extends ASTWithName {
   constructor(
       span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
-      public receiver: AST, public name: string, public args: any[]) {
+      public receiver: AST, public name: string, public args: any[],
+      public argumentSpan: AbsoluteSourceSpan) {
     super(span, sourceSpan, nameSpan);
   }
   visit(visitor: AstVisitor, context: any = null): any {
@@ -583,13 +585,13 @@ export class AstTransformer implements AstVisitor {
   visitMethodCall(ast: MethodCall, context: any): AST {
     return new MethodCall(
         ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name,
-        this.visitAll(ast.args));
+        this.visitAll(ast.args), ast.argumentSpan);
   }
 
   visitSafeMethodCall(ast: SafeMethodCall, context: any): AST {
     return new SafeMethodCall(
         ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name,
-        this.visitAll(ast.args));
+        this.visitAll(ast.args), ast.argumentSpan);
   }
 
   visitFunctionCall(ast: FunctionCall, context: any): AST {
@@ -719,7 +721,8 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
     const receiver = ast.receiver.visit(this);
     const args = this.visitAll(ast.args);
     if (receiver !== ast.receiver || args !== ast.args) {
-      return new MethodCall(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args);
+      return new MethodCall(
+          ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args, ast.argumentSpan);
     }
     return ast;
   }
@@ -728,7 +731,8 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
     const receiver = ast.receiver.visit(this);
     const args = this.visitAll(ast.args);
     if (receiver !== ast.receiver || args !== ast.args) {
-      return new SafeMethodCall(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args);
+      return new SafeMethodCall(
+          ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args, ast.argumentSpan);
     }
     return ast;
   }

packages/compiler/src/expression_parser/parser.ts

@@ -461,6 +461,19 @@ export class _ParseAST {
     if (artificialEndIndex !== undefined && artificialEndIndex > this.currentEndIndex) {
       endIndex = artificialEndIndex;
     }
+
+    // In some unusual parsing scenarios (like when certain tokens are missing and an `EmptyExpr` is
+    // being created), the current token may already be advanced beyond the `currentEndIndex`. This
+    // appears to be a deep-seated parser bug.
+    //
+    // As a workaround for now, swap the start and end indices to ensure a valid `ParseSpan`.
+    // TODO(alxhub): fix the bug upstream in the parser state, and remove this workaround.
+    if (start > endIndex) {
+      const tmp = endIndex;
+      endIndex = start;
+      start = tmp;
+    }
+
     return new ParseSpan(start, endIndex);
   }
 
@@ -940,14 +953,19 @@ export class _ParseAST {
     const nameSpan = this.sourceSpan(nameStart);
 
     if (this.consumeOptionalCharacter(chars.$LPAREN)) {
+      const argumentStart = this.inputIndex;
       this.rparensExpected++;
       const args = this.parseCallArguments();
+      const argumentSpan =
+          this.span(argumentStart, this.inputIndex).toAbsolute(this.absoluteOffset);
+
       this.expectCharacter(chars.$RPAREN);
       this.rparensExpected--;
       const span = this.span(start);
       const sourceSpan = this.sourceSpan(start);
-      return isSafe ? new SafeMethodCall(span, sourceSpan, nameSpan, receiver, id, args) :
-                      new MethodCall(span, sourceSpan, nameSpan, receiver, id, args);
+      return isSafe ?
+          new SafeMethodCall(span, sourceSpan, nameSpan, receiver, id, args, argumentSpan) :
+          new MethodCall(span, sourceSpan, nameSpan, receiver, id, args, argumentSpan);
 
     } else {
       if (isSafe) {

packages/compiler/test/expression_parser/parser_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AbsoluteSourceSpan, ASTWithSource, BindingPipe, EmptyExpr, Interpolation, ParserError, TemplateBinding, VariableBinding} from '@angular/compiler/src/expression_parser/ast';
+import {AbsoluteSourceSpan, ASTWithSource, BindingPipe, EmptyExpr, Interpolation, MethodCall, ParserError, TemplateBinding, VariableBinding} from '@angular/compiler/src/expression_parser/ast';
 import {Lexer} from '@angular/compiler/src/expression_parser/lexer';
 import {IvyParser, Parser, SplitInterpolation} from '@angular/compiler/src/expression_parser/parser';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
@@ -188,6 +188,13 @@ describe('parser', () => {
         checkAction('a.add(1, 2)');
         checkAction('fn().add(1, 2)');
       });
+
+      it('should parse an EmptyExpr with a correct span for a trailing empty argument', () => {
+        const ast = parseAction('fn(1, )').ast as MethodCall;
+        expect(ast.args[1]).toBeAnInstanceOf(EmptyExpr);
+        const sourceSpan = (ast.args[1] as EmptyExpr).sourceSpan;
+        expect([sourceSpan.start, sourceSpan.end]).toEqual([5, 6]);
+      });
     });
 
     describe('functional calls', () => {
@@ -350,6 +357,11 @@ describe('parser', () => {
       expect(unparseWithSpan(ast)).toContain(['foo()', '[nameSpan] foo']);
     });
 
+    it('should record method call argument span', () => {
+      const ast = parseAction('foo(1 + 2)');
+      expect(unparseWithSpan(ast)).toContain(['foo(1 + 2)', '[argumentSpan] 1 + 2']);
+    });
+
     it('should record accessed method call span', () => {
       const ast = parseAction('foo.bar()');
       expect(unparseWithSpan(ast)).toContain(['foo.bar()', 'foo.bar()']);

packages/compiler/test/expression_parser/utils/unparser.ts

@@ -229,6 +229,9 @@ export function unparseWithSpan(
       if (ast.hasOwnProperty('nameSpan')) {
         this.recordUnparsed(ast, 'nameSpan', unparsedList);
       }
+      if (ast.hasOwnProperty('argumentSpan')) {
+        this.recordUnparsed(ast, 'argumentSpan', unparsedList);
+      }
       ast.visit(this, unparsedList);
     }
   };

packages/language-service/ivy/language_service.ts

@@ -27,6 +27,7 @@ import {CompletionBuilder, CompletionNodeContext} from './completions';
 import {DefinitionBuilder} from './definitions';
 import {QuickInfoBuilder} from './quick_info';
 import {ReferencesAndRenameBuilder} from './references';
+import {getSignatureHelp} from './signature_help';
 import {getTargetAtPosition, TargetContext, TargetNodeKind} from './template_target';
 import {findTightestNode, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './ts_utils';
 import {getTemplateInfoAtPosition, isTypeScriptFile} from './utils';
@@ -254,6 +255,19 @@ export class LanguageService {
     });
   }
 
+  getSignatureHelpItems(fileName: string, position: number, options?: ts.SignatureHelpItemsOptions):
+      ts.SignatureHelpItems|undefined {
+    return this.withCompilerAndPerfTracing(PerfPhase.LsSignatureHelp, compiler => {
+      if (!isTemplateContext(compiler.getCurrentProgram(), fileName, position)) {
+        return undefined;
+      }
+
+      return getSignatureHelp(compiler, this.tsLS, fileName, position, options);
+
+      return undefined;
+    });
+  }
+
   getCompletionEntrySymbol(fileName: string, position: number, entryName: string): ts.Symbol
       |undefined {
     return this.withCompilerAndPerfTracing(PerfPhase.LsCompletions, (compiler) => {

packages/language-service/ivy/signature_help.ts

@@ -0,0 +1,135 @@
+/**
+ * @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 {MethodCall, SafeMethodCall} from '@angular/compiler';
+import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
+import {getSourceFileOrError} from '@angular/compiler-cli/src/ngtsc/file_system';
+import {SymbolKind} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
+import * as ts from 'typescript/lib/tsserverlibrary';
+
+import {getTargetAtPosition, TargetNodeKind} from './template_target';
+import {findTightestNode} from './ts_utils';
+import {getTemplateInfoAtPosition} from './utils';
+
+/**
+ * Queries the TypeScript Language Service to get signature help for a template position.
+ */
+export function getSignatureHelp(
+    compiler: NgCompiler, tsLS: ts.LanguageService, fileName: string, position: number,
+    options: ts.SignatureHelpItemsOptions|undefined): ts.SignatureHelpItems|undefined {
+  const templateInfo = getTemplateInfoAtPosition(fileName, position, compiler);
+  if (templateInfo === undefined) {
+    return undefined;
+  }
+
+  const targetInfo = getTargetAtPosition(templateInfo.template, position);
+  if (targetInfo === null) {
+    return undefined;
+  }
+
+  if (targetInfo.context.kind !== TargetNodeKind.RawExpression &&
+      targetInfo.context.kind !== TargetNodeKind.MethodCallExpressionInArgContext) {
+    // Signature completions are only available in expressions.
+    return undefined;
+  }
+
+  const symbol = compiler.getTemplateTypeChecker().getSymbolOfNode(
+      targetInfo.context.node, templateInfo.component);
+  if (symbol === null || symbol.kind !== SymbolKind.Expression) {
+    return undefined;
+  }
+
+  // Determine a shim position to use in the request to the TypeScript Language Service.
+  // Additionally, extract the `MethodCall` or `SafeMethodCall` node for which signature help is
+  // being queried, as this is needed to construct the correct span for the results later.
+  let shimPosition: number;
+  let expr: MethodCall|SafeMethodCall;
+  switch (targetInfo.context.kind) {
+    case TargetNodeKind.RawExpression:
+      // For normal expressions, just use the primary TCB position of the expression.
+      shimPosition = symbol.shimLocation.positionInShimFile;
+
+      // Walk up the parents of this expression and try to find a `MethodCall` or `SafeMethodCall`
+      // for which signature information is being fetched.
+      let callExpr: MethodCall|SafeMethodCall|null = null;
+      const parents = targetInfo.context.parents;
+      for (let i = parents.length - 1; i >= 0; i--) {
+        const parent = parents[i];
+        if (parent instanceof MethodCall || parent instanceof SafeMethodCall) {
+          callExpr = parent;
+          break;
+        }
+      }
+
+      // If no MethodCall or SafeMethodCall node could be found, then this query cannot be safely
+      // answered as a correct span for the results will not be obtainable.
+      if (callExpr === null) {
+        return undefined;
+      }
+
+      expr = callExpr;
+      break;
+    case TargetNodeKind.MethodCallExpressionInArgContext:
+      // The `Symbol` points to a `MethodCall` or `SafeMethodCall` expression in the TCB (where it
+      // will be represented as a `ts.CallExpression`) *and* the template position was within the
+      // argument list of the method call. This happens when there was no narrower expression inside
+      // the argument list that matched the template position, such as when the call has no
+      // arguments: `foo(|)`.
+      //
+      // The `Symbol`'s shim position is to the start of the call expression (`|foo()`) and
+      // therefore wouldn't return accurate signature help from the TS language service. For that, a
+      // position within the argument list for the `ts.CallExpression` in the TCB will need to be
+      // determined. This is done by finding that call expression and extracting a viable position
+      // from it directly.
+      //
+      // First, use `findTightestNode` to locate the `ts.Node` at `symbol`'s location.
+      const shimSf =
+          getSourceFileOrError(compiler.getCurrentProgram(), symbol.shimLocation.shimPath);
+      let shimNode: ts.Node|null =
+          findTightestNode(shimSf, symbol.shimLocation.positionInShimFile) ?? null;
+
+      // This node should be somewhere inside a `ts.CallExpression`. Walk up the AST to find it.
+      while (shimNode !== null) {
+        if (ts.isCallExpression(shimNode)) {
+          break;
+        }
+        shimNode = shimNode.parent ?? null;
+      }
+
+      // If one couldn't be found, something is wrong, so bail rather than report incorrect results.
+      if (shimNode === null || !ts.isCallExpression(shimNode)) {
+        return undefined;
+      }
+
+      // Position the cursor in the TCB at the start of the argument list for the
+      // `ts.CallExpression`. This will allow us to get the correct signature help, even though the
+      // template itself doesn't have an expression inside the argument list.
+      shimPosition = shimNode.arguments.pos;
+
+      // In this case, getting the right call AST node is easy.
+      expr = targetInfo.context.node;
+      break;
+  }
+
+  const res = tsLS.getSignatureHelpItems(symbol.shimLocation.shimPath, shimPosition, options);
+  if (res === undefined) {
+    return undefined;
+  }
+
+  // The TS language service results are almost returnable as-is. However, they contain an
+  // `applicableSpan` which marks the entire argument list, and that span is in the context of the
+  // TCB's `ts.CallExpression`. It needs to be replaced with the span for the `MethodCall` (or
+  // `SafeMethodCall`) argument list.
+  return {
+    ...res,
+    applicableSpan: {
+      start: expr.argumentSpan.start,
+      length: expr.argumentSpan.end - expr.argumentSpan.start,
+    },
+  };
+}