Skip to content

Commit 107164d

Browse files
Account for type parameters in missing function codefix
1 parent 8ed846c commit 107164d

10 files changed

+252
-15
lines changed

src/services/codefixes/helpers.ts

Lines changed: 64 additions & 8 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(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,7 +348,28 @@ namespace ts.codefix {
349348
}
350349
}
351350

352-
export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
351+
function createTypeParametersForArguments(argumentTypeParameters: string[], typeArguments: NodeArray<TypeNode> | undefined) {
352+
const usedNames = new Set(argumentTypeParameters);
353+
354+
if (typeArguments) {
355+
for (let i = 0, targetSize = usedNames.size + typeArguments.length; usedNames.size < targetSize; i += 1) {
356+
usedNames.add(
357+
CharacterCodes.T + i <= CharacterCodes.Z
358+
? String.fromCharCode(CharacterCodes.T + i)
359+
: `T${i}`
360+
);
361+
}
362+
}
363+
364+
return map(
365+
arrayFrom(usedNames.values()),
366+
(usedName) => factory.createTypeParameterDeclaration(/*modifiers*/ undefined, usedName),
367+
);
368+
}
369+
370+
export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, instanceType: Type, contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
371+
// Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {"
372+
const type = checker.getBaseTypeOfLiteralType(instanceType);
353373
let typeNode = checker.typeToTypeNode(type, contextNode, flags, tracker);
354374
if (typeNode && isImportTypeNode(typeNode)) {
355375
const importableReference = tryGetAutoImportableReferenceFromTypeNode(typeNode, scriptTarget);
@@ -362,6 +382,42 @@ namespace ts.codefix {
362382
return getSynthesizedDeepClone(typeNode);
363383
}
364384

385+
export function getArgumentTypesAndTypeParameters(checker: TypeChecker, importAdder: ImportAdder, instanceTypes: Type[], contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker) {
386+
const argumentTypeNodes: TypeNode[] = [];
387+
const argumentTypeParameters = new Set<string>();
388+
389+
for (const instanceType of instanceTypes) {
390+
const argumentTypeNode = typeToAutoImportableTypeNode(checker, importAdder, instanceType, contextNode, scriptTarget, flags, tracker);
391+
if (!argumentTypeNode) {
392+
continue;
393+
}
394+
395+
argumentTypeNodes.push(argumentTypeNode);
396+
const argumentTypeParameter = getFirstTypeParameterName(instanceType);
397+
398+
if (argumentTypeParameter) {
399+
argumentTypeParameters.add(argumentTypeParameter);
400+
}
401+
}
402+
403+
return { argumentTypeNodes, argumentTypeParameters: arrayFrom(argumentTypeParameters.values()) };
404+
}
405+
406+
function getFirstTypeParameterName(type: Type): string | undefined {
407+
if (type.flags & TypeFlags.Intersection) {
408+
for (const subType of (type as IntersectionType).types) {
409+
const subTypeName = getFirstTypeParameterName(subType);
410+
if (subTypeName) {
411+
return subTypeName;
412+
}
413+
}
414+
}
415+
416+
return type.flags & TypeFlags.TypeParameter
417+
? type.getSymbol()?.getName()
418+
: undefined;
419+
}
420+
365421
function createDummyParameters(argCount: number, names: (string | undefined)[] | undefined, types: (TypeNode | undefined)[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] {
366422
const parameters: ParameterDeclaration[] = [];
367423
for (let i = 0; i < argCount; i++) {

src/services/refactors/extractSymbol.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -878,13 +878,16 @@ namespace ts.refactor.extractSymbol {
878878
const callArguments: Identifier[] = [];
879879
let writes: UsageEntry[] | undefined;
880880
usagesInScope.forEach((usage, name) => {
881-
let typeNode: TypeNode | undefined;
882-
if (!isJS) {
883-
let type = checker.getTypeOfSymbolAtLocation(usage.symbol, usage.node);
884-
// Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {"
885-
type = checker.getBaseTypeOfLiteralType(type);
886-
typeNode = codefix.typeToAutoImportableTypeNode(checker, importAdder, type, scope, scriptTarget, NodeBuilderFlags.NoTruncation);
887-
}
881+
const typeNode = isJS
882+
? undefined
883+
: codefix.typeToAutoImportableTypeNode(
884+
checker,
885+
importAdder,
886+
checker.getTypeOfSymbolAtLocation(usage.symbol, usage.node),
887+
scope,
888+
scriptTarget,
889+
NodeBuilderFlags.NoTruncation,
890+
);
888891

889892
const paramDecl = factory.createParameterDeclaration(
890893
/*modifiers*/ undefined,
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*/(value);
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(value);
16+
}
17+
18+
function added<T>(value: T) {
19+
throw new Error("Function not implemented.");
20+
}
21+
`,
22+
});
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+
});
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>(value: T) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});
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 & number) {
21+
throw new Error("Function not implemented.");
22+
}
23+
`,
24+
});
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, t2a: T2, t2c: T2, t2b: T2) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});
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, U extends T>(value: T, other: U) {
5+
//// added/*1*/(value, other);
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, U extends T>(value: T, other: U) {
13+
added(value, other);
14+
}
15+
16+
function added<T, U>(value: T, other: U) {
17+
throw new Error("Function not implemented.");
18+
}
19+
`,
20+
});
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 outer<O>(o: O) {
5+
//// return function inner<I>(i: I) {
6+
//// added/*1*/(o, i);
7+
//// }
8+
////}
9+
10+
goTo.marker("1");
11+
verify.codeFix({
12+
description: "Add missing function declaration 'added'",
13+
index: 0,
14+
newFileContent: `function outer<O>(o: O) {
15+
return function inner<I>(i: I) {
16+
added(o, i);
17+
}
18+
}
19+
20+
function added<O, I>(o: O, i: I) {
21+
throw new Error("Function not implemented.");
22+
}
23+
`,
24+
});
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitAny: true
4+
////function outer<O>(o: O) {
5+
//// return function middle<M>(m: M) {
6+
//// return function inner<I>(i: I) {
7+
//// added/*1*/(o, m, i);
8+
//// }
9+
//// }
10+
////}
11+
12+
goTo.marker("1");
13+
verify.codeFix({
14+
description: "Add missing function declaration 'added'",
15+
index: 0,
16+
newFileContent: `function outer<O>(o: O) {
17+
return function middle<M>(m: M) {
18+
return function inner<I>(i: I) {
19+
added(o, m, i);
20+
}
21+
}
22+
}
23+
24+
function added<O, M, I>(o: O, m: M, i: I) {
25+
throw new Error("Function not implemented.");
26+
}
27+
`,
28+
});

0 commit comments

Comments
 (0)