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.

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

feat(51870): Add a Quick Fix to add an additional parameter to a method or function #56411

Merged
merged 7 commits into from Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
Expand Up @@ -7796,6 +7796,18 @@
"category": "Message",
"code": 95190
},
"Add optional parameter to '{0}'": {
"category": "Message",
"code": 95191
},
"Add optional parameters to '{0}'": {
"category": "Message",
"code": 95192
},
"Add all optional parameters": {
"category": "Message",
"code": 95193
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
206 changes: 139 additions & 67 deletions src/services/codefixes/fixAddMissingParam.ts
@@ -1,13 +1,14 @@
import {
append,
ArrowFunction,
CodeFixAction,
declarationNameToString,
DiagnosticOrDiagnosticAndArguments,
Diagnostics,
factory,
find,
findAncestor,
first,
firstDefined,
forEach,
FunctionDeclaration,
FunctionExpression,
Expand All @@ -23,46 +24,84 @@ import {
isSourceFileFromLibrary,
isVariableDeclaration,
last,
lastOrUndefined,
length,
map,
mapDefined,
MethodDeclaration,
Node,
NodeBuilderFlags,
ParameterDeclaration,
Program,
QuestionToken,
Signature,
SignatureKind,
sort,
some,
SourceFile,
SyntaxKind,
textChanges,
Type,
TypeChecker,
TypeNode,
} from "../_namespaces/ts";
import {
codeFixAll,
createCodeFixAction,
registerCodeFix,
} from "../_namespaces/ts.codefix";

const fixId = "addMissingParam";
const addMissingParamFixId = "addMissingParam";
const addOptionalParamFixId = "addOptionalParam";
const errorCodes = [Diagnostics.Expected_0_arguments_but_got_1.code];

registerCodeFix({
errorCodes,
fixIds: [fixId],
fixIds: [addMissingParamFixId, addOptionalParamFixId],
getCodeActions(context) {
const info = getInfo(context.sourceFile, context.program, context.span.start);
if (info === undefined) return undefined;

const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, info));
return [createCodeFixAction(fixId, changes, info.description, fixId, Diagnostics.Add_all_missing_parameters)];
const { name, declarations, newParameters, newOptionalParameters } = info;
const actions: CodeFixAction[] = [];

if (length(newParameters)) {
append(
actions,
createCodeFixAction(
addMissingParamFixId,
textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, declarations, newParameters)),
[length(newParameters) > 1 ? Diagnostics.Add_missing_parameters_to_0 : Diagnostics.Add_missing_parameter_to_0, name],
addMissingParamFixId,
Diagnostics.Add_all_missing_parameters,
),
);
}

if (length(newOptionalParameters)) {
append(
actions,
createCodeFixAction(
addOptionalParamFixId,
textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, declarations, newOptionalParameters)),
[length(newOptionalParameters) > 1 ? Diagnostics.Add_optional_parameters_to_0 : Diagnostics.Add_optional_parameter_to_0, name],
addOptionalParamFixId,
Diagnostics.Add_all_optional_parameters,
),
);
}

return actions;
},
getAllCodeActions: context =>
codeFixAll(context, errorCodes, (changes, diag) => {
const info = getInfo(context.sourceFile, context.program, diag.start);
if (info) {
doChange(changes, context.sourceFile, info);
const { declarations, newParameters, newOptionalParameters } = info;
if (context.fixId === addMissingParamFixId) {
doChange(changes, context.sourceFile, declarations, newParameters);
}
if (context.fixId === addOptionalParamFixId) {
doChange(changes, context.sourceFile, declarations, newOptionalParameters);
}
}
}),
});
Expand All @@ -74,10 +113,10 @@ type ConvertibleSignatureDeclaration =
| MethodDeclaration;

interface SignatureInfo {
readonly description: DiagnosticOrDiagnosticAndArguments;
readonly newParameters: ParameterInfo[];
readonly newOptionalParameters: ParameterInfo[];
readonly name: string;
readonly declarations: ConvertibleSignatureDeclaration[];
readonly overload: ConvertibleSignatureDeclaration | undefined;
}

