Repository: rust-analyzer/rust-analyzer
Pull request #1454 (2019-06-29 07:48:54)

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.


Patch:
diff --git a/editors/code/src/commands/cargo_watch.ts b/editors/code/src/commands/cargo_watch.ts
index 1ec5f8d5f0..4c3c10c8b1 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,
diff --git a/editors/code/src/utils/diagnostics/SuggestedFix.ts b/editors/code/src/utils/diagnostics/SuggestedFix.ts
new file mode 100644
index 0000000000..b1be2a225b
--- /dev/null
+++ b/editors/code/src/utils/diagnostics/SuggestedFix.ts
@@ -0,0 +1,67 @@
+import * as vscode from 'vscode';
+
+import { SuggestionApplicability } from './rust';
+
+/**
+ * Model object for text replacements suggested by the Rust compiler
+ *
+ * This is an intermediate form between the raw `rustc` JSON and a
+ * `vscode.CodeAction`. It's optimised for the use-cases of
+ * `SuggestedFixCollection`.
+ */
+export default class SuggestedFix {
+    public readonly title: string;
+    public readonly location: vscode.Location;
+    public readonly replacement: string;
+    public readonly applicability: SuggestionApplicability;
+
+    /**
+     * Diagnostics this suggested fix could resolve
+     */
+    public diagnostics: vscode.Diagnostic[];
+
+    constructor(
+        title: string,
+        location: vscode.Location,
+        replacement: string,
+        applicability: SuggestionApplicability = SuggestionApplicability.Unspecified
+    ) {
+        this.title = title;
+        this.location = location;
+        this.replacement = replacement;
+        this.applicability = applicability;
+        this.diagnostics = [];
+    }
+
+    /**
+     * Determines if this suggested fix is equivalent to another instance
+     */
+    public isEqual(other: SuggestedFix): boolean {
+        return (
+            this.title === other.title &&
+            this.location.range.isEqual(other.location.range) &&
+            this.replacement === other.replacement &&
+            this.applicability === other.applicability
+        );
+    }
+
+    /**
+     * Converts this suggested fix to a VS Code Quick Fix code action
+     */
+    public toCodeAction(): vscode.CodeAction {
+        const codeAction = new vscode.CodeAction(
+            this.title,
+            vscode.CodeActionKind.QuickFix
+        );
+
+        const edit = new vscode.WorkspaceEdit();
+        edit.replace(this.location.uri, this.location.range, this.replacement);
+        codeAction.edit = edit;
+
+        codeAction.isPreferred =
+            this.applicability === SuggestionApplicability.MachineApplicable;
+
+        codeAction.diagnostics = [...this.diagnostics];
+        return codeAction;
+    }
+}
diff --git a/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts
new file mode 100644
index 0000000000..132ce12f89
--- /dev/null
+++ b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts
@@ -0,0 +1,77 @@
+import * as vscode from 'vscode';
+import SuggestedFix from './SuggestedFix';
+
+/**
+ * Collection of suggested fixes across multiple documents
+ *
+ * This stores `SuggestedFix` model objects and returns them via the
+ * `vscode.CodeActionProvider` interface.
+ */
+export default class SuggestedFixCollection
+    implements vscode.CodeActionProvider {
+    public static PROVIDED_CODE_ACTION_KINDS = [vscode.CodeActionKind.QuickFix];
+
+    /**
+     * Map of document URI strings to suggested fixes
+     */
+    private suggestedFixes: Map<string, SuggestedFix[]>;
+
+    constructor() {
+        this.suggestedFixes = new Map();
+    }
+
+    /**
+     * Clears all suggested fixes across all documents
+     */
+    public clear(): void {
+        this.suggestedFixes = new Map();
+    }
+
+    /**
+     * Adds a suggested fix for the given diagnostic
+     *
+     * Some suggested fixes will appear in multiple diagnostics. For example,
+     * forgetting a `mut` on a variable will suggest changing the delaration on
+     * every mutable usage site. If the suggested fix has already been added
+     * this method will instead associate the existing fix with the new
+     * diagnostic.
+     */
+    public addSuggestedFixForDiagnostic(
+        suggestedFix: SuggestedFix,
+        diagnostic: vscode.Diagnostic
+    ): void {
+        const fileUriString = suggestedFix.location.uri.toString();
+        const fileSuggestions = this.suggestedFixes.get(fileUriString) || [];
+
+        const existingSuggestion = fileSuggestions.find(s =>
+            s.isEqual(suggestedFix)
+        );
+
+        if (existingSuggestion) {
+            // The existing suggestion also applies to this new diagnostic
+            existingSuggestion.diagnostics.push(diagnostic);
+        } else {
+            // We haven't seen this suggestion before
+            suggestedFix.diagnostics.push(diagnostic);
+            fileSuggestions.push(suggestedFix);
+        }
+
+        this.suggestedFixes.set(fileUriString, fileSuggestions);
+    }
+
+    /**
+     * Filters suggested fixes by their document and range and converts them to
+     * code actions
+     */
+    public provideCodeActions(
+        document: vscode.TextDocument,
+        range: vscode.Range
+    ): vscode.CodeAction[] {
+        const documentUriString = document.uri.toString();
+
+        const suggestedFixes = this.suggestedFixes.get(documentUriString);
+        return (suggestedFixes || [])
+            .filter(({ location }) => location.range.intersection(range))
+            .map(suggestedEdit => suggestedEdit.toCodeAction());
+    }
+}
diff --git a/editors/code/src/utils/rust_diagnostics.ts b/editors/code/src/utils/diagnostics/rust.ts
similarity index 85%
rename from editors/code/src/utils/rust_diagnostics.ts
rename to editors/code/src/utils/diagnostics/rust.ts
index 3c524cb378..d16576eb1e 100644
--- a/editors/code/src/utils/rust_diagnostics.ts
+++ b/editors/code/src/utils/diagnostics/rust.ts
@@ -1,6 +1,15 @@
 import * as path from 'path';
 import * as vscode from 'vscode';
 
