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 <293473+sandersn@users.noreply.github.com>

* 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 <293473+sandersn@users.noreply.github.com>
This commit is contained in:
Josh Goldberg 2022-07-26 18:22:19 -04:00 committed by GitHub
parent 78e2bfd712
commit ebd42abf95
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 444 additions and 10 deletions

View File

@ -295,8 +295,10 @@ namespace ts.codefix {
const contextualType = isJs ? undefined : checker.getContextualType(call);
const names = map(args, arg =>
isIdentifier(arg) ? arg.text : isPropertyAccessExpression(arg) && isIdentifier(arg.name) ? arg.name.text : undefined);
const types = isJs ? [] : map(args, arg =>
typeToAutoImportableTypeNode(checker, importAdder, checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)), contextNode, scriptTarget, /*flags*/ undefined, tracker));
const instanceTypes = isJs ? [] : map(args, arg => checker.getTypeAtLocation(arg));
const { argumentTypeNodes, argumentTypeParameters } = getArgumentTypesAndTypeParameters(
checker, importAdder, instanceTypes, contextNode, scriptTarget, /*flags*/ undefined, tracker
);
const modifiers = modifierFlags
? factory.createNodeArray(factory.createModifiersFromModifierFlags(modifierFlags))
@ -304,11 +306,8 @@ namespace ts.codefix {
const asteriskToken = isYieldExpression(parent)
? factory.createToken(SyntaxKind.AsteriskToken)
: undefined;
const typeParameters = isJs || typeArguments === undefined
? undefined
: map(typeArguments, (_, i) =>
factory.createTypeParameterDeclaration(/*modifiers*/ undefined, CharacterCodes.T + typeArguments.length - 1 <= CharacterCodes.Z ? String.fromCharCode(CharacterCodes.T + i) : `T${i}`));
const parameters = createDummyParameters(args.length, names, types, /*minArgumentCount*/ undefined, isJs);
const typeParameters = isJs ? undefined : createTypeParametersForArguments(checker, argumentTypeParameters, typeArguments);
const parameters = createDummyParameters(args.length, names, argumentTypeNodes, /*minArgumentCount*/ undefined, isJs);
const type = isJs || contextualType === undefined
? undefined
: checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker);
@ -349,6 +348,35 @@ namespace ts.codefix {
}
}
interface ArgumentTypeParameterAndConstraint {
argumentType: Type;
constraint?: TypeNode;
}
function createTypeParametersForArguments(checker: TypeChecker, argumentTypeParameters: [string, ArgumentTypeParameterAndConstraint | undefined][], typeArguments: NodeArray<TypeNode> | undefined) {
const usedNames = new Set(argumentTypeParameters.map(pair => pair[0]));
const constraintsByName = new Map(argumentTypeParameters);
if (typeArguments) {
const typeArgumentsWithNewTypes = typeArguments.filter(typeArgument => !argumentTypeParameters.some(pair => checker.getTypeAtLocation(typeArgument) === pair[1]?.argumentType));
const targetSize = usedNames.size + typeArgumentsWithNewTypes.length;
for (let i = 0; usedNames.size < targetSize; i += 1) {
usedNames.add(createTypeParameterName(i));
}
}
return map(
arrayFrom(usedNames.values()),
usedName => factory.createTypeParameterDeclaration(/*modifiers*/ undefined, usedName, constraintsByName.get(usedName)?.constraint),
);
}
function createTypeParameterName(index: number) {
return CharacterCodes.T + index <= CharacterCodes.Z
? String.fromCharCode(CharacterCodes.T + index)
: `T${index}`;
}
export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
let typeNode = checker.typeToTypeNode(type, contextNode, flags, tracker);
if (typeNode && isImportTypeNode(typeNode)) {
@ -358,19 +386,124 @@ namespace ts.codefix {
typeNode = importableReference.typeNode;
}
}
// Ensure nodes are fresh so they can have different positions when going through formatting.
return getSynthesizedDeepClone(typeNode);
}
function typeContainsTypeParameter(type: Type) {
if (type.isUnionOrIntersection()) {
return type.types.some(typeContainsTypeParameter);
}
return type.flags & TypeFlags.TypeParameter;
}
export function getArgumentTypesAndTypeParameters(checker: TypeChecker, importAdder: ImportAdder, instanceTypes: Type[], contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker) {
// Types to be used as the types of the parameters in the new function
// E.g. from this source:
// added("", 0)
// The value will look like:
// [{ typeName: { text: "string" } }, { typeName: { text: "number" }]
// And in the output function will generate:
// function added(a: string, b: number) { ... }
const argumentTypeNodes: TypeNode[] = [];
// Names of type parameters provided as arguments to the call
// E.g. from this source:
// added<T, U>(value);
// The value will look like:
// [
// ["T", { argumentType: { typeName: { text: "T" } } } ],
// ["U", { argumentType: { typeName: { text: "U" } } } ],
// ]
// And in the output function will generate:
// function added<T, U>() { ... }
const argumentTypeParameters = new Map<string, ArgumentTypeParameterAndConstraint | undefined>();
for (let i = 0; i < instanceTypes.length; i += 1) {
const instanceType = instanceTypes[i];
// If the instance type contains a deep reference to an existing type parameter,
// instead of copying the full union or intersection, create a new type parameter
// E.g. from this source:
// function existing<T, U>(value: T | U & string) {
// added/*1*/(value);
// We don't want to output this:
// function added<T>(value: T | U & string) { ... }
// We instead want to output:
// function added<T>(value: T) { ... }
if (instanceType.isUnionOrIntersection() && instanceType.types.some(typeContainsTypeParameter)) {
const synthesizedTypeParameterName = createTypeParameterName(i);
argumentTypeNodes.push(factory.createTypeReferenceNode(synthesizedTypeParameterName));
argumentTypeParameters.set(synthesizedTypeParameterName, undefined);
continue;
}
// Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {"
const widenedInstanceType = checker.getBaseTypeOfLiteralType(instanceType);
const argumentTypeNode = typeToAutoImportableTypeNode(checker, importAdder, widenedInstanceType, contextNode, scriptTarget, flags, tracker);
if (!argumentTypeNode) {
continue;
}
argumentTypeNodes.push(argumentTypeNode);
const argumentTypeParameter = getFirstTypeParameterName(instanceType);
// If the instance type is a type parameter with a constraint (other than an anonymous object),
// remember that constraint for when we create the new type parameter
// E.g. from this source:
// function existing<T extends string>(value: T) {
// added/*1*/(value);
// We don't want to output this:
// function added<T>(value: T) { ... }
// We instead want to output:
// function added<T extends string>(value: T) { ... }
const instanceTypeConstraint = instanceType.isTypeParameter() && instanceType.constraint && !isAnonymousObjectConstraintType(instanceType.constraint)
? typeToAutoImportableTypeNode(checker, importAdder, instanceType.constraint, contextNode, scriptTarget, flags, tracker)
: undefined;
if (argumentTypeParameter) {
argumentTypeParameters.set(argumentTypeParameter, { argumentType: instanceType, constraint: instanceTypeConstraint });
}
}
return { argumentTypeNodes, argumentTypeParameters: arrayFrom(argumentTypeParameters.entries()) };
}
function isAnonymousObjectConstraintType(type: Type) {
return (type.flags & TypeFlags.Object) && (type as ObjectType).objectFlags === ObjectFlags.Anonymous;
}
function getFirstTypeParameterName(type: Type): string | undefined {
if (type.flags & (TypeFlags.Union | TypeFlags.Intersection)) {
for (const subType of (type as UnionType | IntersectionType).types) {
const subTypeName = getFirstTypeParameterName(subType);
if (subTypeName) {
return subTypeName;
}
}
}
return type.flags & TypeFlags.TypeParameter
? type.getSymbol()?.getName()
: undefined;
}
function createDummyParameters(argCount: number, names: (string | undefined)[] | undefined, types: (TypeNode | undefined)[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] {
const parameters: ParameterDeclaration[] = [];
const parameterNameCounts = new Map<string, number>();
for (let i = 0; i < argCount; i++) {
const parameterName = names?.[i] || `arg${i}`;
const parameterNameCount = parameterNameCounts.get(parameterName);
parameterNameCounts.set(parameterName, (parameterNameCount || 0) + 1);
const newParameter = factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
/*name*/ names && names[i] || `arg${i}`,
/*name*/ parameterName + (parameterNameCount || ""),
/*questionToken*/ minArgumentCount !== undefined && i >= minArgumentCount ? factory.createToken(SyntaxKind.QuestionToken) : undefined,
/*type*/ inJs ? undefined : types && types[i] || factory.createKeywordTypeNode(SyntaxKind.UnknownKeyword),
/*type*/ inJs ? undefined : types?.[i] || factory.createKeywordTypeNode(SyntaxKind.UnknownKeyword),
/*initializer*/ undefined);
parameters.push(newParameter);
}

View File

@ -63,7 +63,7 @@ verify.codeFix({
// 8 type args
this.foo3<1,2,3,4,5,6,7,8>();
}
foo3<T0, T1, T2, T3, T4, T5, T6, T7>() {
foo3<T, U, V, W, X, Y, Z, T7>() {
throw new Error("Method not implemented.");
}
foo2<T, U, V, W, X, Y, Z>() {

View File

@ -0,0 +1,22 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T>(value: T) {
//// const keyofTypeof = Object.keys(value)[0] as keyof T;
//// added/*1*/(keyofTypeof);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T>(value: T) {
const keyofTypeof = Object.keys(value)[0] as keyof T;
added(keyofTypeof);
}
function added(keyofTypeof: string | number | symbol) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T>(value: T) {
//// added/*1*/(value);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T>(value: T) {
added(value);
}
function added<T>(value: T) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T>(value: T) {
//// added/*1*/<T, string>(value, "");
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T>(value: T) {
added<T, string>(value, "");
}
function added<T, U>(value: T, arg1: string) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T>(value: T) {
//// added/*1*/<T>(value, value);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T>(value: T) {
added<T>(value, value);
}
function added<T>(value: T, value1: T) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T extends string>(value: T) {
//// added/*1*/(value);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T extends string>(value: T) {
added(value);
}
function added<T extends string>(value: T) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,24 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T>(value: T) {
//// if (typeof value === "number") {
//// added/*1*/(value);
//// }
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T>(value: T) {
if (typeof value === "number") {
added(value);
}
}
function added<T>(value: T) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,43 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function e<T extends "phone" | "home">() {
//// let et: T = 'phone'
//// added1/*1*/(et)
//// et = 'home'
//// added2/*2*/(et)
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added1'",
index: 0,
newFileContent: `function e<T extends "phone" | "home">() {
let et: T = 'phone'
added1(et)
et = 'home'
added2(et)
}
function added1(et: string) {
throw new Error("Function not implemented.")
}
`,
});
goTo.marker("2");
verify.codeFix({
description: "Add missing function declaration 'added1'",
index: 0,
newFileContent: `function e<T extends "phone" | "home">() {
let et: T = 'phone'
added1(et)
et = 'home'
added2(et)
}
function added1(et: string) {
throw new Error("Function not implemented.")
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T1, T, T2>(t1: T1, t: T, t2a: T2, t2b: T2, t2c: T2) {
//// added/*1*/(t2a, t2b, t2c, t1, t, t2a, t2c, t2b);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T1, T, T2>(t1: T1, t: T, t2a: T2, t2b: T2, t2c: T2) {
added(t2a, t2b, t2c, t1, t, t2a, t2c, t2b);
}
function added<T2, T1, T>(t2a: T2, t2b: T2, t2c: T2, t1: T1, t: T, t2a1: T2, t2c1: T2, t2b1: T2) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T extends string, U extends T>(value: T, other: U) {
//// added/*1*/(value, other);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T extends string, U extends T>(value: T, other: U) {
added(value, other);
}
function added<T extends string, U extends T>(value: T, other: U) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,24 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function outer<O>(o: O) {
//// return function inner<I>(i: I) {
//// added/*1*/(o, i);
//// }
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function outer<O>(o: O) {
return function inner<I>(i: I) {
added(o, i);
}
}
function added<O, I>(o: O, i: I) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,28 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function outer<O>(o: O) {
//// return function middle<M>(m: M) {
//// return function inner<I>(i: I) {
//// added/*1*/(o, m, i);
//// }
//// }
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function outer<O>(o: O) {
return function middle<M>(m: M) {
return function inner<I>(i: I) {
added(o, m, i);
}
}
}
function added<O, M, I>(o: O, m: M, i: I) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T, U>(value: T | U & string) {
//// added/*1*/(value);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T, U>(value: T | U & string) {
added(value);
}
function added<T>(value: T) {
throw new Error("Function not implemented.");
}
`,
});

View File

@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />
// @noImplicitAny: true
////function existing<T, U>(value1: T | U & string, value2: U & T, value3: U | T, value4: U) {
//// added/*1*/(value1, value2, value3, value4);
////}
goTo.marker("1");
verify.codeFix({
description: "Add missing function declaration 'added'",
index: 0,
newFileContent: `function existing<T, U>(value1: T | U & string, value2: U & T, value3: U | T, value4: U) {
added(value1, value2, value3, value4);
}
function added<T, U, V>(value1: T, value2: U, value3: V, value4: U) {
throw new Error("Function not implemented.");
}
`,
});