Skip to content

Commit

Permalink
fix(48260): Incorrect parameter hint is highlighted when arguments co…
Browse files Browse the repository at this point in the history 
…ntain spread syntax (microsoft#56372)

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
  • Loading branch information
@a-tarasyuk @sandersn @c0sta
2 people authored and c0sta committed Dec 4, 2023
1 parent 7e21131 commit 77acb4a
Show file tree
Hide file tree
Showing 5 changed files with 773 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/services/completions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3122,7 +3122,7 @@ function getContextualType(previousToken: Node, position: number, sourceFile: So
case SyntaxKind.OpenBraceToken:
return isJsxExpression(parent) && !isJsxElement(parent.parent) && !isJsxFragment(parent.parent) ? checker.getContextualTypeForJsxAttribute(parent.parent) : undefined;
default:
const argInfo = SignatureHelp.getArgumentInfoForCompletions(previousToken, position, sourceFile);
const argInfo = SignatureHelp.getArgumentInfoForCompletions(previousToken, position, sourceFile, checker);
return argInfo ?
// At `,`, treat this as the next argument after the comma.
checker.getContextualTypeForArgumentAtIndex(argInfo.invocation, argInfo.argumentIndex + (previousToken.kind === SyntaxKind.CommaToken ? 1 : 0)) :
Expand Down
67 changes: 49 additions & 18 deletions src/services/signatureHelp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
createTextSpanFromBounds,
createTextSpanFromNode,
Debug,
ElementFlags,
EmitHint,
emptyArray,
Expression,
Expand Down Expand Up @@ -49,6 +50,7 @@ import {
isPropertyAccessExpression,
isSourceFile,
isSourceFileJS,
isSpreadElement,
isTaggedTemplateExpression,
isTemplateHead,
isTemplateLiteralToken,
Expand All @@ -58,6 +60,7 @@ import {
JsxTagNameExpression,
last,
lastOrUndefined,
length,
ListFormat,
map,
mapToDisplayParts,
Expand All @@ -77,6 +80,7 @@ import {
skipTrivia,
SourceFile,
spacePart,
SpreadElement,
Symbol,
SymbolDisplayPart,
symbolToDisplayParts,
Expand All @@ -85,6 +89,7 @@ import {
TemplateExpression,
TextSpan,
tryCast,
TupleTypeReference,
Type,
TypeChecker,
TypeParameter,
Expand Down Expand Up @@ -272,25 +277,25 @@ export interface ArgumentInfoForCompletions {
readonly argumentCount: number;
}
/** @internal */
export function getArgumentInfoForCompletions(node: Node, position: number, sourceFile: SourceFile): ArgumentInfoForCompletions | undefined {
const info = getImmediatelyContainingArgumentInfo(node, position, sourceFile);
export function getArgumentInfoForCompletions(node: Node, position: number, sourceFile: SourceFile, checker: TypeChecker): ArgumentInfoForCompletions | undefined {
const info = getImmediatelyContainingArgumentInfo(node, position, sourceFile, checker);
return !info || info.isTypeParameterList || info.invocation.kind !== InvocationKind.Call ? undefined
: { invocation: info.invocation.node, argumentCount: info.argumentCount, argumentIndex: info.argumentIndex };
}

function getArgumentOrParameterListInfo(node: Node, position: number, sourceFile: SourceFile): { readonly list: Node; readonly argumentIndex: number; readonly argumentCount: number; readonly argumentsSpan: TextSpan; } | undefined {
const info = getArgumentOrParameterListAndIndex(node, sourceFile);
function getArgumentOrParameterListInfo(node: Node, position: number, sourceFile: SourceFile, checker: TypeChecker): { readonly list: Node; readonly argumentIndex: number; readonly argumentCount: number; readonly argumentsSpan: TextSpan; } | undefined {
const info = getArgumentOrParameterListAndIndex(node, sourceFile, checker);
if (!info) return undefined;
const { list, argumentIndex } = info;

const argumentCount = getArgumentCount(list, /*ignoreTrailingComma*/ isInString(sourceFile, position, node));
const argumentCount = getArgumentCount(list, /*ignoreTrailingComma*/ isInString(sourceFile, position, node), checker);
if (argumentIndex !== 0) {
Debug.assertLessThan(argumentIndex, argumentCount);
}
const argumentsSpan = getApplicableSpanForArguments(list, sourceFile);
return { list, argumentIndex, argumentCount, argumentsSpan };
}
function getArgumentOrParameterListAndIndex(node: Node, sourceFile: SourceFile): { readonly list: Node; readonly argumentIndex: number; } | undefined {
function getArgumentOrParameterListAndIndex(node: Node, sourceFile: SourceFile, checker: TypeChecker): { readonly list: Node; readonly argumentIndex: number; } | undefined {
if (node.kind === SyntaxKind.LessThanToken || node.kind === SyntaxKind.OpenParenToken) {
// Find the list that starts right *after* the < or ( token.
// If the user has just opened a list, consider this item 0.
Expand All @@ -304,15 +309,15 @@ function getArgumentOrParameterListAndIndex(node: Node, sourceFile: SourceFile):
// - On the target of the call (parent.func)
// - On the 'new' keyword in a 'new' expression
const list = findContainingList(node);
return list && { list, argumentIndex: getArgumentIndex(list, node) };
return list && { list, argumentIndex: getArgumentIndex(list, node, checker) };
}
}

/**
* Returns relevant information for the argument list and the current argument if we are
* in the argument of an invocation; returns undefined otherwise.
*/
function getImmediatelyContainingArgumentInfo(node: Node, position: number, sourceFile: SourceFile): ArgumentListInfo | undefined {
function getImmediatelyContainingArgumentInfo(node: Node, position: number, sourceFile: SourceFile, checker: TypeChecker): ArgumentListInfo | undefined {
const { parent } = node;
if (isCallOrNewExpression(parent)) {
const invocation = parent;
Expand All @@ -331,7 +336,7 @@ function getImmediatelyContainingArgumentInfo(node: Node, position: number, sour
// Case 3:
// foo<T#, U#>(a#, #b#) -> The token is buried inside a list, and should give signature help
// Find out if 'node' is an argument, a type argument, or neither
const info = getArgumentOrParameterListInfo(node, position, sourceFile);
const info = getArgumentOrParameterListInfo(node, position, sourceFile, checker);
if (!info) return undefined;
const { list, argumentIndex, argumentCount, argumentsSpan } = info;
const isTypeParameterList = !!parent.typeArguments && parent.typeArguments.pos === list.pos;
Expand Down Expand Up @@ -397,7 +402,7 @@ function getImmediatelyContainingArgumentInfo(node: Node, position: number, sour
}

function getImmediatelyContainingArgumentOrContextualParameterInfo(node: Node, position: number, sourceFile: SourceFile, checker: TypeChecker): ArgumentListInfo | undefined {
return tryGetParameterInfo(node, position, sourceFile, checker) || getImmediatelyContainingArgumentInfo(node, position, sourceFile);
return tryGetParameterInfo(node, position, sourceFile, checker) || getImmediatelyContainingArgumentInfo(node, position, sourceFile, checker);
}

function getHighestBinary(b: BinaryExpression): BinaryExpression {
Expand Down Expand Up @@ -452,7 +457,7 @@ function getContextualSignatureLocationInfo(node: Node, sourceFile: SourceFile,
case SyntaxKind.MethodDeclaration:
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
const info = getArgumentOrParameterListInfo(node, position, sourceFile);
const info = getArgumentOrParameterListInfo(node, position, sourceFile, checker);
if (!info) return undefined;
const { argumentIndex, argumentCount, argumentsSpan } = info;
const contextualType = isMethodDeclaration(parent) ? checker.getContextualTypeForObjectLiteralElement(parent) : checker.getContextualType(parent as ParenthesizedExpression | FunctionExpression | ArrowFunction);
Expand All @@ -476,7 +481,7 @@ function chooseBetterSymbol(s: Symbol): Symbol {
: s;
}

function getArgumentIndex(argumentsList: Node, node: Node) {
function getArgumentIndex(argumentsList: Node, node: Node, checker: TypeChecker) {
// The list we got back can include commas. In the presence of errors it may
// also just have nodes without commas. For example "Foo(a b c)" will have 3
// args without commas. We want to find what index we're at. So we count
Expand All @@ -488,20 +493,39 @@ function getArgumentIndex(argumentsList: Node, node: Node) {
// on. In that case, even if we're after the trailing comma, we'll still see
// that trailing comma in the list, and we'll have generated the appropriate
// arg index.
const args = argumentsList.getChildren();
let argumentIndex = 0;
for (const child of argumentsList.getChildren()) {
for (let pos = 0; pos < length(args); pos++) {
const child = args[pos];
if (child === node) {
break;
}
if (child.kind !== SyntaxKind.CommaToken) {
argumentIndex++;
if (isSpreadElement(child)) {
argumentIndex = argumentIndex + getSpreadElementCount(child, checker) + (pos > 0 ? pos : 0);
}
else {
if (child.kind !== SyntaxKind.CommaToken) {
argumentIndex++;
}
}
}

return argumentIndex;
}

function getArgumentCount(argumentsList: Node, ignoreTrailingComma: boolean) {
function getSpreadElementCount(node: SpreadElement, checker: TypeChecker) {
const spreadType = checker.getTypeAtLocation(node.expression);
if (checker.isTupleType(spreadType)) {
const { elementFlags, fixedLength } = (spreadType as TupleTypeReference).target;
if (fixedLength === 0) {
return 0;
}
const firstOptionalIndex = findIndex(elementFlags, f => !(f & ElementFlags.Required));
return firstOptionalIndex < 0 ? fixedLength : firstOptionalIndex;
}
return 0;
}

function getArgumentCount(argumentsList: Node, ignoreTrailingComma: boolean, checker: TypeChecker) {
// The argument count for a list is normally the number of non-comma children it has.
// For example, if you have "Foo(a,b)" then there will be three children of the arg
// list 'a' '<comma>' 'b'. So, in this case the arg count will be 2. However, there
Expand All @@ -515,7 +539,14 @@ function getArgumentCount(argumentsList: Node, ignoreTrailingComma: boolean) {
// arg count of 3.
const listChildren = argumentsList.getChildren();

let argumentCount = countWhere(listChildren, arg => arg.kind !== SyntaxKind.CommaToken);
let argumentCount = 0;
for (const child of listChildren) {
if (isSpreadElement(child)) {
argumentCount = argumentCount + getSpreadElementCount(child, checker);
}
}

argumentCount = argumentCount + countWhere(listChildren, arg => arg.kind !== SyntaxKind.CommaToken);
if (!ignoreTrailingComma && listChildren.length > 0 && last(listChildren).kind === SyntaxKind.CommaToken) {
argumentCount++;
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/stringCompletions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ function getStringLiteralCompletionEntries(sourceFile: SourceFile, node: StringL
case SyntaxKind.NewExpression:
case SyntaxKind.JsxAttribute:
if (!isRequireCallArgument(node) && !isImportCall(parent)) {
const argumentInfo = SignatureHelp.getArgumentInfoForCompletions(parent.kind === SyntaxKind.JsxAttribute ? parent.parent : node, position, sourceFile);
const argumentInfo = SignatureHelp.getArgumentInfoForCompletions(parent.kind === SyntaxKind.JsxAttribute ? parent.parent : node, position, sourceFile, typeChecker);
// Get string literal completions from specialized signatures of the target
// i.e. declare function f(a: 'A');
// f("/*completion position*/")
Expand Down
Loading

0 comments on commit 77acb4a

Please sign in to comment.