+import SuggestedFix from './SuggestedFix';
+
+export enum SuggestionApplicability {
+    MachineApplicable = 'MachineApplicable',
+    HasPlaceholders = 'HasPlaceholders',
+    MaybeIncorrect = 'MaybeIncorrect',
+    Unspecified = 'Unspecified'
+}
+
 // Reference:
 // https://github.com/rust-lang/rust/blob/master/src/libsyntax/json.rs
 export interface RustDiagnosticSpan {
@@ -12,11 +21,7 @@ export interface RustDiagnosticSpan {
     file_name: string;
     label?: string;
     suggested_replacement?: string;
-    suggestion_applicability?:
-        | 'MachineApplicable'
-        | 'HasPlaceholders'
-        | 'MaybeIncorrect'
-        | 'Unspecified';
+    suggestion_applicability?: SuggestionApplicability;
 }
 
 export interface RustDiagnostic {
@@ -33,12 +38,12 @@ export interface RustDiagnostic {
 export interface MappedRustDiagnostic {
     location: vscode.Location;
     diagnostic: vscode.Diagnostic;
-    codeActions: vscode.CodeAction[];
+    suggestedFixes: SuggestedFix[];
 }
 
 interface MappedRustChildDiagnostic {
     related?: vscode.DiagnosticRelatedInformation;
-    codeAction?: vscode.CodeAction;
+    suggestedFix?: SuggestedFix;
     messageLine?: string;
 }
 
@@ -130,24 +135,19 @@ function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic {
 
     // We need to distinguish `null` from an empty string
     if (span && typeof span.suggested_replacement === 'string') {
-        const edit = new vscode.WorkspaceEdit();
-        edit.replace(location.uri, location.range, span.suggested_replacement);
-
-        // Include our replacement in the label unless it's empty
+        // Include our replacement in the title unless it's empty
         const title = span.suggested_replacement
             ? `${rd.message}: \`${span.suggested_replacement}\``
             : rd.message;
 
-        const codeAction = new vscode.CodeAction(
-            title,
-            vscode.CodeActionKind.QuickFix
-        );
-
-        codeAction.edit = edit;
-        codeAction.isPreferred =
-            span.suggestion_applicability === 'MachineApplicable';
-
-        return { codeAction };
+        return {
+            suggestedFix: new SuggestedFix(
+                title,
+                location,
+                span.suggested_replacement,
+                span.suggestion_applicability
+            )
+        };
     } else {
         const related = new vscode.DiagnosticRelatedInformation(
             location,
@@ -165,7 +165,7 @@ function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic {
  *
  * 1. Creating a `vscode.Diagnostic` with the root message and primary span.
  * 2. Adding any labelled secondary spans to `relatedInformation`
- * 3. Categorising child diagnostics as either Quick Fix actions,
+ * 3. Categorising child diagnostics as either `SuggestedFix`es,
  *    `relatedInformation` or additional message lines.
  *
  * If the diagnostic has no primary span this will return `undefined`
@@ -173,8 +173,6 @@ function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic {
 export function mapRustDiagnosticToVsCode(
     rd: RustDiagnostic
 ): MappedRustDiagnostic | undefined {
-    const codeActions = [];
-
     const primarySpan = rd.spans.find(s => s.is_primary);
     if (!primarySpan) {
         return;
@@ -208,16 +206,17 @@ export function mapRustDiagnosticToVsCode(
         }
     }
 
+    const suggestedFixes = [];
     for (const child of rd.children) {
-        const { related, codeAction, messageLine } = mapRustChildDiagnostic(
+        const { related, suggestedFix, messageLine } = mapRustChildDiagnostic(
             child
         );
 
         if (related) {
             vd.relatedInformation.push(related);
         }
-        if (codeAction) {
-            codeActions.push(codeAction);
+        if (suggestedFix) {
+            suggestedFixes.push(suggestedFix);
         }
         if (messageLine) {
             vd.message += `\n${messageLine}`;
@@ -231,6 +230,6 @@ export function mapRustDiagnosticToVsCode(
     return {
         location,
         diagnostic: vd,
-        codeActions
+        suggestedFixes
     };
 }
diff --git a/editors/code/src/utils/diagnostics/vscode.ts b/editors/code/src/utils/diagnostics/vscode.ts
new file mode 100644
index 0000000000..d8b85b7200
--- /dev/null
+++ b/editors/code/src/utils/diagnostics/vscode.ts
@@ -0,0 +1,14 @@
+import * as vscode from 'vscode';
+
+/** Compares two `vscode.Diagnostic`s for equality */
+export function areDiagnosticsEqual(
+    left: vscode.Diagnostic,
+    right: vscode.Diagnostic
+): boolean {
+    return (
+        left.source === right.source &&
+        left.severity === right.severity &&
+        left.range.isEqual(right.range) &&
+        left.message === right.message
+    );
+}
diff --git a/editors/code/src/utils/vscode_diagnostics.ts b/editors/code/src/utils/vscode_diagnostics.ts
deleted file mode 100644
index 9d763c8d64..0000000000
--- a/editors/code/src/utils/vscode_diagnostics.ts
+++ /dev/null
@@ -1,73 +0,0 @@
-import * as vscode from 'vscode';
-
-/** Compares two `vscode.Diagnostic`s for equality */
-export function areDiagnosticsEqual(
-    left: vscode.Diagnostic,
-    right: vscode.Diagnostic
-): boolean {
-    return (
-        left.source === right.source &&
-        left.severity === right.severity &&
-        left.range.isEqual(right.range) &&
-        left.message === right.message
-    );
-}
-
-/** Compares two `vscode.TextEdit`s for equality */
-function areTextEditsEqual(
-    left: vscode.TextEdit,
-    right: vscode.TextEdit
-): boolean {
-    if (!left.range.isEqual(right.range)) {
-        return false;
-    }
-
-    if (left.newText !== right.newText) {
-        return false;
-    }
-
-    return true;
-}
-
-/** Compares two `vscode.CodeAction`s for equality */
-export function areCodeActionsEqual(
-    left: vscode.CodeAction,
-    right: vscode.CodeAction
-): boolean {
-    if (
-        left.kind !== right.kind ||
-        left.title !== right.title ||
-        !left.edit ||
-        !right.edit
-    ) {
-        return false;
-    }
-
-    const leftEditEntries = left.edit.entries();
-    const rightEditEntries = right.edit.entries();
-
-    if (leftEditEntries.length !== rightEditEntries.length) {
-        return false;
-    }
-
-    for (let i = 0; i < leftEditEntries.length; i++) {
-        const [leftUri, leftEdits] = leftEditEntries[i];
-        const [rightUri, rightEdits] = rightEditEntries[i];
-
-        if (leftUri.toString() !== rightUri.toString()) {
-            return false;
-        }
-
-        if (leftEdits.length !== rightEdits.length) {
-            return false;
-        }
-
-        for (let j = 0; j < leftEdits.length; j++) {
-            if (!areTextEditsEqual(leftEdits[j], rightEdits[j])) {
-                return false;
-            }
-        }
-    }
-
-    return true;
-}


Test patch:
diff --git a/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts b/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts
new file mode 100644
index 0000000000..6c7f436f35
--- /dev/null
+++ b/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts
@@ -0,0 +1,133 @@
+import * as assert from 'assert';
+import * as vscode from 'vscode';
+
+import { SuggestionApplicability } from '../../../utils/diagnostics/rust';
+import SuggestedFix from '../../../utils/diagnostics/SuggestedFix';
+
+const location1 = new vscode.Location(
+    vscode.Uri.file('/file/1'),
+    new vscode.Range(new vscode.Position(1, 2), new vscode.Position(3, 4))
+);
+
+const location2 = new vscode.Location(
+    vscode.Uri.file('/file/2'),
+    new vscode.Range(new vscode.Position(5, 6), new vscode.Position(7, 8))
+);
+
+describe('SuggestedFix', () => {
+    describe('isEqual', () => {
+        it('should treat identical instances as equal', () => {
+            const suggestion1 = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With this!'
+            );
+
+            const suggestion2 = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With this!'
+            );
+
+            assert(suggestion1.isEqual(suggestion2));
+        });
+
+        it('should treat instances with different titles as inequal', () => {
+            const suggestion1 = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With this!'
+            );
+
+            const suggestion2 = new SuggestedFix(
+                'Not the same title!',
+                location1,
+                'With this!'
+            );
+
+            assert(!suggestion1.isEqual(suggestion2));
+        });
+
+        it('should treat instances with different replacements as inequal', () => {
+            const suggestion1 = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With this!'
+            );
+
+            const suggestion2 = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With something else!'
+            );
+
+            assert(!suggestion1.isEqual(suggestion2));
+        });
+
+        it('should treat instances with different locations as inequal', () => {
+            const suggestion1 = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With this!'
+            );
+
+            const suggestion2 = new SuggestedFix(
+                'Replace me!',
+                location2,
+                'With this!'
+            );
+
+            assert(!suggestion1.isEqual(suggestion2));
+        });
+
+        it('should treat instances with different applicability as inequal', () => {
+            const suggestion1 = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With this!',
+                SuggestionApplicability.MachineApplicable
+            );
+
+            const suggestion2 = new SuggestedFix(
+                'Replace me!',
+                location2,
+                'With this!',
+                SuggestionApplicability.HasPlaceholders
+            );
+
+            assert(!suggestion1.isEqual(suggestion2));
+        });
+    });
+
+    describe('toCodeAction', () => {
+        it('should map a simple suggestion', () => {
+            const suggestion = new SuggestedFix(
+                'Replace me!',
+                location1,
+                'With this!'
+            );
+
+            const codeAction = suggestion.toCodeAction();
+            assert.strictEqual(codeAction.kind, vscode.CodeActionKind.QuickFix);
+            assert.strictEqual(codeAction.title, 'Replace me!');
+            assert.strictEqual(codeAction.isPreferred, false);
+
+            const edit = codeAction.edit;
+            if (!edit) {
+                return assert.fail('Code Action edit unexpectedly missing');
+            }
+
+            const editEntries = edit.entries();
+            assert.strictEqual(editEntries.length, 1);
+
+            const [[editUri, textEdits]] = editEntries;
+            assert.strictEqual(editUri.toString(), location1.uri.toString());
+
+            assert.strictEqual(textEdits.length, 1);
+            const [textEdit] = textEdits;
+
+            assert(textEdit.range.isEqual(location1.range));
+            assert.strictEqual(textEdit.newText, 'With this!');
+        });
+    });
+});
diff --git a/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts b/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts
new file mode 100644
index 0000000000..f0328893e7
--- /dev/null
+++ b/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts
@@ -0,0 +1,125 @@
+import * as assert from 'assert';
+import * as vscode from 'vscode';
+
+import SuggestedFix from '../../../utils/diagnostics/SuggestedFix';
+import SuggestedFixCollection from '../../../utils/diagnostics/SuggestedFixCollection';
+
+const uri1 = vscode.Uri.file('/file/1');
+const uri2 = vscode.Uri.file('/file/2');
+
+const mockDocument1 = ({
+    uri: uri1
+} as unknown) as vscode.TextDocument;
+
+const mockDocument2 = ({
+    uri: uri2
+} as unknown) as vscode.TextDocument;
+
+const range1 = new vscode.Range(
+    new vscode.Position(1, 2),
+    new vscode.Position(3, 4)
+);
+const range2 = new vscode.Range(
+    new vscode.Position(5, 6),
+    new vscode.Position(7, 8)
+);
+
+const diagnostic1 = new vscode.Diagnostic(range1, 'First diagnostic');
+const diagnostic2 = new vscode.Diagnostic(range2, 'Second diagnostic');
+
+// This is a mutable object so return a fresh instance every time
+function suggestion1(): SuggestedFix {
+    return new SuggestedFix(
+        'Replace me!',
+        new vscode.Location(uri1, range1),
+        'With this!'
+    );
+}
+
+describe('SuggestedFixCollection', () => {
+    it('should add a suggestion then return it as a code action', () => {
+        const suggestedFixes = new SuggestedFixCollection();
+        suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
+
+        // Specify the document and range that exactly matches
+        const codeActions = suggestedFixes.provideCodeActions(
+            mockDocument1,
+            range1
+        );
+
+        assert.strictEqual(codeActions.length, 1);
+        const [codeAction] = codeActions;
+        assert.strictEqual(codeAction.title, suggestion1().title);
+
+        const { diagnostics } = codeAction;
+        if (!diagnostics) {
+            return assert.fail('Diagnostics unexpectedly missing');
+        }
+
+        assert.strictEqual(diagnostics.length, 1);
+        assert.strictEqual(diagnostics[0], diagnostic1);
+    });
+
+    it('should not return code actions for different ranges', () => {
+        const suggestedFixes = new SuggestedFixCollection();
+        suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
+
+        const codeActions = suggestedFixes.provideCodeActions(
+            mockDocument1,
+            range2
+        );
+
+        assert(!codeActions || codeActions.length === 0);
+    });
+
+    it('should not return code actions for different documents', () => {
+        const suggestedFixes = new SuggestedFixCollection();
+        suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
+
+        const codeActions = suggestedFixes.provideCodeActions(
+            mockDocument2,
+            range1
+        );
+
+        assert(!codeActions || codeActions.length === 0);
+    });
+
+    it('should not return code actions that have been cleared', () => {
+        const suggestedFixes = new SuggestedFixCollection();
+        suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
+        suggestedFixes.clear();
+
+        const codeActions = suggestedFixes.provideCodeActions(
+            mockDocument1,
+            range1
+        );
+
+        assert(!codeActions || codeActions.length === 0);
+    });
+
+    it('should merge identical suggestions together', () => {
+        const suggestedFixes = new SuggestedFixCollection();
+
+        // Add the same suggestion for two diagnostics
+        suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
+        suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic2);
+
+        const codeActions = suggestedFixes.provideCodeActions(
+            mockDocument1,
+            range1
+        );
+
+        assert.strictEqual(codeActions.length, 1);
+        const [codeAction] = codeActions;
+        const { diagnostics } = codeAction;
+
+        if (!diagnostics) {
+            return assert.fail('Diagnostics unexpectedly missing');
+        }
+
+        // We should be associated with both diagnostics
+        assert.strictEqual(diagnostics.length, 2);
+        assert.strictEqual(diagnostics[0], diagnostic1);
+        assert.strictEqual(diagnostics[1], diagnostic2);
+    });
+});
diff --git a/editors/code/src/test/rust_diagnostics.test.ts b/editors/code/src/test/utils/diagnotics/rust.test.ts
similarity index 77%
rename from editors/code/src/test/rust_diagnostics.test.ts
rename to editors/code/src/test/utils/diagnotics/rust.test.ts
index f27c58fe29..b555a48198 100644
--- a/editors/code/src/test/rust_diagnostics.test.ts
+++ b/editors/code/src/test/utils/diagnotics/rust.test.ts
@@ -5,14 +5,15 @@ import * as vscode from 'vscode';
 import {
     MappedRustDiagnostic,
     mapRustDiagnosticToVsCode,
-    RustDiagnostic
-} from '../utils/rust_diagnostics';
+    RustDiagnostic,
+    SuggestionApplicability
+} from '../../../utils/diagnostics/rust';
 
 function loadDiagnosticFixture(name: string): RustDiagnostic {
     const jsonText = fs
         .readFileSync(
             // We're actually in our JavaScript output directory, climb out
-            `${__dirname}/../../src/test/fixtures/rust-diagnostics/${name}.json`
+            `${__dirname}/../../../../src/test/fixtures/rust-diagnostics/${name}.json`
         )
         .toString();
 
@@ -31,7 +32,9 @@ function mapFixtureToVsCode(name: string): MappedRustDiagnostic {
 
 describe('mapRustDiagnosticToVsCode', () => {
     it('should map an incompatible type for trait error', () => {
-        const { diagnostic, codeActions } = mapFixtureToVsCode('error/E0053');
+        const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
+            'error/E0053'
+        );
 
         assert.strictEqual(
             diagnostic.severity,
@@ -52,12 +55,12 @@ describe('mapRustDiagnosticToVsCode', () => {
         // No related information
         assert.deepStrictEqual(diagnostic.relatedInformation, []);
 
-        // There are no code actions available
-        assert.strictEqual(codeActions.length, 0);
+        // There are no suggested fixes
+        assert.strictEqual(suggestedFixes.length, 0);
     });
 
     it('should map an unused variable warning', () => {
-        const { diagnostic, codeActions } = mapFixtureToVsCode(
+        const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
             'warning/unused_variables'
         );
 
@@ -81,18 +84,23 @@ describe('mapRustDiagnosticToVsCode', () => {
         // No related information
         assert.deepStrictEqual(diagnostic.relatedInformation, []);
 
-        // One code action available to prefix the variable
-        assert.strictEqual(codeActions.length, 1);
-        const [codeAction] = codeActions;
+        // One suggested fix available to prefix the variable
+        assert.strictEqual(suggestedFixes.length, 1);
+        const [suggestedFix] = suggestedFixes;
         assert.strictEqual(
-            codeAction.title,
+            suggestedFix.title,
             'consider prefixing with an underscore: `_foo`'
         );
-        assert(codeAction.isPreferred);
+        assert.strictEqual(
+            suggestedFix.applicability,
+            SuggestionApplicability.MachineApplicable
+        );
     });
 
     it('should map a wrong number of parameters error', () => {
-        const { diagnostic, codeActions } = mapFixtureToVsCode('error/E0061');
+        const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
+            'error/E0061'
+        );
 
         assert.strictEqual(
             diagnostic.severity,
@@ -115,12 +123,12 @@ describe('mapRustDiagnosticToVsCode', () => {
         const [related] = relatedInformation;
         assert.strictEqual(related.message, 'defined here');
 
-        // There are no actions available
-        assert.strictEqual(codeActions.length, 0);
+        // There are no suggested fixes
+        assert.strictEqual(suggestedFixes.length, 0);
     });
 
     it('should map a Clippy copy pass by ref warning', () => {
-        const { diagnostic, codeActions } = mapFixtureToVsCode(
+        const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
             'clippy/trivially_copy_pass_by_ref'
         );
 
@@ -149,14 +157,17 @@ describe('mapRustDiagnosticToVsCode', () => {
         const [related] = relatedInformation;
         assert.strictEqual(related.message, 'lint level defined here');
 
-        // One code action available to pass by value
-        assert.strictEqual(codeActions.length, 1);
-        const [codeAction] = codeActions;
+        // One suggested fix to pass by value
+        assert.strictEqual(suggestedFixes.length, 1);
+        const [suggestedFix] = suggestedFixes;
         assert.strictEqual(
-            codeAction.title,
+            suggestedFix.title,
             'consider passing by value instead: `self`'
         );
-        // Clippy does not mark this as machine applicable
-        assert.strictEqual(codeAction.isPreferred, false);
+        // Clippy does not mark this with any applicability
+        assert.strictEqual(
+            suggestedFix.applicability,
+            SuggestionApplicability.Unspecified
+        );
     });
 });
diff --git a/editors/code/src/test/vscode_diagnostics.test.ts b/editors/code/src/test/utils/diagnotics/vscode.test.ts
similarity index 50%
rename from editors/code/src/test/vscode_diagnostics.test.ts
rename to editors/code/src/test/utils/diagnotics/vscode.test.ts
index 9c5d812fa3..542dec1f54 100644
--- a/editors/code/src/test/vscode_diagnostics.test.ts
+++ b/editors/code/src/test/utils/diagnotics/vscode.test.ts
@@ -1,12 +1,7 @@
 import * as assert from 'assert';
 import * as vscode from 'vscode';
 
-import {
-    areCodeActionsEqual,
-    areDiagnosticsEqual
-} from '../utils/vscode_diagnostics';
-
-const uri = vscode.Uri.file('/file/1');
+import { areDiagnosticsEqual } from '../../../utils/diagnostics/vscode';
 
 const range1 = new vscode.Range(
     new vscode.Position(1, 2),
@@ -101,82 +96,3 @@ describe('areDiagnosticsEqual', () => {
         assert(!areDiagnosticsEqual(diagnostic1, diagnostic2));
     });
 });
-
-describe('areCodeActionsEqual', () => {
-    it('should treat identical actions as equal', () => {
-        const codeAction1 = new vscode.CodeAction(
-            'Fix me!',
-            vscode.CodeActionKind.QuickFix
-        );
-
-        const codeAction2 = new vscode.CodeAction(
-            'Fix me!',
-            vscode.CodeActionKind.QuickFix
-        );
-
-        const edit = new vscode.WorkspaceEdit();
-        edit.replace(uri, range1, 'Replace with this');
-        codeAction1.edit = edit;
-        codeAction2.edit = edit;
-
-        assert(areCodeActionsEqual(codeAction1, codeAction2));
-    });
-
-    it('should treat actions with different types as inequal', () => {
-        const codeAction1 = new vscode.CodeAction(
-            'Fix me!',
-            vscode.CodeActionKind.Refactor
-        );
-
-        const codeAction2 = new vscode.CodeAction(
-            'Fix me!',
-            vscode.CodeActionKind.QuickFix
-        );
-
-        const edit = new vscode.WorkspaceEdit();
-        edit.replace(uri, range1, 'Replace with this');
-        codeAction1.edit = edit;
-        codeAction2.edit = edit;
-
-        assert(!areCodeActionsEqual(codeAction1, codeAction2));
-    });
-
-    it('should treat actions with different titles as inequal', () => {
-        const codeAction1 = new vscode.CodeAction(
-            'Fix me!',
-            vscode.CodeActionKind.Refactor
-        );
-
-        const codeAction2 = new vscode.CodeAction(
-            'Do something different!',
-            vscode.CodeActionKind.Refactor
-        );
-
-        const edit = new vscode.WorkspaceEdit();
-        edit.replace(uri, range1, 'Replace with this');
-        codeAction1.edit = edit;
-        codeAction2.edit = edit;
-
-        assert(!areCodeActionsEqual(codeAction1, codeAction2));
-    });
-
-    it('should treat actions with different edits as inequal', () => {
-        const codeAction1 = new vscode.CodeAction(
-            'Fix me!',
-            vscode.CodeActionKind.Refactor
-        );
-        const edit1 = new vscode.WorkspaceEdit();
-        edit1.replace(uri, range1, 'Replace with this');
-        codeAction1.edit = edit1;
-
-        const codeAction2 = new vscode.CodeAction(
-            'Fix me!',
-            vscode.CodeActionKind.Refactor
-        );
-        const edit2 = new vscode.WorkspaceEdit();
-        edit2.replace(uri, range1, 'Replace with this other thing');
-        codeAction2.edit = edit2;
-
-        assert(!areCodeActionsEqual(codeAction1, codeAction2));
-    });
-});