Merge pull request #1606 from DustinCampbell/fix-refactorings

Use correct span when requesting code actions from OmniSharp

Author: DustinCampbell

src/features/codeActionProvider.ts

@@ -5,14 +5,14 @@
 
 'use strict';
 
-import {CodeActionProvider, CodeActionContext, Command, CancellationToken, TextDocument, WorkspaceEdit, TextEdit, Range, Uri, workspace, commands} from 'vscode';
-import {OmniSharpServer} from '../omnisharp/server';
+import * as vscode from 'vscode';
+import { OmniSharpServer } from '../omnisharp/server';
 import AbstractProvider from './abstractProvider';
 import * as protocol from '../omnisharp/protocol';
-import {toRange2} from '../omnisharp/typeConvertion';
+import { toRange2 } from '../omnisharp/typeConvertion';
 import * as serverUtils from '../omnisharp/utils';
 
-export default class OmnisharpCodeActionProvider extends AbstractProvider implements CodeActionProvider {
+export default class CodeActionProvider extends AbstractProvider implements vscode.CodeActionProvider {
 
     private _disabled: boolean;
     private _commandId: string;
@@ -21,38 +21,80 @@ export default class OmnisharpCodeActionProvider extends AbstractProvider implem
         super(server);
         this._commandId = 'omnisharp.runCodeAction';
 
-        this._updateEnablement();
-        let d1 = workspace.onDidChangeConfiguration(this._updateEnablement, this);
-        let d2 = commands.registerCommand(this._commandId, this._runCodeAction, this);
+        this._checkOption();
+
+        let d1 = vscode.workspace.onDidChangeConfiguration(this._checkOption, this);
+        let d2 = vscode.commands.registerCommand(this._commandId, this._runCodeAction, this);
         this._disposables.push(d1, d2);
     }
 
-    private _updateEnablement(): void {
-        let value = workspace.getConfiguration().get('csharp.disableCodeActions', false);
+    private _checkOption(): void {
+        let value = vscode.workspace.getConfiguration().get('csharp.disableCodeActions', false);
         this._disabled = value;
     }
 
-    public provideCodeActions(document: TextDocument, range: Range, context: CodeActionContext, token: CancellationToken): Promise<Command[]> {
+    public provideCodeActions(document: vscode.TextDocument, range: vscode.Range, context: vscode.CodeActionContext, token: vscode.CancellationToken): Promise<vscode.Command[]> {
         if (this._disabled) {
             return;
         }
 
-        let req: protocol.V2.GetCodeActionsRequest = {
+        let line: number;
+        let column: number;
+        let selection: protocol.V2.Range;
+
+        // VS Code will pass the range of the word at the editor caret, even if there isn't a selection.
+        // To ensure that we don't suggest selection-based refactorings when there isn't a selection, we first
+        // find the text editor for this document and verify that there is a selection.
+        let editor = vscode.window.visibleTextEditors.find(e => e.document === document);
+        if (editor) {
+            if (editor.selection.isEmpty) {
+                // The editor does not have a selection. Use the active position of the selection (i.e. the caret).
+                let active = editor.selection.active;
+
+                line = active.line + 1;
+                column = active.character + 1;
+            }
+            else {
+                // The editor has a selection. Use it.
+                let start = editor.selection.start;
+                let end = editor.selection.end;
+
+                selection = {
+                    Start: { Line: start.line + 1, Column: start.character + 1 },
+                    End: { Line: end.line + 1, Column: end.character + 1 }
+                };
+            }
+        }
+        else {
+            // We couldn't find the editor, so just use the range we were provided.
+            selection = {
+                Start: { Line: range.start.line + 1, Column: range.start.character + 1 },
+                End: { Line: range.end.line + 1, Column: range.end.character + 1 }
+            };
+        }
+
+        let request: protocol.V2.GetCodeActionsRequest = {
             FileName: document.fileName,
-            Selection: OmnisharpCodeActionProvider._asRange(range)
+            Line: line,
+            Column: column,
+            Selection: selection
         };
 
-        return serverUtils.getCodeActions(this._server, req, token).then(response => {
+        return serverUtils.getCodeActions(this._server, request, token).then(response => {
             return response.CodeActions.map(codeAction => {
+                let runRequest: protocol.V2.RunCodeActionRequest = {
+                    FileName: document.fileName,
+                    Line: line,
+                    Column: column,
+                    Selection: selection,
+                    Identifier: codeAction.Identifier,
+                    WantsTextChanges: true
+                };
+
                 return {
                     title: codeAction.Name,
                     command: this._commandId,
-                    arguments: [<protocol.V2.RunCodeActionRequest>{
-                        FileName: document.fileName,
-                        Selection: OmnisharpCodeActionProvider._asRange(range),
-                        Identifier: codeAction.Identifier,
-                        WantsTextChanges: true
-                    }]
+                    arguments: [runRequest]
                 };
             });
         }, (error) => {
@@ -66,31 +108,23 @@ export default class OmnisharpCodeActionProvider extends AbstractProvider implem
 
             if (response && Array.isArray(response.Changes)) {
 
-                let edit = new WorkspaceEdit();
+                let edit = new vscode.WorkspaceEdit();
 
                 for (let change of response.Changes) {
-                    let uri = Uri.file(change.FileName);
-                    let edits: TextEdit[] = [];
+                    let uri = vscode.Uri.file(change.FileName);
+                    let edits: vscode.TextEdit[] = [];
                     for (let textChange of change.Changes) {
-                        edits.push(TextEdit.replace(toRange2(textChange), textChange.NewText));
+                        edits.push(vscode.TextEdit.replace(toRange2(textChange), textChange.NewText));
                     }
 
                     edit.set(uri, edits);
                 }
 
-                return workspace.applyEdit(edit);
+                return vscode.workspace.applyEdit(edit);
             }
 
         }, (error) => {
-            return Promise.reject('Problem invoking \'RunCodeAction\' on OmniSharp server: ' + error);
+            return Promise.reject(`Problem invoking 'RunCodeAction' on OmniSharp server: ${error}`);
         });
     }
-
-    private static _asRange(range: Range): protocol.V2.Range {
-        let {start, end} = range;
-        return {
-            Start: { Line: start.line + 1, Column: start.character + 1 },
-            End: { Line: end.line + 1, Column: end.character + 1 }
-        };
-    }
-}
+}

src/omnisharp/protocol.ts

@@ -436,7 +436,7 @@ export namespace V2 {
     }
 
     export interface GetCodeActionsRequest extends Request {
-        Selection: Range;
+        Selection?: Range;
     }
 
     export interface OmniSharpCodeAction {
@@ -450,7 +450,7 @@ export namespace V2 {
 
     export interface RunCodeActionRequest extends Request {
         Identifier: string;
-        Selection: Range;
+        Selection?: Range;
         WantsTextChanges: boolean;
     }