Skip to content

Commit 1b19af0

Browse files
Jesse TrinityDanielRosenwassersandersn
authored
Refactor params for interface (#42652)
* apply refactor to interface methods * handle calls * no available refactors for union type * add tests * Update src/services/refactors/convertParamsToDestructuredObject.ts Co-authored-by: Daniel Rosenwasser <[email protected]> * address comments * Update src/services/refactors/convertParamsToDestructuredObject.ts Co-authored-by: Daniel Rosenwasser <[email protected]> * Update src/services/refactors/convertParamsToDestructuredObject.ts Co-authored-by: Nathan Shively-Sanders <[email protected]> Co-authored-by: Daniel Rosenwasser <[email protected]> Co-authored-by: Nathan Shively-Sanders <[email protected]>
1 parent 664ed17 commit 1b19af0

8 files changed

+214
-14
lines changed

‎src/services/refactors/convertParamsToDestructuredObject.ts

Lines changed: 81 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,14 @@ namespace ts.refactor.convertParamsToDestructuredObject {
5151
changes: textChanges.ChangeTracker,
5252
functionDeclaration: ValidFunctionDeclaration,
5353
groupedReferences: GroupedReferences): void {
54-
const newParamDeclaration = map(createNewParameters(functionDeclaration, program, host), param => getSynthesizedDeepClone(param));
55-
changes.replaceNodeRangeWithNodes(
56-
sourceFile,
57-
first(functionDeclaration.parameters),
58-
last(functionDeclaration.parameters),
59-
newParamDeclaration,
60-
{ joiner: ", ",
61-
// indentation is set to 0 because otherwise the object parameter will be indented if there is a `this` parameter
62-
indentation: 0,
63-
leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll,
64-
trailingTriviaOption: textChanges.TrailingTriviaOption.Include
65-
});
54+
const signature = groupedReferences.signature;
55+
const newFunctionDeclarationParams = map(createNewParameters(functionDeclaration, program, host), param => getSynthesizedDeepClone(param));
56+
57+
if (signature) {
58+
const newSignatureParams = map(createNewParameters(signature, program, host), param => getSynthesizedDeepClone(param));
59+
replaceParameters(signature, newSignatureParams);
60+
}
61+
replaceParameters(functionDeclaration, newFunctionDeclarationParams);
6662

6763
const functionCalls = sortAndDeduplicate(groupedReferences.functionCalls, /*comparer*/ (a, b) => compareValues(a.pos, b.pos));
6864
for (const call of functionCalls) {
@@ -76,6 +72,21 @@ namespace ts.refactor.convertParamsToDestructuredObject {
7672
{ leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll, trailingTriviaOption: textChanges.TrailingTriviaOption.Include });
7773
}
7874
}
75+
76+
function replaceParameters(declarationOrSignature: ValidFunctionDeclaration | ValidMethodSignature, parameterDeclarations: ParameterDeclaration[]) {
77+
changes.replaceNodeRangeWithNodes(
78+
sourceFile,
79+
first(declarationOrSignature.parameters),
80+
last(declarationOrSignature.parameters),
81+
parameterDeclarations,
82+
{
83+
joiner: ", ",
84+
// indentation is set to 0 because otherwise the object parameter will be indented if there is a `this` parameter
85+
indentation: 0,
86+
leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll,
87+
trailingTriviaOption: textChanges.TrailingTriviaOption.Include
88+
});
89+
}
7990
}
8091

