fix(54458): The inlay hint does not handle TypeScript's spread operat…

…or well (microsoft#54503)
Andarist · Jun 5, 2023 · 67299ed · 67299ed
1 parent cbda2fc
commit 67299ed
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 3 deletions.
diff --git a/src/services/inlayHints.ts b/src/services/inlayHints.ts
@@ -4,11 +4,13 @@ import {
     CallExpression,
     createPrinterWithRemoveComments,
     Debug,
+    ElementFlags,
     EmitHint,
     EnumMember,
     equateStringsCaseInsensitive,
     Expression,
     findChildOfKind,
+    findIndex,
     forEachChild,
     FunctionDeclaration,
     FunctionExpression,
@@ -44,6 +46,7 @@ import {
     isParameterDeclaration,
     isPropertyAccessExpression,
     isPropertyDeclaration,
+    isSpreadElement,
     isTypeNode,
     isVarConst,
     isVariableDeclaration,
@@ -61,6 +64,7 @@ import {
     SymbolFlags,
     SyntaxKind,
     textSpanIntersectsWith,
+    TupleTypeReference,
     Type,
     TypeFormatFlags,
     unescapeLeadingUnderscores,
@@ -226,14 +230,31 @@ export function provideInlayHints(context: InlayHintsContext): InlayHint[] {
             return;
         }
 
-        for (let i = 0; i < args.length; ++i) {
-            const originalArg = args[i];
+        let signatureParamPos = 0;
+        for (const originalArg of args) {
             const arg = skipParentheses(originalArg);
             if (shouldShowLiteralParameterNameHintsOnly(preferences) && !isHintableLiteral(arg)) {
                 continue;
             }
 
-            const identifierNameInfo = checker.getParameterIdentifierNameAtPosition(signature, i);
+            let spreadArgs = 0;
+            if (isSpreadElement(arg)) {
+                const spreadType = checker.getTypeAtLocation(arg.expression);
+                if (checker.isTupleType(spreadType)) {
+                    const { elementFlags, fixedLength } = (spreadType as TupleTypeReference).target;
+                    if (fixedLength === 0) {
+                        continue;
+                    }
+                    const firstOptionalIndex = findIndex(elementFlags, f => !(f & ElementFlags.Required));
+                    const requiredArgs = firstOptionalIndex < 0 ? fixedLength : firstOptionalIndex;
+                    if (requiredArgs > 0) {
+                        spreadArgs = firstOptionalIndex < 0 ? fixedLength : firstOptionalIndex;
+                    }
+                }
+            }
+
+            const identifierNameInfo = checker.getParameterIdentifierNameAtPosition(signature, signatureParamPos);
+            signatureParamPos = signatureParamPos + (spreadArgs || 1);
             if (identifierNameInfo) {
                 const [parameterName, isFirstVariadicArgument] = identifierNameInfo;
                 const isParameterNameNotSameAsArgument = preferences.includeInlayParameterNameHintsWhenArgumentMatchesName || !identifierOrAccessExpressionPostfixMatchesParameterName(arg, parameterName);

diff --git a/tests/cases/fourslash/inlayHintsShouldWork70.ts b/tests/cases/fourslash/inlayHintsShouldWork70.ts
@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+////function foo(a: unknown, b: unknown, c: unknown) { }
+////function bar(...x: [number, number]) {
+////    foo(/*a*/...x, /*c*/3);
+////}
+
+const [a, c] = test.markers();
+
+verify.getInlayHints([
+    {
+        text: 'a:',
+        position: a.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+    {
+        text: 'c:',
+        position: c.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+], undefined, {
+    includeInlayParameterNameHints: "all"
+});
diff --git a/tests/cases/fourslash/inlayHintsShouldWork71.ts b/tests/cases/fourslash/inlayHintsShouldWork71.ts
@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+////function foo(a: unknown, b: unknown, c: unknown, d: unknown) { }
+////function bar(...x: [number, number, number]) {
+////    foo(/*a*/...x, /*d*/3);
+////}
+
+const [a, d] = test.markers();
+
+verify.getInlayHints([
+    {
+        text: 'a:',
+        position: a.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+    {
+        text: 'd:',
+        position: d.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+], undefined, {
+    includeInlayParameterNameHints: "all"
+});
diff --git a/tests/cases/fourslash/inlayHintsShouldWork72.ts b/tests/cases/fourslash/inlayHintsShouldWork72.ts
@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+////function foo(a: unknown, b: unknown, c: unknown) { }
+////function bar(...x: [number, number?]) {
+////    foo(/*a*/...x, /*b*/3);
+////}
+
+const [a, b] = test.markers();
+
+verify.getInlayHints([
+    {
+        text: 'a:',
+        position: a.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+    {
+        text: 'b:',
+        position: b.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+], undefined, {
+    includeInlayParameterNameHints: "all"
+});
diff --git a/tests/cases/fourslash/inlayHintsShouldWork73.ts b/tests/cases/fourslash/inlayHintsShouldWork73.ts
@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+////function foo(a: unknown, b: unknown, c: unknown) { }
+////function bar(...x: [number, number?]) {
+////    foo(/*a*/1, /*b*/...x);
+////}
+
+const [a, b] = test.markers();
+
+verify.getInlayHints([
+    {
+        text: 'a:',
+        position: a.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+    {
+        text: 'b:',
+        position: b.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+], undefined, {
+    includeInlayParameterNameHints: "all"
+});
diff --git a/tests/cases/fourslash/inlayHintsShouldWork74.ts b/tests/cases/fourslash/inlayHintsShouldWork74.ts
@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+////function foo(a: unknown, b: unknown, c: unknown) { }
+////function bar(...x: [number, number | undefined]) {
+////    foo(/*a*/...x, /*c*/3);
+////}
+
+const [a, b] = test.markers();
+
+verify.getInlayHints([
+    {
+        text: 'a:',
+        position: a.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+    {
+        text: 'c:',
+        position: b.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+], undefined, {
+    includeInlayParameterNameHints: "all"
+});
diff --git a/tests/cases/fourslash/inlayHintsShouldWork75.ts b/tests/cases/fourslash/inlayHintsShouldWork75.ts
@@ -0,0 +1,31 @@
+/// <reference path="fourslash.ts" />
+
+////function foo(a: unknown, b: unknown, c: unknown) { }
+////function bar(...x: []) {
+////    foo(...x, /*a*/1, /*b*/2, /*c*/3);
+////}
+
+const [a, b, c] = test.markers();
+
+verify.getInlayHints([
+    {
+        text: 'a:',
+        position: a.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+    {
+        text: 'b:',
+        position: b.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+    {
+        text: 'c:',
+        position: c.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+], undefined, {
+    includeInlayParameterNameHints: "all"
+});
diff --git a/tests/cases/fourslash/inlayHintsShouldWork76.ts b/tests/cases/fourslash/inlayHintsShouldWork76.ts
@@ -0,0 +1,19 @@
+/// <reference path="fourslash.ts" />
+
+////function foo({ a, b }: { a: unknown, b: unknown }, c: unknown, d: unknown) { }
+////function bar(...x: [{ a: unknown, b: unknown }, number]) {
+////    foo(...x, /*d*/1);
+////}
+
+const [d] = test.markers();
+
+verify.getInlayHints([
+    {
+        text: 'd:',
+        position: d.position,
+        kind: ts.InlayHintKind.Parameter,
+        whitespaceAfter: true
+    },
+], undefined, {
+    includeInlayParameterNameHints: "all"
+});