Skip to content

Commit c295531

Browse files
navya9singhnavya9singh
authored
Adopt code action ranges for refactorings (#57608)
1 parent a46664a commit c295531

13 files changed

+846
-52
lines changed

‎src/server/session.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2750,7 +2750,8 @@ export class Session<TMessage = string> implements EventSender {
27502750
private getApplicableRefactors(args: protocol.GetApplicableRefactorsRequestArgs): protocol.ApplicableRefactorInfo[] {
27512751
const { file, project } = this.getFileAndProject(args);
27522752
const scriptInfo = project.getScriptInfoForNormalizedPath(file)!;
2753-
return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason, args.kind, args.includeInteractiveActions);
2753+
const result = project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason, args.kind, args.includeInteractiveActions);
2754+
return result.map(result => ({ ...result, actions: result.actions.map(action => ({ ...action, range: action.range ? { start: convertToLocation({ line: action.range.start.line, character: action.range.start.offset }), end: convertToLocation({ line: action.range.end.line, character: action.range.end.offset }) } : undefined })) }));
27542755
}
27552756

27562757
private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): RefactorEditInfo | protocol.RefactorEditInfo {

‎src/services/refactors/extractSymbol.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
getEffectiveTypeParameterDeclarations,
4848
getEmitScriptTarget,
4949
getEnclosingBlockScopeContainer,
50+
getLineAndCharacterOfPosition,
5051
getLocaleSpecificMessage,
5152
getModifiers,
5253
getNodeId,
@@ -219,7 +220,7 @@ export function getRefactorActionsToExtractSymbol(context: RefactorContext): rea
219220
return errors;
220221
}
221222

