From abbdaf75a2df998f3c40ccc66c63cf7592dd2c15 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 28 Jun 2017 11:24:57 -0700 Subject: [PATCH] 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: https://github.com/OmniSharp/omnisharp-roslyn/pull/899. Once that goes in, we'll need to update the version of OmniSharp used by C# for VS Code. --- src/features/codeActionProvider.ts | 102 +++++++++++++++++++---------- src/omnisharp/protocol.ts | 4 +- 2 files changed, 70 insertions(+), 36 deletions(-) diff --git a/src/features/codeActionProvider.ts b/src/features/codeActionProvider.ts index 81b164611..073dc584d 100644 --- a/src/features/codeActionProvider.ts +++ b/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 { + public provideCodeActions(document: vscode.TextDocument, range: vscode.Range, context: vscode.CodeActionContext, token: vscode.CancellationToken): Promise { 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: [{ - 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 } - }; - } -} +} \ No newline at end of file diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index 38a766f30..422e1c926 100644 --- a/src/omnisharp/protocol.ts +++ b/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; }