8192
function getGroupedReferences(functionDeclaration: ValidFunctionDeclaration, program: Program, cancellationToken: CancellationToken): GroupedReferences {
@@ -99,13 +110,41 @@ namespace ts.refactor.convertParamsToDestructuredObject {
99110
const functionSymbols = map(functionNames, getSymbolTargetAtLocation);
100111
const classSymbols = map(classNames, getSymbolTargetAtLocation);
101112
const isConstructor = isConstructorDeclaration(functionDeclaration);
113+
const contextualSymbols = map(functionNames, name => getSymbolForContextualType(name, checker));
102114

103115
for (const entry of referenceEntries) {
104-
if (entry.kind !== FindAllReferences.EntryKind.Node) {
116+
if (entry.kind === FindAllReferences.EntryKind.Span) {
105117
groupedReferences.valid = false;
106118
continue;
107119
}
108120

121+
/* Declarations in object literals may be implementations of method signatures which have a different symbol from the declaration
122+
For example:
123+
interface IFoo { m(a: number): void }
124+
const foo: IFoo = { m(a: number): void {} }
125+
In these cases we get the symbol for the signature from the contextual type.
126+
*/
127+
if (contains(contextualSymbols, getSymbolTargetAtLocation(entry.node))) {
128+
if (isValidMethodSignature(entry.node.parent)) {
129+
groupedReferences.signature = entry.node.parent;
130+
continue;
131+
}
132+
const call = entryToFunctionCall(entry);
133+
if (call) {
134+
groupedReferences.functionCalls.push(call);
135+
continue;
136+
}
137+
}
138+
139+
const contextualSymbol = getSymbolForContextualType(entry.node, checker);
140+
if (contextualSymbol && contains(contextualSymbols, contextualSymbol)) {
141+
const decl = entryToDeclaration(entry);
142+
if (decl) {
143+
groupedReferences.declarations.push(decl);
144+
continue;
145+
}
146+
}
147+
109148
/* We compare symbols because in some cases find all references wil return a reference that may or may not be to the refactored function.
110149
Example from the refactorConvertParamsToDestructuredObject_methodCallUnion.ts test:
111150
class A { foo(a: number, b: number) { return a + b; } }
@@ -175,6 +214,20 @@ namespace ts.refactor.convertParamsToDestructuredObject {
175214
}
176215
}
177216

217+
/**
218+
* Gets the symbol for the contextual type of the node if it is not a union or intersection.
219+
*/
220+
function getSymbolForContextualType(node: Node, checker: TypeChecker): Symbol | undefined {
221+
const element = getContainingObjectLiteralElement(node);
222+
if (element) {
223+
const contextualType = checker.getContextualTypeForObjectLiteralElement(<ObjectLiteralElementLike>element);
224+
const symbol = contextualType?.getSymbol();
225+
if (symbol && !(getCheckFlags(symbol) & CheckFlags.Synthetic)) {
226+
return symbol;
227+
}
228+
}
229+
}
230+
178231
function entryToImportOrExport(entry: FindAllReferences.NodeEntry): Node | undefined {
179232
const node = entry.node;
180233

@@ -292,6 +345,10 @@ namespace ts.refactor.convertParamsToDestructuredObject {
292345
return false;
293346
}
294347

348+
function isValidMethodSignature(node: Node): node is ValidMethodSignature {
349+
return isMethodSignature(node) && (isInterfaceDeclaration(node.parent) || isTypeLiteralNode(node.parent));
350+
}
351+
295352
function isValidFunctionDeclaration(
296353
functionDeclaration: FunctionLikeDeclaration,
297354
checker: TypeChecker): functionDeclaration is ValidFunctionDeclaration {
@@ -300,6 +357,11 @@ namespace ts.refactor.convertParamsToDestructuredObject {
300357
case SyntaxKind.FunctionDeclaration:
301358
return hasNameOrDefault(functionDeclaration) && isSingleImplementation(functionDeclaration, checker);
302359
case SyntaxKind.MethodDeclaration:
360+
if (isObjectLiteralExpression(functionDeclaration.parent)) {
361+
const contextualSymbol = getSymbolForContextualType(functionDeclaration.name, checker);
362+
// don't offer the refactor when there are multiple signatures since we won't know which ones the user wants to change
363+
return contextualSymbol?.declarations.length === 1 && isSingleImplementation(functionDeclaration, checker);
364+
}
303365
return isSingleImplementation(functionDeclaration, checker);
304366
case SyntaxKind.Constructor:
305367
if (isClassDeclaration(functionDeclaration.parent)) {
@@ -398,7 +460,7 @@ namespace ts.refactor.convertParamsToDestructuredObject {
398460
return objectLiteral;
399461
}
400462

401-
function createNewParameters(functionDeclaration: ValidFunctionDeclaration, program: Program, host: LanguageServiceHost): NodeArray<ParameterDeclaration> {
463+
function createNewParameters(functionDeclaration: ValidFunctionDeclaration | ValidMethodSignature, program: Program, host: LanguageServiceHost): NodeArray<ParameterDeclaration> {
402464
const checker = program.getTypeChecker();
403465
const refactorableParameters = getRefactorableParameters(functionDeclaration.parameters);
404466
const bindingElements = map(refactorableParameters, createBindingElementFromParameterDeclaration);
@@ -584,6 +646,10 @@ namespace ts.refactor.convertParamsToDestructuredObject {
584646
parameters: NodeArray<ValidParameterDeclaration>;
585647
}
586648

649+
interface ValidMethodSignature extends MethodSignature {
650+
parameters: NodeArray<ValidParameterDeclaration>;
651+
}
652+
587653
type ValidFunctionDeclaration = ValidConstructor | ValidFunction | ValidMethod | ValidArrowFunction | ValidFunctionExpression;
588654

589655
interface ValidParameterDeclaration extends ParameterDeclaration {
@@ -595,6 +661,7 @@ namespace ts.refactor.convertParamsToDestructuredObject {
595661
interface GroupedReferences {
596662
functionCalls: (CallExpression | NewExpression)[];
597663
declarations: Node[];
664+
signature?: ValidMethodSignature;
598665
classReferences?: ClassReferences;
599666
valid: boolean;
600667
}

‎tests/cases/fourslash/refactorConvertParamsToDestructuredObject_interface.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////interface IFoo {
4+
//// method(x: string, y: string): void;
5+
////}
6+
////const x: IFoo = {
7+
//// method(/*a*/x: string, y: string/*b*/): void {},
8+
////};
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Convert parameters to destructured object",
13+
actionName: "Convert parameters to destructured object",
14+
actionDescription: "Convert parameters to destructured object",
15+
newContent: `interface IFoo {
16+
method({ x, y }: { x: string; y: string; }): void;
17+
}
18+
const x: IFoo = {
19+
method({ x, y }: { x: string; y: string; }): void {},
20+
};`
21+
});

‎tests/cases/fourslash/refactorConvertParamsToDestructuredObject_interfaceAssignability.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////interface IFoo {
4+
//// method(x: string, y: string): void;
5+
////}
6+
////const x: IFoo = {
7+
//// method(/*a*/x: string, y: string, z?: string/*b*/): void {},
8+
////};
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Convert parameters to destructured object",
13+
actionName: "Convert parameters to destructured object",
14+
actionDescription: "Convert parameters to destructured object",
15+
newContent: `interface IFoo {
16+
method({ x, y }: { x: string; y: string; }): void;
17+
}
18+
const x: IFoo = {
19+
method({ x, y, z }: { x: string; y: string; z?: string; }): void {},
20+
};`
21+
});

‎tests/cases/fourslash/refactorConvertParamsToDestructuredObject_interfaceContextualParams.ts

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+
////interface IFoo {
4+
//// method(x: string, y: string): void;
5+
////}
6+
////const x: IFoo = {
7+
//// method(/*a*/x, y/*b*/): void {},
8+
////};
9+
10+
/*
11+
When there are no type annotations on the params in the implementation, we ultimately
12+
would like to handle them like we do for calls resulting in `method({x, y}) {}`.
13+
14+
Note that simply adding the annotations from the signature can fail as the implementation
15+
can take more paramters than the signatures.
16+
*/
17+
goTo.select("a", "b");
18+
edit.applyRefactor({
19+
refactorName: "Convert parameters to destructured object",
20+
actionName: "Convert parameters to destructured object",
21+
actionDescription: "Convert parameters to destructured object",
22+
newContent: `interface IFoo {
23+
method({ x, y }: { x: string; y: string; }): void;
24+
}
25+
const x: IFoo = {
26+
method({ x, y }: { x; y; }): void {},
27+
};`
28+
});

‎tests/cases/fourslash/refactorConvertParamsToDestructuredObject_interfaceMultipleSignatures.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////interface IFoo {
4+
//// method(x: string, y: string): void;
5+
//// method(x: number, y: string): void;
6+
////}
7+
////const x: IFoo = {
8+
//// method(/*a*/x: string | number, y: string/*b*/): void {},
9+
////};
10+
11+
// For multiple signatures, we don't have a reliable way to determine
12+
// which signature to match to or if all signatures should be changed.
13+
goTo.select("a", "b");
14+
verify.not.refactorAvailable("Convert parameters to destructured object");

‎tests/cases/fourslash/refactorConvertParamsToDestructuredObject_interfaceNoIntersection.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////interface IFoo {
4+
//// method(x: string, y: string): void;
5+
////}
6+
////interface IBar {
7+
//// method(x: number): void;
8+
////}
9+
////const x: IFoo & IBar = {
10+
//// method(/*a*/x: string, y: string/*b*/): void {},
11+
////};
12+
13+
goTo.select("a", "b");
14+
verify.not.refactorAvailable("Convert parameters to destructured object");

‎tests/cases/fourslash/refactorConvertParamsToDestructuredObject_interfaceNoUnion.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////interface IFoo {
4+
//// method(x: string, y: string): void;
5+
////}
6+
////interface IBar {
7+
//// method(x: number): void;
8+
////}
9+
////const x: IFoo | IBar = {
10+
//// method(/*a*/x: string, y: string/*b*/): void {},
11+
////};
12+
13+
goTo.select("a", "b");
14+
verify.not.refactorAvailable("Convert parameters to destructured object");

‎tests/cases/fourslash/refactorConvertParamsToDestructuredObject_typeLiteral.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////type Foo = {
4+
//// method(x: string, y: string): void;
5+
////}
6+
////const x: Foo = {
7+
//// method(/*a*/x: string, y: string/*b*/): void {},
8+
////};
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Convert parameters to destructured object",
13+
actionName: "Convert parameters to destructured object",
14+
actionDescription: "Convert parameters to destructured object",
15+
newContent: `type Foo = {
16+
method({ x, y }: { x: string; y: string; }): void;
17+
}
18+
const x: Foo = {
19+
method({ x, y }: { x: string; y: string; }): void {},
20+
};`
21+
});

0 commit comments

Comments
 (0)