222-
const extractions = getPossibleExtractions(targetRange, context);
223+
const { affectedTextRange, extractions } = getPossibleExtractions(targetRange, context);
223224
if (extractions === undefined) {
224225
// No extractions possible
225226
return emptyArray;
@@ -247,6 +248,10 @@ export function getRefactorActionsToExtractSymbol(context: RefactorContext): rea
247248
description,
248249
name: `function_scope_${i}`,
249250
kind: extractFunctionAction.kind,
251+
range: {
252+
start: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).character },
253+
end: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).character },
254+
},
250255
});
251256
}
252257
}
@@ -272,6 +277,10 @@ export function getRefactorActionsToExtractSymbol(context: RefactorContext): rea
272277
description,
273278
name: `constant_scope_${i}`,
274279
kind: extractConstantAction.kind,
280+
range: {
281+
start: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).character },
282+
end: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).character },
283+
},
275284
});
276285
}
277286
}
@@ -909,8 +918,8 @@ interface ScopeExtractions {
909918
* Each returned ExtractResultForScope corresponds to a possible target scope and is either a set of changes
910919
* or an error explaining why we can't extract into that scope.
911920
*/
912-
function getPossibleExtractions(targetRange: TargetRange, context: RefactorContext): readonly ScopeExtractions[] | undefined {
913-
const { scopes, readsAndWrites: { functionErrorsPerScope, constantErrorsPerScope } } = getPossibleExtractionsWorker(targetRange, context);
921+
function getPossibleExtractions(targetRange: TargetRange, context: RefactorContext): { readonly affectedTextRange: TextRange; readonly extractions: ScopeExtractions[] | undefined; } {
922+
const { scopes, affectedTextRange, readsAndWrites: { functionErrorsPerScope, constantErrorsPerScope } } = getPossibleExtractionsWorker(targetRange, context);
914923
// Need the inner type annotation to avoid https://github.com/Microsoft/TypeScript/issues/7547
915924
const extractions = scopes.map((scope, i): ScopeExtractions => {
916925
const functionDescriptionPart = getDescriptionForFunctionInScope(scope);
@@ -953,10 +962,10 @@ function getPossibleExtractions(targetRange: TargetRange, context: RefactorConte
953962
},
954963
};
955964
});
956-
return extractions;
965+
return { affectedTextRange, extractions };
957966
}
958967

959-
function getPossibleExtractionsWorker(targetRange: TargetRange, context: RefactorContext): { readonly scopes: Scope[]; readonly readsAndWrites: ReadsAndWrites; } {
968+
function getPossibleExtractionsWorker(targetRange: TargetRange, context: RefactorContext): { readonly scopes: Scope[]; readonly affectedTextRange: TextRange; readonly readsAndWrites: ReadsAndWrites; } {
960969
const { file: sourceFile } = context;
961970

962971
const scopes = collectEnclosingScopes(targetRange);
@@ -969,7 +978,7 @@ function getPossibleExtractionsWorker(targetRange: TargetRange, context: Refacto
969978
context.program.getTypeChecker(),
970979
context.cancellationToken!,
971980
);
972-
return { scopes, readsAndWrites };
981+
return { scopes, affectedTextRange: enclosingTextRange, readsAndWrites };
973982
}
974983

975984
function getDescriptionForFunctionInScope(scope: Scope): string {

‎src/services/refactors/extractType.ts

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -99,16 +99,27 @@ registerRefactor(refactorName, {
9999
extractToTypeDefAction.kind,
100100
],
101101
getAvailableActions: function getRefactorActionsToExtractType(context): readonly ApplicableRefactorInfo[] {
102-
const info = getRangeToExtract(context, context.triggerReason === "invoked");
102+
const { info, affectedTextRange } = getRangeToExtract(context, context.triggerReason === "invoked");
103103
if (!info) return emptyArray;
104104

105105
if (!isRefactorErrorInfo(info)) {
106-
return [{
106+
const refactorInfo: ApplicableRefactorInfo[] = [{
107107
name: refactorName,
108108
description: getLocaleSpecificMessage(Diagnostics.Extract_type),
109109
actions: info.isJS ?
110110
[extractToTypeDefAction] : append([extractToTypeAliasAction], info.typeElements && extractToInterfaceAction),
111111
}];
112+
return refactorInfo.map(info => ({
113+
...info,
114+
actions: info.actions.map(action => ({
115+
...action,
116+
range: affectedTextRange ? {
117+
start: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).character },
118+
end: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).character },
119+
}
120+
: undefined,
121+
})),
122+
}));
112123
}
113124

114125
if (context.preferences.provideRefactorNotApplicableReason) {
@@ -127,7 +138,7 @@ registerRefactor(refactorName, {
127138
},
128139
getEditsForAction: function getRefactorEditsToExtractType(context, actionName): RefactorEditInfo {
129140
const { file } = context;
130-
const info = getRangeToExtract(context);
141+
const { info } = getRangeToExtract(context);
131142
Debug.assert(info && !isRefactorErrorInfo(info), "Expected to find a range to extract");
132143

133144
const name = getUniqueName("NewType", file);
@@ -171,20 +182,20 @@ interface InterfaceInfo {
171182

172183
type ExtractInfo = TypeAliasInfo | InterfaceInfo;
173184

174-
function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): ExtractInfo | RefactorErrorInfo | undefined {
185+
function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): { info: ExtractInfo | RefactorErrorInfo | undefined; affectedTextRange?: TextRange; } {
175186
const { file, startPosition } = context;
176187
const isJS = isSourceFileJS(file);
177188
const range = createTextRangeFromSpan(getRefactorContextSpan(context));
178189
const isCursorRequest = range.pos === range.end && considerEmptySpans;
179190
const firstType = getFirstTypeAt(file, startPosition, range, isCursorRequest);
180-
if (!firstType || !isTypeNode(firstType)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) };
191+
if (!firstType || !isTypeNode(firstType)) return { info: { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) }, affectedTextRange: undefined };
181192

182193
const checker = context.program.getTypeChecker();
183194
const enclosingNode = getEnclosingNode(firstType, isJS);
184-
if (enclosingNode === undefined) return { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) };
195+
if (enclosingNode === undefined) return { info: { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) }, affectedTextRange: undefined };
185196

186197
const expandedFirstType = getExpandedSelectionNode(firstType, enclosingNode);
187-
if (!isTypeNode(expandedFirstType)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) };
198+
if (!isTypeNode(expandedFirstType)) return { info: { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) }, affectedTextRange: undefined };
188199

189200
const typeList: TypeNode[] = [];
190201
if ((isUnionTypeNode(expandedFirstType.parent) || isIntersectionTypeNode(expandedFirstType.parent)) && range.end > firstType.end) {
@@ -198,11 +209,11 @@ function getRangeToExtract(context: RefactorContext, considerEmptySpans = true):
198209
}
199210
const selection = typeList.length > 1 ? typeList : expandedFirstType;
200211

201-
const typeParameters = collectTypeParameters(checker, selection, enclosingNode, file);
202-
if (!typeParameters) return { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) };
212+
const { typeParameters, affectedTextRange } = collectTypeParameters(checker, selection, enclosingNode, file);
213+
if (!typeParameters) return { info: { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) }, affectedTextRange: undefined };
203214

204215
const typeElements = flattenTypeLiteralNodeReference(checker, selection);
205-
return { isJS, selection, enclosingNode, typeParameters, typeElements };
216+
return { info: { isJS, selection, enclosingNode, typeParameters, typeElements }, affectedTextRange };
206217
}
207218