interface ParameterInfo {
Expand All @@ -88,76 +127,80 @@ interface ParameterInfo {
function getInfo(sourceFile: SourceFile, program: Program, pos: number): SignatureInfo | undefined {
const token = getTokenAtPosition(sourceFile, pos);
const callExpression = findAncestor(token, isCallExpression);
if (callExpression === undefined || length(callExpression.arguments) === 0) return undefined;
if (callExpression === undefined || length(callExpression.arguments) === 0) {
return undefined;
}

const checker = program.getTypeChecker();
const type = checker.getTypeAtLocation(callExpression.expression);
const signatures = checker.getSignaturesOfType(type, SignatureKind.Call);
const signature = lastOrUndefined(sort(signatures, (a, b) => length(a.parameters) - length(b.parameters)));
if (
signature &&
signature.declaration &&
isConvertibleSignatureDeclaration(signature.declaration)
) {
// find a non-overload signature
const declaration = (signature.declaration.body === undefined ? find(signature.declaration.symbol.declarations, d => isConvertibleSignatureDeclaration(d) && !!d.body) : signature.declaration) as ConvertibleSignatureDeclaration;
if (declaration === undefined) {
return undefined;
}

if (isSourceFileFromLibrary(program, declaration.getSourceFile())) {
return undefined;
// find a non-overload signature
sandersn marked this conversation as resolved.
Show resolved Hide resolved
const declaration = firstDefined(signatures, s => {
if (s.declaration && isConvertibleSignatureDeclaration(s.declaration)) {
return s.declaration.body ? s.declaration :
find(s.declaration.symbol.declarations, d => isConvertibleSignatureDeclaration(d) && !!d.body) as ConvertibleSignatureDeclaration;
sandersn marked this conversation as resolved.
Show resolved Hide resolved
}
return undefined;
});

if (declaration === undefined || isSourceFileFromLibrary(program, declaration.getSourceFile())) {
return undefined;
}

const overload = signature.declaration.body === undefined ? signature.declaration : undefined;
if (overload && length(overload.parameters) !== length(declaration.parameters)) return undefined;
const name = tryGetName(declaration);
if (name === undefined) {
return undefined;
}

const name = tryGetName(declaration);
if (name === undefined) return undefined;
const newParameters: ParameterInfo[] = [];
const newOptionalParameters: ParameterInfo[] = [];
const parametersLength = length(declaration.parameters);
const argumentsLength = length(callExpression.arguments);
if (parametersLength > argumentsLength) {
return undefined;
}

const declarations: ConvertibleSignatureDeclaration[] = append([declaration], overload);
const newParameters: ParameterInfo[] = [];
const parametersLength = length(declaration.parameters);
const argumentsLength = length(callExpression.arguments);
if (parametersLength > argumentsLength) return undefined;
const overloads = getOverloads(declaration, signatures);
for (let i = 0, pos = 0, paramIndex = 0; i < argumentsLength; i++) {
const arg = callExpression.arguments[i];
const expr = isAccessExpression(arg) ? getNameOfAccessExpression(arg) : arg;
const type = checker.getWidenedType(checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)));
const parameter = pos < parametersLength ? declaration.parameters[pos] : undefined;
if (
parameter &&
checker.isTypeAssignableTo(type, checker.getTypeAtLocation(parameter))
) {
pos++;
sandersn marked this conversation as resolved.
Show resolved Hide resolved
}
else {
const name = expr && isIdentifier(expr) ? expr.text : `p${paramIndex++}`;
const typeNode = typeToTypeNode(checker, type, declaration);
append(newParameters, {
pos: i,
declaration: createParameter(name, typeNode, /*questionToken*/ undefined),
});

for (let i = 0, pos = 0, paramIndex = 0; i < argumentsLength; i++) {
const arg = callExpression.arguments[i];
const expr = isAccessExpression(arg) ? getNameOfAccessExpression(arg) : arg;
const type = checker.getWidenedType(checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)));
const parameter = pos < parametersLength ? declaration.parameters[pos] : undefined;
if (
parameter &&
checker.isTypeAssignableTo(type, checker.getTypeAtLocation(parameter))
length(overloads) &&
some(overloads, d => pos < length(d.parameters) && !!d.parameters[pos] && d.parameters[pos].questionToken === undefined)
sandersn marked this conversation as resolved.
Show resolved Hide resolved
) {
pos++;
}
else {
const newParameter = {
pos: i,
declaration: factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
expr && isIdentifier(expr) ? expr.text : `p${paramIndex++}`,
/*questionToken*/ undefined,
typeToTypeNode(checker, type, declaration),
/*initializer*/ undefined,
),
};
append(newParameters, newParameter);
continue;
}

append(newOptionalParameters, {
pos: i,
declaration: createParameter(name, typeNode, factory.createToken(SyntaxKind.QuestionToken)),
});
}
return {
declarations,
newParameters,
overload,
description: [
length(newParameters) > 1 ? Diagnostics.Add_missing_parameters_to_0 : Diagnostics.Add_missing_parameter_to_0,
declarationNameToString(name),
],
};
}
return undefined;

return {
newParameters,
newOptionalParameters,
name: declarationNameToString(name),
declarations: [declaration, ...overloads],
};
}

