Use correct span when requesting code actions from OmniSharp

Fixes #1605

When calling CodeActionProvider.provideCodeActions(...), VS Code passes the range of the word to the left of the editor caret if there's no selection. This ends up causing refactorings like Extract Method to be offered even when the user hasn't selected any code. Now, we'll check to see if there's actually a selection and use that instead of the range that's given.

To fully fix this issue, I've made several changes in OmniSharp: OmniSharp/omnisharp-roslyn#899. Once that goes in, we'll need to update the version of OmniSharp used by C# for VS Code.

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;
     }