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, 125 insertions, 0 deletions

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 000000000..f0328893e
--- /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);
+    });
+});