Adopt code action ranges for refactorings

src/server/session.ts

@@ -2750,7 +2750,8 @@ export class Session<TMessage = string> implements EventSender {
     private getApplicableRefactors(args: protocol.GetApplicableRefactorsRequestArgs): protocol.ApplicableRefactorInfo[] {
         const { file, project } = this.getFileAndProject(args);
         const scriptInfo = project.getScriptInfoForNormalizedPath(file)!;
-        return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason, args.kind, args.includeInteractiveActions);
+        const result = project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason, args.kind, args.includeInteractiveActions);
+        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 })) }));
     }
 
     private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): RefactorEditInfo | protocol.RefactorEditInfo {

src/services/refactors/extractSymbol.ts

@@ -47,6 +47,7 @@ import {
     getEffectiveTypeParameterDeclarations,
     getEmitScriptTarget,
     getEnclosingBlockScopeContainer,
+    getLineAndCharacterOfPosition,
     getLocaleSpecificMessage,
     getModifiers,
     getNodeId,
@@ -219,7 +220,7 @@ export function getRefactorActionsToExtractSymbol(context: RefactorContext): rea
         return errors;
     }
 
-    const extractions = getPossibleExtractions(targetRange, context);
+    const { affectedTextRange, extractions } = getPossibleExtractions(targetRange, context);
     if (extractions === undefined) {
         // No extractions possible
         return emptyArray;
@@ -247,6 +248,10 @@ export function getRefactorActionsToExtractSymbol(context: RefactorContext): rea
                         description,
                         name: `function_scope_${i}`,
                         kind: extractFunctionAction.kind,
+                        range: {
+                            start: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).character },
+                            end: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).character },
+                        },
                     });
                 }
             }
@@ -272,6 +277,10 @@ export function getRefactorActionsToExtractSymbol(context: RefactorContext): rea
                         description,
                         name: `constant_scope_${i}`,
                         kind: extractConstantAction.kind,
+                        range: {
+                            start: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).character },
+                            end: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).character },
+                        },
                     });
                 }
             }
