fix(language-service): Improve signature selection by finding exact m…

…atch (angular#37494)

The function signature selection algorithm is totally naive. It'd
unconditionally pick the first signature if there are multiple
overloads. This commit improves the algorithm by returning an exact
match if one exists.

PR Close angular#37494

packages/language-service/src/typescript_symbols.ts

@@ -222,13 +222,35 @@ function signaturesOf(type: ts.Type, context: TypeContext): Signature[] {
   return type.getCallSignatures().map(s => new SignatureWrapper(s, context));
 }
 
-function selectSignature(type: ts.Type, context: TypeContext, _types: Symbol[]): Signature|
+function selectSignature(type: ts.Type, context: TypeContext, types: Symbol[]): Signature|
     undefined {
   // TODO: Do a better job of selecting the right signature. TypeScript does not currently support a
   // Type Relationship API (see https://github.com/angular/vscode-ng-language-service/issues/143).
   // Consider creating a TypeCheckBlock host in the language service that may also act as a
   // scratchpad for type comparisons.
   const signatures = type.getCallSignatures();
+  const passedInTypes: Array<ts.Type|undefined> = types.map(type => {
+    if (type instanceof TypeWrapper) {
+      return type.tsType;
+    }
+  });
+  // Try to select a matching signature in which all parameter types match.
+  // Note that this is just a best-effort approach, because we're checking for
+  // strict type equality rather than compatibility.
+  // For example, if the signature contains a ReadonlyArray<number> and the
+  // passed parameter type is an Array<number>, this will fail.
+  function allParameterTypesMatch(signature: ts.Signature) {
+    const tc = context.checker;
+    return signature.getParameters().every((parameter: ts.Symbol, i: number) => {
+      const type = tc.getTypeOfSymbolAtLocation(parameter, parameter.valueDeclaration);
+      return type === passedInTypes[i];
+    });
+  }
+  const exactMatch = signatures.find(allParameterTypesMatch);
+  if (exactMatch) {
+    return new SignatureWrapper(exactMatch, context);
+  }
+  // If not, fallback to a naive selection
   return signatures.length ? new SignatureWrapper(signatures[0], context) : undefined;
 }
 

packages/language-service/test/completions_spec.ts

@@ -832,6 +832,20 @@ describe('completions', () => {
       expectContain(completions, CompletionKind.METHOD, ['charAt', 'substring']);
     });
   });
+
+  it('should select the right signature for a pipe given exact type', () => {
+    mockHost.override(TEST_TEMPLATE, '{{ ("world" | prefixPipe:"hello").~{cursor} }}');
+    const m1 = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor');
+    const c1 = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, m1.start);
+    // should resolve to transform(value: string, prefix: string): string
+    expectContain(c1, CompletionKind.METHOD, ['charCodeAt', 'trim']);
+
+    mockHost.override(TEST_TEMPLATE, '{{ (456 | prefixPipe:123).~{cursor} }}');
+    const m2 = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor');
+    const c2 = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, m2.start);
+    // should resolve to transform(value: number, prefix: number): number
+    expectContain(c2, CompletionKind.METHOD, ['toFixed', 'toExponential']);
+  });
 });
 
 function expectContain(

packages/language-service/test/project/app/main.ts

@@ -22,6 +22,7 @@ import * as ParsingCases from './parsing-cases';
     ParsingCases.StringModel,
     ParsingCases.TemplateReference,
     ParsingCases.TestComponent,
+    ParsingCases.TestPipe,
     ParsingCases.WithContextDirective,
   ]
 })

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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, EventEmitter, Input, OnChanges, Output, SimpleChanges, TemplateRef, ViewContainerRef} from '@angular/core';
+import {Component, Directive, EventEmitter, Input, OnChanges, Output, Pipe, PipeTransform, SimpleChanges, TemplateRef, ViewContainerRef} from '@angular/core';
 
 import {Hero} from './app.component';
 
@@ -69,6 +69,20 @@ export class WithContextDirective {
   }
 }
 
+@Pipe({
+  name: 'prefixPipe',
+})
+export class TestPipe implements PipeTransform {
+  transform(value: string, prefix: string): string;
+  transform(value: number, prefix: number): number;
+  transform(value: string|number, prefix: string|number): string|number {
+    if (typeof value === 'string') {
+      return `${prefix} ${value}`;
+    }
+    return parseInt(`${prefix}${value}`, 10 /* radix */);
+  }
+}
+
 /**
  * This Component provides the `test-comp` selector.
  */