diff --git a/CHANGELOG.md b/CHANGELOG.md index 19efc1f70..d1440d146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ## 1.23.5 (Not yet released) * Set meaning of UseGlobalMono "auto" to "never" since Mono 6.12.0 still ships with MSBuild 16.7 (PR: [#4130](https://github.com/OmniSharp/omnisharp-vscode/pull/4130)) +* Ensure that the rename identifier and run code action providers do not apply changes twice PR: [4133](https://github.com/OmniSharp/omnisharp-vscode/pull/4133) ## 1.23.4 (October, 19, 2020) * Use incremental changes to update language server (PR: [#4088](https://github.com/OmniSharp/omnisharp-vscode/pull/4088)) diff --git a/src/features/codeActionProvider.ts b/src/features/codeActionProvider.ts index 95a609f06..d519924cd 100644 --- a/src/features/codeActionProvider.ts +++ b/src/features/codeActionProvider.ts @@ -7,13 +7,11 @@ import * as vscode from 'vscode'; import { OmniSharpServer } from '../omnisharp/server'; import AbstractProvider from './abstractProvider'; import * as protocol from '../omnisharp/protocol'; -import { toRange2 } from '../omnisharp/typeConversion'; import * as serverUtils from '../omnisharp/utils'; -import { FileModificationType } from '../omnisharp/protocol'; -import { Uri } from 'vscode'; import CompositeDisposable from '../CompositeDisposable'; import OptionProvider from '../observers/OptionProvider'; import { LanguageMiddlewareFeature } from '../omnisharp/LanguageMiddlewareFeature'; +import { buildEditForResponse } from '../omnisharp/fileOperationsResponseEditBuilder'; export default class CodeActionProvider extends AbstractProvider implements vscode.CodeActionProvider { @@ -84,7 +82,8 @@ export default class CodeActionProvider extends AbstractProvider implements vsco Selection: selection, Identifier: codeAction.Identifier, WantsTextChanges: true, - WantsAllCodeActionOperations: true + WantsAllCodeActionOperations: true, + ApplyTextChanges: false }; return { @@ -101,80 +100,12 @@ export default class CodeActionProvider extends AbstractProvider implements vsco private async _runCodeAction(req: protocol.V2.RunCodeActionRequest, token: vscode.CancellationToken): Promise { - return serverUtils.runCodeAction(this._server, req).then(async response => { - if (response && Array.isArray(response.Changes)) { - - let edit = new vscode.WorkspaceEdit(); - - let fileToOpen: Uri = null; - let renamedFiles: Uri[] = []; - - for (let change of response.Changes) { - if (change.ModificationType == FileModificationType.Renamed) - { - // The file was renamed. Omnisharp has already persisted - // the right changes to disk. We don't need to try to - // apply text changes (and will skip this file if we see an edit) - renamedFiles.push(Uri.file(change.FileName)); - } - } - - for (let change of response.Changes) { - if (change.ModificationType == FileModificationType.Opened) - { - // The CodeAction requested that we open a file. - // Record that file name and keep processing CodeActions. - // If a CodeAction requests that we open multiple files - // we only open the last one (what would it mean to open multiple files?) - fileToOpen = vscode.Uri.file(change.FileName); - } - - if (change.ModificationType == FileModificationType.Modified) - { - let uri = vscode.Uri.file(change.FileName); - if (renamedFiles.some(r => r == uri)) - { - // This file got renamed. OmniSharp has already - // persisted the new file with any applicable changes. - continue; - } - - let edits: vscode.TextEdit[] = []; - for (let textChange of change.Changes) { - edits.push(vscode.TextEdit.replace(toRange2(textChange), textChange.NewText)); - } - - edit.set(uri, edits); - } - } - - // Allow language middlewares to re-map its edits if necessary. - edit = await this._languageMiddlewareFeature.remap("remapWorkspaceEdit", edit, token); - - let applyEditPromise = vscode.workspace.applyEdit(edit); - - // Unfortunately, the textEditor.Close() API has been deprecated - // and replaced with a command that can only close the active editor. - // If files were renamed that weren't the active editor, their tabs will - // be left open and marked as "deleted" by VS Code - let next = applyEditPromise; - if (renamedFiles.some(r => r.fsPath == vscode.window.activeTextEditor.document.uri.fsPath)) - { - next = applyEditPromise.then(_ => - { - return vscode.commands.executeCommand("workbench.action.closeActiveEditor"); - }); - } - - return fileToOpen != null - ? next.then(_ => - { - return vscode.commands.executeCommand("vscode.open", fileToOpen); - }) - : next; - } - }, async (error) => { + return serverUtils.runCodeAction(this._server, req).then(response => { + if (response) { + return buildEditForResponse(response.Changes, this._languageMiddlewareFeature, token); + } + }, async (error) => { return Promise.reject(`Problem invoking 'RunCodeAction' on OmniSharp server: ${error}`); }); } -} \ No newline at end of file +} diff --git a/src/features/fixAllProvider.ts b/src/features/fixAllProvider.ts index e5da42c8c..1299755b0 100644 --- a/src/features/fixAllProvider.ts +++ b/src/features/fixAllProvider.ts @@ -7,12 +7,12 @@ import * as vscode from 'vscode'; import * as serverUtils from '../omnisharp/utils'; import * as protocol from '../omnisharp/protocol'; import { OmniSharpServer } from '../omnisharp/server'; -import { FixAllScope, FixAllItem, FileModificationType } from '../omnisharp/protocol'; -import { Uri } from 'vscode'; +import { FixAllScope, FixAllItem } from '../omnisharp/protocol'; import CompositeDisposable from '../CompositeDisposable'; import AbstractProvider from './abstractProvider'; -import { toRange2 } from '../omnisharp/typeConversion'; import { LanguageMiddlewareFeature } from '../omnisharp/LanguageMiddlewareFeature'; +import { buildEditForResponse } from '../omnisharp/fileOperationsResponseEditBuilder'; +import { CancellationToken } from 'vscode-languageserver-protocol'; export class FixAllProvider extends AbstractProvider implements vscode.CodeActionProvider { public constructor(private server: OmniSharpServer, languageMiddlewareFeature: LanguageMiddlewareFeature) { @@ -80,72 +80,8 @@ export class FixAllProvider extends AbstractProvider implements vscode.CodeActio ApplyChanges: false }); - if (response && Array.isArray(response.Changes)) { - let edit = new vscode.WorkspaceEdit(); - - let fileToOpen: Uri = null; - let renamedFiles: Uri[] = []; - - for (let change of response.Changes) { - if (change.ModificationType == FileModificationType.Renamed) - { - // The file was renamed. Omnisharp has already persisted - // the right changes to disk. We don't need to try to - // apply text changes (and will skip this file if we see an edit) - renamedFiles.push(Uri.file(change.FileName)); - } - } - - for (let change of response.Changes) { - if (change.ModificationType == FileModificationType.Opened) - { - // The CodeAction requested that we open a file. - // Record that file name and keep processing CodeActions. - // If a CodeAction requests that we open multiple files - // we only open the last one (what would it mean to open multiple files?) - fileToOpen = vscode.Uri.file(change.FileName); - } - - if (change.ModificationType == FileModificationType.Modified) - { - let uri = vscode.Uri.file(change.FileName); - if (renamedFiles.some(r => r == uri)) - { - // This file got renamed. Omnisharp has already - // persisted the new file with any applicable changes. - continue; - } - - let edits: vscode.TextEdit[] = []; - for (let textChange of change.Changes) { - edits.push(vscode.TextEdit.replace(toRange2(textChange), textChange.NewText)); - } - - edit.set(uri, edits); - } - } - - let applyEditPromise = vscode.workspace.applyEdit(edit); - - // Unfortunately, the textEditor.Close() API has been deprecated - // and replaced with a command that can only close the active editor. - // If files were renamed that weren't the active editor, their tabs will - // be left open and marked as "deleted" by VS Code - let next = applyEditPromise; - if (renamedFiles.some(r => r.fsPath == vscode.window.activeTextEditor.document.uri.fsPath)) - { - next = applyEditPromise.then(_ => - { - return vscode.commands.executeCommand("workbench.action.closeActiveEditor"); - }); - } - - return fileToOpen != null - ? next.then(_ => - { - return vscode.commands.executeCommand("vscode.open", fileToOpen); - }) - : next; + if (response) { + return buildEditForResponse(response.Changes, this._languageMiddlewareFeature, CancellationToken.None); } } } diff --git a/src/features/renameProvider.ts b/src/features/renameProvider.ts index e4bc47afc..733ab274c 100644 --- a/src/features/renameProvider.ts +++ b/src/features/renameProvider.ts @@ -16,6 +16,7 @@ export default class OmnisharpRenameProvider extends AbstractSupport implements let req = createRequest(document, position); req.WantsTextChanges = true; req.RenameTo = newName; + req.ApplyTextChanges = false; try { let response = await serverUtils.rename(this._server, req, token); diff --git a/src/omnisharp/fileOperationsResponseEditBuilder.ts b/src/omnisharp/fileOperationsResponseEditBuilder.ts new file mode 100644 index 000000000..8c4bd6c66 --- /dev/null +++ b/src/omnisharp/fileOperationsResponseEditBuilder.ts @@ -0,0 +1,63 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { Uri } from 'vscode'; +import { LanguageMiddlewareFeature } from './LanguageMiddlewareFeature'; +import { FileModificationType, FileOperationResponse, ModifiedFileResponse, RenamedFileResponse } from "./protocol"; +import { toRange2 } from './typeConversion'; + +export async function buildEditForResponse(changes: FileOperationResponse[], languageMiddlewareFeature: LanguageMiddlewareFeature, token: vscode.CancellationToken): Promise { + let edit = new vscode.WorkspaceEdit(); + + let fileToOpen: Uri = null; + + if (!changes || !Array.isArray(changes) || !changes.length) { + return true; + } + + for (const change of changes) { + if (change.ModificationType == FileModificationType.Opened) { + // The CodeAction requested that we open a file. + // Record that file name and keep processing CodeActions. + // If a CodeAction requests that we open multiple files + // we only open the last one (what would it mean to open multiple files?) + fileToOpen = vscode.Uri.file(change.FileName); + } + + if (change.ModificationType == FileModificationType.Modified) { + const modifiedChange = change; + const uri = vscode.Uri.file(modifiedChange.FileName); + let edits: vscode.TextEdit[] = []; + for (let textChange of modifiedChange.Changes) { + edits.push(vscode.TextEdit.replace(toRange2(textChange), textChange.NewText)); + } + + edit.set(uri, edits); + } + } + + for (const change of changes) { + if (change.ModificationType == FileModificationType.Renamed) { + const renamedChange = change; + edit.renameFile(vscode.Uri.file(renamedChange.FileName), vscode.Uri.file(renamedChange.NewFileName)); + } + } + + // Allow language middlewares to re-map its edits if necessary. + edit = await languageMiddlewareFeature.remap("remapWorkspaceEdit", edit, token); + + const applyEditPromise = vscode.workspace.applyEdit(edit); + + // Unfortunately, the textEditor.Close() API has been deprecated + // and replaced with a command that can only close the active editor. + // If files were renamed that weren't the active editor, their tabs will + // be left open and marked as "deleted" by VS Code + return fileToOpen != null + ? applyEditPromise.then(_ => { + return vscode.commands.executeCommand("vscode.open", fileToOpen); + }) + : applyEditPromise; +} diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index 35d8d42e6..5cb73f2c1 100644 --- a/src/omnisharp/protocol.ts +++ b/src/omnisharp/protocol.ts @@ -262,7 +262,7 @@ export interface GetCodeActionsResponse { export interface RunFixAllActionResponse { Text: string; - Changes: ModifiedFileResponse[]; + Changes: FileOperationResponse[]; } export interface FixAllItem { @@ -371,13 +371,24 @@ export interface DotNetFramework { export interface RenameRequest extends Request { RenameTo: string; WantsTextChanges?: boolean; + ApplyTextChanges: boolean; } -export interface ModifiedFileResponse { +export interface FileOperationResponse { FileName: string; + ModificationType: FileModificationType; +} + +export interface ModifiedFileResponse extends FileOperationResponse { Buffer: string; Changes: TextChange[]; - ModificationType: FileModificationType; +} + +export interface RenamedFileResponse extends FileOperationResponse { + NewFileName: string; +} + +export interface OpenFileResponse extends FileOperationResponse { } export enum FileModificationType { @@ -596,10 +607,11 @@ export namespace V2 { Selection?: Range; WantsTextChanges: boolean; WantsAllCodeActionOperations: boolean; + ApplyTextChanges: boolean; } export interface RunCodeActionResponse { - Changes: ModifiedFileResponse[]; + Changes: FileOperationResponse[]; } export interface MSBuildProjectDiagnostics {