208219
function getFirstTypeAt(file: SourceFile, startPosition: number, range: TextRange, isCursorRequest: boolean): Node | undefined {
@@ -260,14 +271,14 @@ function rangeContainsSkipTrivia(r1: TextRange, node: TextRange, file: SourceFil
260271
return rangeContainsStartEnd(r1, skipTrivia(file.text, node.pos), node.end);
261272
}
262273

263-
function collectTypeParameters(checker: TypeChecker, selection: TypeNode | TypeNode[], enclosingNode: Node, file: SourceFile): TypeParameterDeclaration[] | undefined {
274+
function collectTypeParameters(checker: TypeChecker, selection: TypeNode | TypeNode[], enclosingNode: Node, file: SourceFile): { typeParameters: TypeParameterDeclaration[] | undefined; affectedTextRange: TextRange | undefined; } {
264275
const result: TypeParameterDeclaration[] = [];
265276
const selectionArray = toArray(selection);
266-
const selectionRange = { pos: selectionArray[0].pos, end: selectionArray[selectionArray.length - 1].end };
277+
const selectionRange = { pos: selectionArray[0].getStart(file), end: selectionArray[selectionArray.length - 1].end };
267278
for (const t of selectionArray) {
268-
if (visitor(t)) return undefined;
279+
if (visitor(t)) return { typeParameters: undefined, affectedTextRange: undefined };
269280
}
270-
return result;
281+
return { typeParameters: result, affectedTextRange: selectionRange };
271282

272283
function visitor(node: Node): true | undefined {
273284
if (isTypeReferenceNode(node)) {

‎src/services/refactors/moveToFile.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
GetCanonicalFileName,
5151
getDecorators,
5252
getDirectoryPath,
53+
getLineAndCharacterOfPosition,
5354
getLocaleSpecificMessage,
5455
getModifiers,
5556
getPropertySymbolFromBindingElement,
@@ -164,22 +165,26 @@ const moveToFileAction = {
164165
registerRefactor(refactorNameForMoveToFile, {
165166
kinds: [moveToFileAction.kind],
166167
getAvailableActions: function getRefactorActionsToMoveToFile(context, interactiveRefactorArguments): readonly ApplicableRefactorInfo[] {
168+
const file = context.file;
167169
const statements = getStatementsToMove(context);
168170
if (!interactiveRefactorArguments) {
169171
return emptyArray;
170172
}
171173
/** If the start/end nodes of the selection are inside a block like node do not show the `Move to file` code action
172174
* This condition is used in order to show less often the `Move to file` code action */
173175
if (context.endPosition !== undefined) {
174-
const file = context.file;
175176
const startNodeAncestor = findAncestor(getTokenAtPosition(file, context.startPosition), isBlockLike);
176177
const endNodeAncestor = findAncestor(getTokenAtPosition(file, context.endPosition), isBlockLike);
177178
if (startNodeAncestor && !isSourceFile(startNodeAncestor) && endNodeAncestor && !isSourceFile(endNodeAncestor)) {
178179
return emptyArray;
179180
}
180181
}
181182
if (context.preferences.allowTextChangesInNewFiles && statements) {
182-
return [{ name: refactorNameForMoveToFile, description, actions: [moveToFileAction] }];
183+
const affectedTextRange = {
184+
start: { line: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).line, offset: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).character },
185+
end: { line: getLineAndCharacterOfPosition(file, last(statements.all).end).line, offset: getLineAndCharacterOfPosition(file, last(statements.all).end).character },
186+
};
187+
return [{ name: refactorNameForMoveToFile, description, actions: [{ ...moveToFileAction, range: affectedTextRange }] }];
183188
}
184189
if (context.preferences.provideRefactorNotApplicableReason) {
185190
return [{ name: refactorNameForMoveToFile, description, actions: [{ ...moveToFileAction, notApplicableReason: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_statement_or_statements) }] }];

‎src/services/refactors/moveToNewFile.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
emptyArray,
77
fileShouldUseJavaScriptRequire,
88
getBaseFileName,
9+
getLineAndCharacterOfPosition,
910
getLocaleSpecificMessage,
1011
getQuotePreference,
1112
hasSyntacticModifier,
@@ -14,6 +15,7 @@ import {
1415
insertImports,
1516
isPrologueDirective,
1617
LanguageServiceHost,
18+
last,
1719
ModifierFlags,
1820
nodeSeenTracker,
1921
Program,
@@ -64,7 +66,12 @@ registerRefactor(refactorName, {
6466
getAvailableActions: function getRefactorActionsToMoveToNewFile(context): readonly ApplicableRefactorInfo[] {
6567
const statements = getStatementsToMove(context);
6668
if (context.preferences.allowTextChangesInNewFiles && statements) {
67-
return [{ name: refactorName, description, actions: [moveToNewFileAction] }];
69+
const file = context.file;
70+
const affectedTextRange = {
71+
start: { line: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).line, offset: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).character },
72+
end: { line: getLineAndCharacterOfPosition(file, last(statements.all).end).line, offset: getLineAndCharacterOfPosition(file, last(statements.all).end).character },
73+
};
74+
return [{ name: refactorName, description, actions: [{ ...moveToNewFileAction, range: affectedTextRange }] }];
6875
}
6976
if (context.preferences.provideRefactorNotApplicableReason) {
7077
return [{ name: refactorName, description, actions: [{ ...moveToNewFileAction, notApplicableReason: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_statement_or_statements) }] }];

‎src/services/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,11 @@ export interface RefactorActionInfo {
10081008
* when calling `getEditsForRefactor`.
10091009
*/
10101010
isInteractive?: boolean;
1011+
1012+
/**
1013+
* Range of code the refactoring will be applied to.
1014+
*/
1015+
range?: { start: { line: number; offset: number; }; end: { line: number; offset: number; }; };
10111016
}
10121017

10131018
/**

‎src/testRunner/unittests/tsserver/getApplicableRefactors.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,69 @@ describe("unittests:: tsserver:: getApplicableRefactors", () => {
2121
});
2222
baselineTsserverLogs("getApplicableRefactors", "works when taking position", session);
2323
});
24+
25+
it("returns the affected range of text for extract symbol refactor", () => {
26+
const file1: File = {
27+
path: "/a.ts",
28+
content: `class Foo {
29+
someMethod(m: number) {
30+
var x = m;
31+
x = x * 3;
32+
var y = 30;
33+
var j = 10;
34+
var z = y + j;
35+
console.log(z);
36+
var q = 10;
37+
return q;
38+
}
39+
}`,
40+
};
41+
const host = createServerHost([file1]);
42+
const session = new TestSession(host);
43+
openFilesForSession([file1], session);
44+
session.executeCommandSeq<ts.server.protocol.GetApplicableRefactorsRequest>({
45+
command: ts.server.protocol.CommandTypes.GetApplicableRefactors,
46+
arguments: { file: file1.path, startLine: 3, startOffset: 9, endLine: 5, endOffset: 20 },
47+
});
48+
baselineTsserverLogs("getApplicableRefactors", "returns the affected range of text for extract symbol refactor", session);
49+
});
50+
51+
it("returns the affected range of text for extract type refactor", () => {
52+
const file1: File = {
53+
path: "/a.ts",
54+
content: `type A<B, C, D = B> = Partial<C | string | D> & D | C;`,
55+
};
56+
const host = createServerHost([file1]);
57+
const session = new TestSession(host);
58+
openFilesForSession([file1], session);
59+
session.executeCommandSeq<ts.server.protocol.GetApplicableRefactorsRequest>({
60+
command: ts.server.protocol.CommandTypes.GetApplicableRefactors,
61+
arguments: { file: file1.path, startLine: 1, startOffset: 26, endLine: 1, endOffset: 38 },
62+
});
63+
baselineTsserverLogs("getApplicableRefactors", "returns the affected range of text for extract type refactor", session);
64+
});
65+
66+
it("returns the affected range of text for 'move to file' and 'move to new file' refactors", () => {
67+
const file1: File = {
68+
path: "/a.ts",
69+
content: `const a = 1;
70+
const b = 1;
71+
function foo() { }`,
72+
};
73+
const host = createServerHost([file1]);
74+
const session = new TestSession(host);
75+
openFilesForSession([file1], session);
76+
77+
session.executeCommandSeq<ts.server.protocol.ConfigureRequest>({
78+
command: ts.server.protocol.CommandTypes.Configure,
79+
arguments: {
80+
preferences: { allowTextChangesInNewFiles: true },
81+
},
82+
});
83+
session.executeCommandSeq<ts.server.protocol.GetApplicableRefactorsRequest>({
84+
command: ts.server.protocol.CommandTypes.GetApplicableRefactors,
85+
arguments: { file: file1.path, startLine: 1, startOffset: 3, endLine: 2, endOffset: 3, includeInteractiveActions: true },
86+
});
87+
baselineTsserverLogs("getApplicableRefactors", "returns the affected range of text for 'move to file' and 'move to new file' refactors", session);
88+
});
2489
});

0 commit comments

Comments
 (0)