function tryGetName(node: FunctionLikeDeclaration) {
sandersn marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -176,10 +219,16 @@ function tryGetName(node: FunctionLikeDeclaration) {
}

function typeToTypeNode(checker: TypeChecker, type: Type, enclosingDeclaration: Node) {
return checker.typeToTypeNode(checker.getWidenedType(type), enclosingDeclaration, NodeBuilderFlags.NoTruncation) ?? factory.createKeywordTypeNode(SyntaxKind.UnknownKeyword);
return checker.typeToTypeNode(checker.getWidenedType(type), enclosingDeclaration, NodeBuilderFlags.NoTruncation)
?? factory.createKeywordTypeNode(SyntaxKind.UnknownKeyword);
}

function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, { declarations, newParameters }: SignatureInfo) {
function doChange(
changes: textChanges.ChangeTracker,
sourceFile: SourceFile,
declarations: ConvertibleSignatureDeclaration[],
newParameters: ParameterInfo[],
) {
forEach(declarations, declaration => {
if (length(declaration.parameters)) {
changes.replaceNodeRangeWithNodes(
Expand Down Expand Up @@ -248,3 +297,26 @@ function updateParameters(node: ConvertibleSignatureDeclaration, newParameters:
}
return parameters;
}

function getOverloads(implementation: ConvertibleSignatureDeclaration, signatures: readonly Signature[]): ConvertibleSignatureDeclaration[] {
const maxParams = Math.max(...mapDefined(signatures, s => isOverload(s) ? length(s.parameters) : undefined));
sandersn marked this conversation as resolved.
Show resolved Hide resolved
if (length(implementation.parameters) === maxParams) {
return mapDefined(signatures, s => isOverload(s) && length(s.parameters) === maxParams ? s.declaration : undefined) as ConvertibleSignatureDeclaration[];
sandersn marked this conversation as resolved.
Show resolved Hide resolved
}
return [];
}

function isOverload(signature: Signature) {
return !!signature.declaration && isConvertibleSignatureDeclaration(signature.declaration) && signature.declaration.body === undefined;
}

function createParameter(name: string, type: TypeNode, questionToken: QuestionToken | undefined) {
return factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
name,
questionToken,
type,
/*initializer*/ undefined,
);
}
20 changes: 20 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingParam16.ts
@@ -0,0 +1,20 @@
/// <reference path="fourslash.ts" />

////function f(a: string, b: string): string
////function f(a: number, b: number): number
////function f(a: number | string, b: number | string): number | string {
//// return a + b;
////}
////f(1, 2, "")

verify.codeFix({
description: [ts.Diagnostics.Add_missing_parameter_to_0.message, "f"],
index: 0,
newFileContent:
`function f(a: string, b: string, p0: string): string
function f(a: number, b: number, p0: string): number
function f(a: number | string, b: number | string, p0: string): number | string {
return a + b;
}
f(1, 2, "")`
});
12 changes: 12 additions & 0 deletions tests/cases/fourslash/codeFixAddOptionalParam1.ts
@@ -0,0 +1,12 @@
/// <reference path="fourslash.ts" />

////[|function f() {}|]
////
////const a = 1;
////f(a);

verify.codeFix({
description: [ts.Diagnostics.Add_optional_parameter_to_0.message, "f"],
index: 1,
newRangeContent: "function f(a?: number) {}"
});
13 changes: 13 additions & 0 deletions tests/cases/fourslash/codeFixAddOptionalParam10.ts
@@ -0,0 +1,13 @@
/// <reference path="fourslash.ts" />

////[|function f() {}|]
////
////const a = 1;
////const b = "";
////f(a, b, true);

verify.codeFix({
description: [ts.Diagnostics.Add_optional_parameters_to_0.message, "f"],
index: 1,
newRangeContent: "function f(a?: number, b?: string, p0?: boolean) {}"
});
14 changes: 14 additions & 0 deletions tests/cases/fourslash/codeFixAddOptionalParam11.ts
@@ -0,0 +1,14 @@
/// <reference path="fourslash.ts" />

////class C {
//// [|private p = () => {}|]
//// m(a: boolean) {
//// this.p(a);
//// }
////}

verify.codeFix({
description: [ts.Diagnostics.Add_optional_parameter_to_0.message, "p"],
index: 1,
newRangeContent: "private p = (a?: boolean) => {}"
});
11 changes: 11 additions & 0 deletions tests/cases/fourslash/codeFixAddOptionalParam12.ts
@@ -0,0 +1,11 @@
/// <reference path="fourslash.ts" />

////function f([|cb = () => {}|]) {
//// cb("");
////}

verify.codeFix({
description: [ts.Diagnostics.Add_optional_parameter_to_0.message, "cb"],
index: 1,
newRangeContent: "cb = (p0?: string) => {}"
});