Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(48260): Incorrect parameter hint is highlighted when arguments contain spread syntax #56372

Merged
merged 2 commits into from
Dec 4, 2023
Merged

fix(48260): Incorrect parameter hint is highlighted when arguments contain spread syntax #56372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a30ec40
fix(48260): handle spread elements in the signature help
a-tarasyuk Nov 12, 2023
df63b05
Merge branch 'main' into fix/48260
sandersn Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/services/completions.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading