Skip to content

Commit ebd42ab

Browse files
JoshuaKGoldbergsandersn
JoshuaKGoldberg
and
sandersn
authored
Account for type parameters in missing function codefix (#49727)
* Account for type parameters in missing function codefix * Apply suggestions from code review Co-authored-by: Nathan Shively-Sanders <[email protected]> * WIP * Synthesize new type parameters instead of deep unions and intersections * Pass along type parameter constraints * E.T. phone home * Clean up comments just a bit * Only widen the instance type sometimes Co-authored-by: Nathan Shively-Sanders <[email protected]>
1 parent 78e2bfd commit ebd42ab

15 files changed

+444
-10
lines changed

‎src/services/codefixes/helpers.ts‎

Lines changed: 142 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -295,20 +295,19 @@ namespace ts.codefix {
295295
const contextualType = isJs ? undefined : checker.getContextualType(call);
296296
const names = map(args, arg =>
297297
isIdentifier(arg) ? arg.text : isPropertyAccessExpression(arg) && isIdentifier(arg.name) ? arg.name.text : undefined);
298-
const types = isJs ? [] : map(args, arg =>
299-
typeToAutoImportableTypeNode(checker, importAdder, checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)), contextNode, scriptTarget, /*flags*/ undefined, tracker));
298+
const instanceTypes = isJs ? [] : map(args, arg => checker.getTypeAtLocation(arg));
299+
const { argumentTypeNodes, argumentTypeParameters } = getArgumentTypesAndTypeParameters(
300+
checker, importAdder, instanceTypes, contextNode, scriptTarget, /*flags*/ undefined, tracker
301+
);
300302

301303
const modifiers = modifierFlags
302304
? factory.createNodeArray(factory.createModifiersFromModifierFlags(modifierFlags))
303305
: undefined;
304306
const asteriskToken = isYieldExpression(parent)
305307
? factory.createToken(SyntaxKind.AsteriskToken)
306308
: undefined;
307-
const typeParameters = isJs || typeArguments === undefined
308-
? undefined
309-
: map(typeArguments, (_, i) =>
310-
factory.createTypeParameterDeclaration(/*modifiers*/ undefined, CharacterCodes.T + typeArguments.length - 1 <= CharacterCodes.Z ? String.fromCharCode(CharacterCodes.T + i) : `T${i}`));
311-
const parameters = createDummyParameters(args.length, names, types, /*minArgumentCount*/ undefined, isJs);
309+
const typeParameters = isJs ? undefined : createTypeParametersForArguments(checker, argumentTypeParameters, typeArguments);
310+
const parameters = createDummyParameters(args.length, names, argumentTypeNodes, /*minArgumentCount*/ undefined, isJs);
312311
const type = isJs || contextualType === undefined
313312
? undefined
314313
: checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker);
@@ -349,6 +348,35 @@ namespace ts.codefix {
349348
}
350349
}
351350

351+
interface ArgumentTypeParameterAndConstraint {
352+
argumentType: Type;
353+
constraint?: TypeNode;
354+
}
355+
356+
function createTypeParametersForArguments(checker: TypeChecker, argumentTypeParameters: [string, ArgumentTypeParameterAndConstraint | undefined][], typeArguments: NodeArray<TypeNode> | undefined) {
357+
const usedNames = new Set(argumentTypeParameters.map(pair => pair[0]));
358+
const constraintsByName = new Map(argumentTypeParameters);
359+
360+
if (typeArguments) {
361+
const typeArgumentsWithNewTypes = typeArguments.filter(typeArgument => !argumentTypeParameters.some(pair => checker.getTypeAtLocation(typeArgument) === pair[1]?.argumentType));
362+
const targetSize = usedNames.size + typeArgumentsWithNewTypes.length;
363+
for (let i = 0; usedNames.size < targetSize; i += 1) {
364+
usedNames.add(createTypeParameterName(i));
365+
}
366+
}
367+
368+
return map(
369+
arrayFrom(usedNames.values()),
370+
usedName => factory.createTypeParameterDeclaration(/*modifiers*/ undefined, usedName, constraintsByName.get(usedName)?.constraint),
371+
);
372+
}
373+
374+
function createTypeParameterName(index: number) {
375+
return CharacterCodes.T + index <= CharacterCodes.Z
376+
? String.fromCharCode(CharacterCodes.T + index)
377+
: `T${index}`;
378+
}
379+
352380
export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
353381
let typeNode = checker.typeToTypeNode(type, contextNode, flags, tracker);
354382
if (typeNode && isImportTypeNode(typeNode)) {
@@ -358,19 +386,124 @@ namespace ts.codefix {
358386
typeNode = importableReference.typeNode;
359387
}
360388
}
389+
361390
// Ensure nodes are fresh so they can have different positions when going through formatting.
362391
return getSynthesizedDeepClone(typeNode);
363392
}
364393

