From abc0784e57610a0cceca63301489918015418df6 Mon Sep 17 00:00:00 2001 From: Ryan Cumming Date: Thu, 27 Jun 2019 21:30:23 +1000 Subject: Fix `cargo watch` code action filtering There are two issues with the implementation of `provideCodeActions` introduced in #1439: 1. We're returning the code action based on the file its diagnostic is in; not the file the suggested fix is in. I'm not sure how often fixes are suggested cross-file but it's something we should handle. 2. We're not filtering code actions based on the passed range. The means if there is any suggestion in a file we'll show an action for every line of the file. I naively thought that VS Code would filter for us but that was wrong. Unfortunately the VS Code `CodeAction` object is very complex - it can handle edits across multiple files, run commands, etc. This makes it complex to check them for equality or see if any of their edits intersects with a specified range. To make it easier to work with suggestions this introduces a `SuggestedFix` model object and a `SuggestFixCollection` code action provider. This is a layer between the raw Rust JSON and VS Code's `CodeAction`s. I was reluctant to introduce another layer of abstraction here but my attempt to work directly with VS Code's model objects was worse. --- editors/code/src/commands/cargo_watch.ts | 67 ++++++++++---------------------- 1 file changed, 21 insertions(+), 46 deletions(-) (limited to 'editors/code/src/commands') diff --git a/editors/code/src/commands/cargo_watch.ts b/editors/code/src/commands/cargo_watch.ts index 1ec5f8d5f..4c3c10c8b 100644 --- a/editors/code/src/commands/cargo_watch.ts +++ b/editors/code/src/commands/cargo_watch.ts @@ -5,16 +5,15 @@ import * as vscode from 'vscode'; import { Server } from '../server'; import { terminate } from '../utils/processes'; +import { LineBuffer } from './line_buffer'; +import { StatusDisplay } from './watch_status'; + import { mapRustDiagnosticToVsCode, RustDiagnostic -} from '../utils/rust_diagnostics'; -import { - areCodeActionsEqual, - areDiagnosticsEqual -} from '../utils/vscode_diagnostics'; -import { LineBuffer } from './line_buffer'; -import { StatusDisplay } from './watch_status'; +} from '../utils/diagnostics/rust'; +import SuggestedFixCollection from '../utils/diagnostics/SuggestedFixCollection'; +import { areDiagnosticsEqual } from '../utils/diagnostics/vscode'; export function registerCargoWatchProvider( subscriptions: vscode.Disposable[] @@ -42,16 +41,13 @@ export function registerCargoWatchProvider( return provider; } -export class CargoWatchProvider - implements vscode.Disposable, vscode.CodeActionProvider { +export class CargoWatchProvider implements vscode.Disposable { private readonly diagnosticCollection: vscode.DiagnosticCollection; private readonly statusDisplay: StatusDisplay; private readonly outputChannel: vscode.OutputChannel; - private codeActions: { - [fileUri: string]: vscode.CodeAction[]; - }; - private readonly codeActionDispose: vscode.Disposable; + private suggestedFixCollection: SuggestedFixCollection; + private codeActionDispose: vscode.Disposable; private cargoProcess?: child_process.ChildProcess; @@ -66,13 +62,14 @@ export class CargoWatchProvider 'Cargo Watch Trace' ); - // Register code actions for rustc's suggested fixes - this.codeActions = {}; + // Track `rustc`'s suggested fixes so we can convert them to code actions + this.suggestedFixCollection = new SuggestedFixCollection(); this.codeActionDispose = vscode.languages.registerCodeActionsProvider( [{ scheme: 'file', language: 'rust' }], - this, + this.suggestedFixCollection, { - providedCodeActionKinds: [vscode.CodeActionKind.QuickFix] + providedCodeActionKinds: + SuggestedFixCollection.PROVIDED_CODE_ACTION_KINDS } ); } @@ -156,13 +153,6 @@ export class CargoWatchProvider this.codeActionDispose.dispose(); } - public provideCodeActions( - document: vscode.TextDocument - ): vscode.ProviderResult> { - const documentActions = this.codeActions[document.uri.toString()]; - return documentActions || []; - } - private logInfo(line: string) { if (Server.config.cargoWatchOptions.trace === 'verbose') { this.outputChannel.append(line); @@ -181,7 +171,7 @@ export class CargoWatchProvider private parseLine(line: string) { if (line.startsWith('[Running')) { this.diagnosticCollection.clear(); - this.codeActions = {}; + this.suggestedFixCollection.clear(); this.statusDisplay.show(); } @@ -225,7 +215,7 @@ export class CargoWatchProvider return; } - const { location, diagnostic, codeActions } = mapResult; + const { location, diagnostic, suggestedFixes } = mapResult; const fileUri = location.uri; const diagnostics: vscode.Diagnostic[] = [ @@ -236,7 +226,6 @@ export class CargoWatchProvider const isDuplicate = diagnostics.some(d => areDiagnosticsEqual(d, diagnostic) ); - if (isDuplicate) { return; } @@ -244,29 +233,15 @@ export class CargoWatchProvider diagnostics.push(diagnostic); this.diagnosticCollection!.set(fileUri, diagnostics); - if (codeActions.length) { - const fileUriString = fileUri.toString(); - const existingActions = this.codeActions[fileUriString] || []; - - for (const newAction of codeActions) { - const existingAction = existingActions.find(existing => - areCodeActionsEqual(existing, newAction) + if (suggestedFixes.length) { + for (const suggestedFix of suggestedFixes) { + this.suggestedFixCollection.addSuggestedFixForDiagnostic( + suggestedFix, + diagnostic ); - - if (existingAction) { - if (!existingAction.diagnostics) { - existingAction.diagnostics = []; - } - // This action also applies to this diagnostic - existingAction.diagnostics.push(diagnostic); - } else { - newAction.diagnostics = [diagnostic]; - existingActions.push(newAction); - } } // Have VsCode query us for the code actions - this.codeActions[fileUriString] = existingActions; vscode.commands.executeCommand( 'vscode.executeCodeActionProvider', fileUri, -- cgit v1.2.3