Merge #1454

1454: Fix `cargo watch` code action filtering r=etaoins a=etaoins

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.

Co-authored-by: Ryan Cumming <[email protected]>

1 files changed, 21 insertions, 46 deletions

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<Array<vscode.Command | vscode.CodeAction>> {
-        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,