394+
function typeContainsTypeParameter(type: Type) {
395+
if (type.isUnionOrIntersection()) {
396+
return type.types.some(typeContainsTypeParameter);
397+
}
398+
399+
return type.flags & TypeFlags.TypeParameter;
400+
}
401+
402+
export function getArgumentTypesAndTypeParameters(checker: TypeChecker, importAdder: ImportAdder, instanceTypes: Type[], contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker) {
403+
// Types to be used as the types of the parameters in the new function
404+
// E.g. from this source:
405+
// added("", 0)
406+
// The value will look like:
407+
// [{ typeName: { text: "string" } }, { typeName: { text: "number" }]
408+
// And in the output function will generate:
409+
// function added(a: string, b: number) { ... }
410+
const argumentTypeNodes: TypeNode[] = [];
411+
412+
// Names of type parameters provided as arguments to the call
413+
// E.g. from this source:
414+
// added<T, U>(value);
415+
// The value will look like:
416+
// [
417+
// ["T", { argumentType: { typeName: { text: "T" } } } ],
418+
// ["U", { argumentType: { typeName: { text: "U" } } } ],
419+
// ]
420+
// And in the output function will generate:
421+
// function added<T, U>() { ... }
422+
const argumentTypeParameters = new Map<string, ArgumentTypeParameterAndConstraint | undefined>();
423+
424+
for (let i = 0; i < instanceTypes.length; i += 1) {
425+
const instanceType = instanceTypes[i];
426+
427+
// If the instance type contains a deep reference to an existing type parameter,
428+
// instead of copying the full union or intersection, create a new type parameter
429+
// E.g. from this source:
430+
// function existing<T, U>(value: T | U & string) {
431+
// added/*1*/(value);
432+
// We don't want to output this:
433+
// function added<T>(value: T | U & string) { ... }
434+
// We instead want to output:
435+
// function added<T>(value: T) { ... }
436+
if (instanceType.isUnionOrIntersection() && instanceType.types.some(typeContainsTypeParameter)) {
437+
const synthesizedTypeParameterName = createTypeParameterName(i);
438+
argumentTypeNodes.push(factory.createTypeReferenceNode(synthesizedTypeParameterName));
439+
argumentTypeParameters.set(synthesizedTypeParameterName, undefined);
440+
continue;
441+
}
442+
443+
// Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {"
444+
const widenedInstanceType = checker.getBaseTypeOfLiteralType(instanceType);
445+
const argumentTypeNode = typeToAutoImportableTypeNode(checker, importAdder, widenedInstanceType, contextNode, scriptTarget, flags, tracker);
446+
if (!argumentTypeNode) {
447+
continue;
448+
}
449+
450+
argumentTypeNodes.push(argumentTypeNode);
451+
const argumentTypeParameter = getFirstTypeParameterName(instanceType);
452+
453+
// If the instance type is a type parameter with a constraint (other than an anonymous object),
454+
// remember that constraint for when we create the new type parameter
455+
// E.g. from this source:
456+
// function existing<T extends string>(value: T) {
457+
// added/*1*/(value);
458+
// We don't want to output this:
459+
// function added<T>(value: T) { ... }
460+
// We instead want to output:
461+
// function added<T extends string>(value: T) { ... }
462+
const instanceTypeConstraint = instanceType.isTypeParameter() && instanceType.constraint && !isAnonymousObjectConstraintType(instanceType.constraint)
463+
? typeToAutoImportableTypeNode(checker, importAdder, instanceType.constraint, contextNode, scriptTarget, flags, tracker)
464+
: undefined;
465+
466+
if (argumentTypeParameter) {
467+
argumentTypeParameters.set(argumentTypeParameter, { argumentType: instanceType, constraint: instanceTypeConstraint });
468+
}
469+
}
470+
471+
return { argumentTypeNodes, argumentTypeParameters: arrayFrom(argumentTypeParameters.entries()) };
472+
}
473+
474+
function isAnonymousObjectConstraintType(type: Type) {
475+
return (type.flags & TypeFlags.Object) && (type as ObjectType).objectFlags === ObjectFlags.Anonymous;
476+
}
477+
478+
function getFirstTypeParameterName(type: Type): string | undefined {
479+
if (type.flags & (TypeFlags.Union | TypeFlags.Intersection)) {
480+
for (const subType of (type as UnionType | IntersectionType).types) {
481+
const subTypeName = getFirstTypeParameterName(subType);
482+
if (subTypeName) {
483+
return subTypeName;
484+
}
485+
}
486+
}
487+
488+
return type.flags & TypeFlags.TypeParameter
489+
? type.getSymbol()?.getName()
490+
: undefined;
491+
}
492+
365493
function createDummyParameters(argCount: number, names: (string | undefined)[] | undefined, types: (TypeNode | undefined)[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] {
366494
const parameters: ParameterDeclaration[] = [];
495+
const parameterNameCounts = new Map<string, number>();
367496
for (let i = 0; i < argCount; i++) {
497+
const parameterName = names?.[i] || `arg${i}`;
498+
const parameterNameCount = parameterNameCounts.get(parameterName);
499+
parameterNameCounts.set(parameterName, (parameterNameCount || 0) + 1);
500+
368501
const newParameter = factory.createParameterDeclaration(
369502
/*modifiers*/ undefined,
370503
/*dotDotDotToken*/ undefined,
371-
/*name*/ names && names[i] || `arg${i}`,
504+
/*name*/ parameterName + (parameterNameCount || ""),
372505
/*questionToken*/ minArgumentCount !== undefined && i >= minArgumentCount ? factory.createToken(SyntaxKind.QuestionToken) : undefined,
373-
/*type*/ inJs ? undefined : types && types[i] || factory.createKeywordTypeNode(SyntaxKind.UnknownKeyword),
506+
/*type*/ inJs ? undefined : types?.[i] || factory.createKeywordTypeNode(SyntaxKind.UnknownKeyword),
374507
/*initializer*/ undefined);
375508
parameters.push(newParameter);
376509
}

‎tests/cases/fourslash/codeFixUndeclaredMethod.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ verify.codeFix({
6363
// 8 type args
6464
this.foo3<1,2,3,4,5,6,7,8>();
6565
}
66-
foo3<T0, T1, T2, T3, T4, T5, T6, T7>() {
66+
foo3<T, U, V, W, X, Y, Z, T7>() {
6767
throw new Error("Method not implemented.");
6868
}
6969
foo2<T, U, V, W, X, Y, Z>() {

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeKeyof.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function existing<T>(value: T) {
5+
//// const keyofTypeof = Object.keys(value)[0] as keyof T;
6+
//// added/*1*/(keyofTypeof);
7+
////}
8+
9+
goTo.marker("1");
10+
verify.codeFix({
11+
description: "Add missing function declaration 'added'",
12+
index: 0,
13+
newFileContent: `function existing<T>(value: T) {
14+
const keyofTypeof = Object.keys(value)[0] as keyof T;
15+
added(keyofTypeof);
16+
}
17+
18+
function added(keyofTypeof: string | number | symbol) {
19+
throw new Error("Function not implemented.");
20+
}
21+
`,
22+
});

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeParameter.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function existing<T>(value: T) {
5+
//// added/*1*/(value);
6+
////}
7+
8+
goTo.marker("1");
9+
verify.codeFix({
10+
description: "Add missing function declaration 'added'",
11+
index: 0,
12+
newFileContent: `function existing<T>(value: T) {
13+
added(value);
14+
}
15+
16+
function added<T>(value: T) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeParameterArgumentDifferent.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function existing<T>(value: T) {
5+
//// added/*1*/<T, string>(value, "");
6+
////}
7+
8+
goTo.marker("1");
9+
verify.codeFix({
10+
description: "Add missing function declaration 'added'",
11+
index: 0,
12+
newFileContent: `function existing<T>(value: T) {
13+
added<T, string>(value, "");
14+
}
15+
16+
function added<T, U>(value: T, arg1: string) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeParameterArgumentSame.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function existing<T>(value: T) {
5+
//// added/*1*/<T>(value, value);
6+
////}
7+
8+
goTo.marker("1");
9+
verify.codeFix({
10+
description: "Add missing function declaration 'added'",
11+
index: 0,
12+
newFileContent: `function existing<T>(value: T) {
13+
added<T>(value, value);
14+
}
15+
16+
function added<T>(value: T, value1: T) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeParameterConstrained.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function existing<T extends string>(value: T) {
5+
//// added/*1*/(value);
6+
////}
7+
8+
goTo.marker("1");
9+
verify.codeFix({
10+
description: "Add missing function declaration 'added'",
11+
index: 0,
12+
newFileContent: `function existing<T extends string>(value: T) {
13+
added(value);
14+
}
15+
16+
function added<T extends string>(value: T) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeParameterNarrowed.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function existing<T>(value: T) {
5+
//// if (typeof value === "number") {
6+
//// added/*1*/(value);
7+
//// }
8+
////}
9+
10+
goTo.marker("1");
11+
verify.codeFix({
12+
description: "Add missing function declaration 'added'",
13+
index: 0,
14+
newFileContent: `function existing<T>(value: T) {
15+
if (typeof value === "number") {
16+
added(value);
17+
}
18+
}
19+
20+
function added<T>(value: T) {
21+
throw new Error("Function not implemented.");
22+
}
23+
`,
24+
});

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeParameterVariable.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function e<T extends "phone" | "home">() {
5+
//// let et: T = 'phone'
6+
//// added1/*1*/(et)
7+
//// et = 'home'
8+
//// added2/*2*/(et)
9+
////}
10+
11+
goTo.marker("1");
12+
verify.codeFix({
13+
description: "Add missing function declaration 'added1'",
14+
index: 0,
15+
newFileContent: `function e<T extends "phone" | "home">() {
16+
let et: T = 'phone'
17+
added1(et)
18+
et = 'home'
19+
added2(et)
20+
}
21+
22+
function added1(et: string) {
23+
throw new Error("Function not implemented.")
24+
}
25+
`,
26+
});
27+
28+
goTo.marker("2");
29+
verify.codeFix({
30+
description: "Add missing function declaration 'added1'",
31+
index: 0,
32+
newFileContent: `function e<T extends "phone" | "home">() {
33+
let et: T = 'phone'
34+
added1(et)
35+
et = 'home'
36+
added2(et)
37+
}
38+
39+
function added1(et: string) {
40+
throw new Error("Function not implemented.")
41+
}
42+
`,
43+
});

‎tests/cases/fourslash/incompleteFunctionCallCodefixTypeParameters.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function existing<T1, T, T2>(t1: T1, t: T, t2a: T2, t2b: T2, t2c: T2) {
5+
//// added/*1*/(t2a, t2b, t2c, t1, t, t2a, t2c, t2b);
6+
////}
7+
8+
goTo.marker("1");
9+
verify.codeFix({
10+
description: "Add missing function declaration 'added'",
11+
index: 0,
12+
newFileContent: `function existing<T1, T, T2>(t1: T1, t: T, t2a: T2, t2b: T2, t2c: T2) {
13+
added(t2a, t2b, t2c, t1, t, t2a, t2c, t2b);
14+
}
15+
16+
function added<T2, T1, T>(t2a: T2, t2b: T2, t2c: T2, t1: T1, t: T, t2a1: T2, t2c1: T2, t2b1: T2) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});

0 commit comments

Comments
 (0)