@@ -909,8 +918,8 @@ interface ScopeExtractions {
  * Each returned ExtractResultForScope corresponds to a possible target scope and is either a set of changes
  * or an error explaining why we can't extract into that scope.
  */
-function getPossibleExtractions(targetRange: TargetRange, context: RefactorContext): readonly ScopeExtractions[] | undefined {
-    const { scopes, readsAndWrites: { functionErrorsPerScope, constantErrorsPerScope } } = getPossibleExtractionsWorker(targetRange, context);
+function getPossibleExtractions(targetRange: TargetRange, context: RefactorContext): { readonly affectedTextRange: TextRange; readonly extractions: ScopeExtractions[] | undefined; } {
+    const { scopes, affectedTextRange, readsAndWrites: { functionErrorsPerScope, constantErrorsPerScope } } = getPossibleExtractionsWorker(targetRange, context);
     // Need the inner type annotation to avoid https://github.com/Microsoft/TypeScript/issues/7547
     const extractions = scopes.map((scope, i): ScopeExtractions => {
         const functionDescriptionPart = getDescriptionForFunctionInScope(scope);
@@ -953,10 +962,10 @@ function getPossibleExtractions(targetRange: TargetRange, context: RefactorConte
             },
         };
     });
-    return extractions;
+    return { affectedTextRange, extractions };
 }
 
-function getPossibleExtractionsWorker(targetRange: TargetRange, context: RefactorContext): { readonly scopes: Scope[]; readonly readsAndWrites: ReadsAndWrites; } {
+function getPossibleExtractionsWorker(targetRange: TargetRange, context: RefactorContext): { readonly scopes: Scope[]; readonly affectedTextRange: TextRange; readonly readsAndWrites: ReadsAndWrites; } {
     const { file: sourceFile } = context;
 
     const scopes = collectEnclosingScopes(targetRange);
@@ -969,7 +978,7 @@ function getPossibleExtractionsWorker(targetRange: TargetRange, context: Refacto
         context.program.getTypeChecker(),
         context.cancellationToken!,
     );
-    return { scopes, readsAndWrites };
+    return { scopes, affectedTextRange: enclosingTextRange, readsAndWrites };
 }
 
 function getDescriptionForFunctionInScope(scope: Scope): string {

src/services/refactors/extractType.ts

@@ -99,16 +99,27 @@ registerRefactor(refactorName, {
         extractToTypeDefAction.kind,
     ],
     getAvailableActions: function getRefactorActionsToExtractType(context): readonly ApplicableRefactorInfo[] {
-        const info = getRangeToExtract(context, context.triggerReason === "invoked");
+        const { info, affectedTextRange } = getRangeToExtract(context, context.triggerReason === "invoked");
         if (!info) return emptyArray;
 
         if (!isRefactorErrorInfo(info)) {
-            return [{
+            const refactorInfo: ApplicableRefactorInfo[] = [{
                 name: refactorName,
                 description: getLocaleSpecificMessage(Diagnostics.Extract_type),
                 actions: info.isJS ?
                     [extractToTypeDefAction] : append([extractToTypeAliasAction], info.typeElements && extractToInterfaceAction),
             }];
+            return refactorInfo.map(info => ({
+                ...info,
+                actions: info.actions.map(action => ({
+                    ...action,
+                    range: affectedTextRange ? {
+                        start: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.pos).character },
+                        end: { line: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).line, offset: getLineAndCharacterOfPosition(context.file, affectedTextRange.end).character },
+                    }
+                        : undefined,
+                })),
+            }));
         }
 
         if (context.preferences.provideRefactorNotApplicableReason) {
@@ -127,7 +138,7 @@ registerRefactor(refactorName, {
     },
     getEditsForAction: function getRefactorEditsToExtractType(context, actionName): RefactorEditInfo {
         const { file } = context;
-        const info = getRangeToExtract(context);
+        const { info } = getRangeToExtract(context);
         Debug.assert(info && !isRefactorErrorInfo(info), "Expected to find a range to extract");
 
         const name = getUniqueName("NewType", file);
@@ -171,20 +182,20 @@ interface InterfaceInfo {
 
 type ExtractInfo = TypeAliasInfo | InterfaceInfo;
 
-function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): ExtractInfo | RefactorErrorInfo | undefined {
+function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): { info: ExtractInfo | RefactorErrorInfo | undefined; affectedTextRange?: TextRange; } {
     const { file, startPosition } = context;
     const isJS = isSourceFileJS(file);
     const range = createTextRangeFromSpan(getRefactorContextSpan(context));
     const isCursorRequest = range.pos === range.end && considerEmptySpans;
     const firstType = getFirstTypeAt(file, startPosition, range, isCursorRequest);
-    if (!firstType || !isTypeNode(firstType)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) };
+    if (!firstType || !isTypeNode(firstType)) return { info: { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) }, affectedTextRange: undefined };
 
     const checker = context.program.getTypeChecker();
     const enclosingNode = getEnclosingNode(firstType, isJS);
-    if (enclosingNode === undefined) return { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) };
+    if (enclosingNode === undefined) return { info: { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) }, affectedTextRange: undefined };
 
     const expandedFirstType = getExpandedSelectionNode(firstType, enclosingNode);
-    if (!isTypeNode(expandedFirstType)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) };
+    if (!isTypeNode(expandedFirstType)) return { info: { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) }, affectedTextRange: undefined };
 
     const typeList: TypeNode[] = [];
     if ((isUnionTypeNode(expandedFirstType.parent) || isIntersectionTypeNode(expandedFirstType.parent)) && range.end > firstType.end) {
@@ -198,11 +209,11 @@ function getRangeToExtract(context: RefactorContext, considerEmptySpans = true):
     }
     const selection = typeList.length > 1 ? typeList : expandedFirstType;
 
-    const typeParameters = collectTypeParameters(checker, selection, enclosingNode, file);
-    if (!typeParameters) return { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) };
+    const { typeParameters, affectedTextRange } = collectTypeParameters(checker, selection, enclosingNode, file);
+    if (!typeParameters) return { info: { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) }, affectedTextRange: undefined };
 
     const typeElements = flattenTypeLiteralNodeReference(checker, selection);
-    return { isJS, selection, enclosingNode, typeParameters, typeElements };
+    return { info: { isJS, selection, enclosingNode, typeParameters, typeElements }, affectedTextRange };
 }
 
 function getFirstTypeAt(file: SourceFile, startPosition: number, range: TextRange, isCursorRequest: boolean): Node | undefined {
@@ -260,14 +271,14 @@ function rangeContainsSkipTrivia(r1: TextRange, node: TextRange, file: SourceFil
     return rangeContainsStartEnd(r1, skipTrivia(file.text, node.pos), node.end);
 }
 
-function collectTypeParameters(checker: TypeChecker, selection: TypeNode | TypeNode[], enclosingNode: Node, file: SourceFile): TypeParameterDeclaration[] | undefined {
+function collectTypeParameters(checker: TypeChecker, selection: TypeNode | TypeNode[], enclosingNode: Node, file: SourceFile): { typeParameters: TypeParameterDeclaration[] | undefined; affectedTextRange: TextRange | undefined; } {
     const result: TypeParameterDeclaration[] = [];
     const selectionArray = toArray(selection);
-    const selectionRange = { pos: selectionArray[0].pos, end: selectionArray[selectionArray.length - 1].end };
+    const selectionRange = { pos: selectionArray[0].getStart(file), end: selectionArray[selectionArray.length - 1].end };
     for (const t of selectionArray) {
-        if (visitor(t)) return undefined;
+        if (visitor(t)) return { typeParameters: undefined, affectedTextRange: undefined };
     }
-    return result;
+    return { typeParameters: result, affectedTextRange: selectionRange };
 
     function visitor(node: Node): true | undefined {
         if (isTypeReferenceNode(node)) {

src/services/refactors/moveToFile.ts

@@ -50,6 +50,7 @@ import {
     GetCanonicalFileName,
     getDecorators,
     getDirectoryPath,
+    getLineAndCharacterOfPosition,
     getLocaleSpecificMessage,
     getModifiers,
     getPropertySymbolFromBindingElement,
@@ -164,22 +165,26 @@ const moveToFileAction = {
 registerRefactor(refactorNameForMoveToFile, {
     kinds: [moveToFileAction.kind],
     getAvailableActions: function getRefactorActionsToMoveToFile(context, interactiveRefactorArguments): readonly ApplicableRefactorInfo[] {
+        const file = context.file;
         const statements = getStatementsToMove(context);
         if (!interactiveRefactorArguments) {
             return emptyArray;
         }
         /** If the start/end nodes of the selection are inside a block like node do not show the `Move to file` code action
          *  This condition is used in order to show less often the `Move to file` code action */
         if (context.endPosition !== undefined) {
-            const file = context.file;
             const startNodeAncestor = findAncestor(getTokenAtPosition(file, context.startPosition), isBlockLike);
             const endNodeAncestor = findAncestor(getTokenAtPosition(file, context.endPosition), isBlockLike);
             if (startNodeAncestor && !isSourceFile(startNodeAncestor) && endNodeAncestor && !isSourceFile(endNodeAncestor)) {
                 return emptyArray;
             }
         }
         if (context.preferences.allowTextChangesInNewFiles && statements) {
-            return [{ name: refactorNameForMoveToFile, description, actions: [moveToFileAction] }];
+            const affectedTextRange = {
+                start: { line: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).line, offset: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).character },
+                end: { line: getLineAndCharacterOfPosition(file, last(statements.all).end).line, offset: getLineAndCharacterOfPosition(file, last(statements.all).end).character },
+            };
+            return [{ name: refactorNameForMoveToFile, description, actions: [{ ...moveToFileAction, range: affectedTextRange }] }];
         }
         if (context.preferences.provideRefactorNotApplicableReason) {
             return [{ name: refactorNameForMoveToFile, description, actions: [{ ...moveToFileAction, notApplicableReason: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_statement_or_statements) }] }];

src/services/refactors/moveToNewFile.ts

@@ -6,6 +6,7 @@ import {
     emptyArray,
     fileShouldUseJavaScriptRequire,
     getBaseFileName,
+    getLineAndCharacterOfPosition,
     getLocaleSpecificMessage,
     getQuotePreference,
     hasSyntacticModifier,
@@ -14,6 +15,7 @@ import {
     insertImports,
     isPrologueDirective,
     LanguageServiceHost,
+    last,
     ModifierFlags,
     nodeSeenTracker,
     Program,
@@ -64,7 +66,12 @@ registerRefactor(refactorName, {
     getAvailableActions: function getRefactorActionsToMoveToNewFile(context): readonly ApplicableRefactorInfo[] {
         const statements = getStatementsToMove(context);
         if (context.preferences.allowTextChangesInNewFiles && statements) {
-            return [{ name: refactorName, description, actions: [moveToNewFileAction] }];
+            const file = context.file;
+            const affectedTextRange = {
+                start: { line: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).line, offset: getLineAndCharacterOfPosition(file, statements.all[0].getStart(file)).character },
+                end: { line: getLineAndCharacterOfPosition(file, last(statements.all).end).line, offset: getLineAndCharacterOfPosition(file, last(statements.all).end).character },
+            };
+            return [{ name: refactorName, description, actions: [{ ...moveToNewFileAction, range: affectedTextRange }] }];
         }
         if (context.preferences.provideRefactorNotApplicableReason) {
             return [{ name: refactorName, description, actions: [{ ...moveToNewFileAction, notApplicableReason: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_statement_or_statements) }] }];

src/services/types.ts

@@ -1008,6 +1008,11 @@ export interface RefactorActionInfo {
      * when calling `getEditsForRefactor`.
      */
     isInteractive?: boolean;
+
+    /**
+     * Range of code the refactoring will be applied to.
+     */
+    range?: { start: { line: number; offset: number; }; end: { line: number; offset: number; }; };
 }
 
 /**

src/testRunner/unittests/tsserver/getApplicableRefactors.ts

@@ -21,4 +21,69 @@ describe("unittests:: tsserver:: getApplicableRefactors", () => {
         });
         baselineTsserverLogs("getApplicableRefactors", "works when taking position", session);
     });
+
+    it("returns the affected range of text for extract symbol refactor", () => {
+        const file1: File = {
+            path: "/a.ts",
+            content: `class Foo {
+    someMethod(m: number) {
+        var x = m;
+        x = x * 3;
+        var y = 30;
+        var j = 10;
+        var z = y + j;
+        console.log(z);
+        var q = 10;
+        return q;
+    }
+}`,
+        };
+        const host = createServerHost([file1]);
+        const session = new TestSession(host);
+        openFilesForSession([file1], session);
+        session.executeCommandSeq<ts.server.protocol.GetApplicableRefactorsRequest>({
+            command: ts.server.protocol.CommandTypes.GetApplicableRefactors,
+            arguments: { file: file1.path, startLine: 3, startOffset: 9, endLine: 5, endOffset: 20 },
+        });
+        baselineTsserverLogs("getApplicableRefactors", "returns the affected range of text for extract symbol refactor", session);
+    });
+
+    it("returns the affected range of text for extract type refactor", () => {
+        const file1: File = {
+            path: "/a.ts",
+            content: `type A<B, C, D = B> = Partial<C | string | D> & D | C;`,
+        };
+        const host = createServerHost([file1]);
+        const session = new TestSession(host);
+        openFilesForSession([file1], session);
+        session.executeCommandSeq<ts.server.protocol.GetApplicableRefactorsRequest>({
+            command: ts.server.protocol.CommandTypes.GetApplicableRefactors,
+            arguments: { file: file1.path, startLine: 1, startOffset: 26, endLine: 1, endOffset: 38 },
+        });
+        baselineTsserverLogs("getApplicableRefactors", "returns the affected range of text for extract type refactor", session);
+    });
+
+    it("returns the affected range of text for 'move to file' and 'move to new file' refactors", () => {
+        const file1: File = {
+            path: "/a.ts",
+            content: `const a = 1;
+const b = 1;
+function foo() { }`,
+        };
+        const host = createServerHost([file1]);
+        const session = new TestSession(host);
+        openFilesForSession([file1], session);
+
+        session.executeCommandSeq<ts.server.protocol.ConfigureRequest>({
+            command: ts.server.protocol.CommandTypes.Configure,
+            arguments: {
+                preferences: { allowTextChangesInNewFiles: true },
+            },
+        });
+        session.executeCommandSeq<ts.server.protocol.GetApplicableRefactorsRequest>({
+            command: ts.server.protocol.CommandTypes.GetApplicableRefactors,
+            arguments: { file: file1.path, startLine: 1, startOffset: 3, endLine: 2, endOffset: 3, includeInteractiveActions: true },
+        });
+        baselineTsserverLogs("getApplicableRefactors", "returns the affected range of text for 'move to file' and 'move to new file' refactors", session);
+    });
 });