aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRyan Cumming <[email protected]>2019-06-27 12:30:23 +0100
committerRyan Cumming <[email protected]>2019-06-29 08:39:36 +0100
commitabc0784e57610a0cceca63301489918015418df6 (patch)
tree05aec9fef88f31cee82e3507903a1dbcd6b4d30d
parent0e1912de528b5092c10eedaf94c43c67d5f86f1a (diff)
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.
-rw-r--r--editors/code/src/commands/cargo_watch.ts67
-rw-r--r--editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts133
-rw-r--r--editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts125
-rw-r--r--editors/code/src/test/utils/diagnotics/rust.test.ts (renamed from editors/code/src/test/rust_diagnostics.test.ts)55
-rw-r--r--editors/code/src/test/utils/diagnotics/vscode.test.ts (renamed from editors/code/src/test/vscode_diagnostics.test.ts)86
-rw-r--r--editors/code/src/utils/diagnostics/SuggestedFix.ts67
-rw-r--r--editors/code/src/utils/diagnostics/SuggestedFixCollection.ts74
-rw-r--r--editors/code/src/utils/diagnostics/rust.ts (renamed from editors/code/src/utils/rust_diagnostics.ts)55
-rw-r--r--editors/code/src/utils/diagnostics/vscode.ts14
-rw-r--r--editors/code/src/utils/vscode_diagnostics.ts73
10 files changed, 495 insertions, 254 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';
5 5
6import { Server } from '../server'; 6import { Server } from '../server';
7import { terminate } from '../utils/processes'; 7import { terminate } from '../utils/processes';
8import { LineBuffer } from './line_buffer';
9import { StatusDisplay } from './watch_status';
10
8import { 11import {
9 mapRustDiagnosticToVsCode, 12 mapRustDiagnosticToVsCode,
10 RustDiagnostic 13 RustDiagnostic
11} from '../utils/rust_diagnostics'; 14} from '../utils/diagnostics/rust';
12import { 15import SuggestedFixCollection from '../utils/diagnostics/SuggestedFixCollection';
13 areCodeActionsEqual, 16import { areDiagnosticsEqual } from '../utils/diagnostics/vscode';
14 areDiagnosticsEqual
15} from '../utils/vscode_diagnostics';
16import { LineBuffer } from './line_buffer';
17import { StatusDisplay } from './watch_status';
18 17
19export function registerCargoWatchProvider( 18export function registerCargoWatchProvider(
20 subscriptions: vscode.Disposable[] 19 subscriptions: vscode.Disposable[]
@@ -42,16 +41,13 @@ export function registerCargoWatchProvider(
42 return provider; 41 return provider;
43} 42}
44 43
45export class CargoWatchProvider 44export class CargoWatchProvider implements vscode.Disposable {
46 implements vscode.Disposable, vscode.CodeActionProvider {
47 private readonly diagnosticCollection: vscode.DiagnosticCollection; 45 private readonly diagnosticCollection: vscode.DiagnosticCollection;
48 private readonly statusDisplay: StatusDisplay; 46 private readonly statusDisplay: StatusDisplay;
49 private readonly outputChannel: vscode.OutputChannel; 47 private readonly outputChannel: vscode.OutputChannel;
50 48
51 private codeActions: { 49 private suggestedFixCollection: SuggestedFixCollection;
52 [fileUri: string]: vscode.CodeAction[]; 50 private codeActionDispose: vscode.Disposable;
53 };
54 private readonly codeActionDispose: vscode.Disposable;
55 51
56 private cargoProcess?: child_process.ChildProcess; 52 private cargoProcess?: child_process.ChildProcess;
57 53
@@ -66,13 +62,14 @@ export class CargoWatchProvider
66 'Cargo Watch Trace' 62 'Cargo Watch Trace'
67 ); 63 );
68 64
69 // Register code actions for rustc's suggested fixes 65 // Track `rustc`'s suggested fixes so we can convert them to code actions
70 this.codeActions = {}; 66 this.suggestedFixCollection = new SuggestedFixCollection();
71 this.codeActionDispose = vscode.languages.registerCodeActionsProvider( 67 this.codeActionDispose = vscode.languages.registerCodeActionsProvider(
72 [{ scheme: 'file', language: 'rust' }], 68 [{ scheme: 'file', language: 'rust' }],
73 this, 69 this.suggestedFixCollection,
74 { 70 {
75 providedCodeActionKinds: [vscode.CodeActionKind.QuickFix] 71 providedCodeActionKinds:
72 SuggestedFixCollection.PROVIDED_CODE_ACTION_KINDS
76 } 73 }
77 ); 74 );
78 } 75 }
@@ -156,13 +153,6 @@ export class CargoWatchProvider
156 this.codeActionDispose.dispose(); 153 this.codeActionDispose.dispose();
157 } 154 }
158 155
159 public provideCodeActions(
160 document: vscode.TextDocument
161 ): vscode.ProviderResult<Array<vscode.Command | vscode.CodeAction>> {
162 const documentActions = this.codeActions[document.uri.toString()];
163 return documentActions || [];
164 }
165
166 private logInfo(line: string) { 156 private logInfo(line: string) {
167 if (Server.config.cargoWatchOptions.trace === 'verbose') { 157 if (Server.config.cargoWatchOptions.trace === 'verbose') {
168 this.outputChannel.append(line); 158 this.outputChannel.append(line);
@@ -181,7 +171,7 @@ export class CargoWatchProvider
181 private parseLine(line: string) { 171 private parseLine(line: string) {
182 if (line.startsWith('[Running')) { 172 if (line.startsWith('[Running')) {
183 this.diagnosticCollection.clear(); 173 this.diagnosticCollection.clear();
184 this.codeActions = {}; 174 this.suggestedFixCollection.clear();
185 this.statusDisplay.show(); 175 this.statusDisplay.show();
186 } 176 }
187 177
@@ -225,7 +215,7 @@ export class CargoWatchProvider
225 return; 215 return;
226 } 216 }
227 217
228 const { location, diagnostic, codeActions } = mapResult; 218 const { location, diagnostic, suggestedFixes } = mapResult;
229 const fileUri = location.uri; 219 const fileUri = location.uri;
230 220
231 const diagnostics: vscode.Diagnostic[] = [ 221 const diagnostics: vscode.Diagnostic[] = [
@@ -236,7 +226,6 @@ export class CargoWatchProvider
236 const isDuplicate = diagnostics.some(d => 226 const isDuplicate = diagnostics.some(d =>
237 areDiagnosticsEqual(d, diagnostic) 227 areDiagnosticsEqual(d, diagnostic)
238 ); 228 );
239
240 if (isDuplicate) { 229 if (isDuplicate) {
241 return; 230 return;
242 } 231 }
@@ -244,29 +233,15 @@ export class CargoWatchProvider
244 diagnostics.push(diagnostic); 233 diagnostics.push(diagnostic);
245 this.diagnosticCollection!.set(fileUri, diagnostics); 234 this.diagnosticCollection!.set(fileUri, diagnostics);
246 235
247 if (codeActions.length) { 236 if (suggestedFixes.length) {
248 const fileUriString = fileUri.toString(); 237 for (const suggestedFix of suggestedFixes) {
249 const existingActions = this.codeActions[fileUriString] || []; 238 this.suggestedFixCollection.addSuggestedFixForDiagnostic(
250 239 suggestedFix,
251 for (const newAction of codeActions) { 240 diagnostic
252 const existingAction = existingActions.find(existing =>
253 areCodeActionsEqual(existing, newAction)
254 ); 241 );
255
256 if (existingAction) {
257 if (!existingAction.diagnostics) {
258 existingAction.diagnostics = [];
259 }
260 // This action also applies to this diagnostic
261 existingAction.diagnostics.push(diagnostic);
262 } else {
263 newAction.diagnostics = [diagnostic];
264 existingActions.push(newAction);
265 }
266 } 242 }
267 243
268 // Have VsCode query us for the code actions 244 // Have VsCode query us for the code actions
269 this.codeActions[fileUriString] = existingActions;
270 vscode.commands.executeCommand( 245 vscode.commands.executeCommand(
271 'vscode.executeCodeActionProvider', 246 'vscode.executeCodeActionProvider',
272 fileUri, 247 fileUri,
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 000000000..6c7f436f3
--- /dev/null
+++ b/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts
@@ -0,0 +1,133 @@
1import * as assert from 'assert';
2import * as vscode from 'vscode';
3
4import { SuggestionApplicability } from '../../../utils/diagnostics/rust';
5import SuggestedFix from '../../../utils/diagnostics/SuggestedFix';
6
7const location1 = new vscode.Location(
8 vscode.Uri.file('/file/1'),
9 new vscode.Range(new vscode.Position(1, 2), new vscode.Position(3, 4))
10);
11
12const location2 = new vscode.Location(
13 vscode.Uri.file('/file/2'),
14 new vscode.Range(new vscode.Position(5, 6), new vscode.Position(7, 8))
15);
16
17describe('SuggestedFix', () => {
18 describe('isEqual', () => {
19 it('should treat identical instances as equal', () => {
20 const suggestion1 = new SuggestedFix(
21 'Replace me!',
22 location1,
23 'With this!'
24 );
25
26 const suggestion2 = new SuggestedFix(
27 'Replace me!',
28 location1,
29 'With this!'
30 );
31
32 assert(suggestion1.isEqual(suggestion2));
33 });
34
35 it('should treat instances with different titles as inequal', () => {
36 const suggestion1 = new SuggestedFix(
37 'Replace me!',
38 location1,
39 'With this!'
40 );
41
42 const suggestion2 = new SuggestedFix(
43 'Not the same title!',
44 location1,
45 'With this!'
46 );
47
48 assert(!suggestion1.isEqual(suggestion2));
49 });
50
51 it('should treat instances with different replacements as inequal', () => {
52 const suggestion1 = new SuggestedFix(
53 'Replace me!',
54 location1,
55 'With this!'
56 );
57
58 const suggestion2 = new SuggestedFix(
59 'Replace me!',
60 location1,
61 'With something else!'
62 );
63
64 assert(!suggestion1.isEqual(suggestion2));
65 });
66
67 it('should treat instances with different locations as inequal', () => {
68 const suggestion1 = new SuggestedFix(
69 'Replace me!',
70 location1,
71 'With this!'
72 );
73
74 const suggestion2 = new SuggestedFix(
75 'Replace me!',
76 location2,
77 'With this!'
78 );
79
80 assert(!suggestion1.isEqual(suggestion2));
81 });
82
83 it('should treat instances with different applicability as inequal', () => {
84 const suggestion1 = new SuggestedFix(
85 'Replace me!',
86 location1,
87 'With this!',
88 SuggestionApplicability.MachineApplicable
89 );
90
91 const suggestion2 = new SuggestedFix(
92 'Replace me!',
93 location2,
94 'With this!',
95 SuggestionApplicability.HasPlaceholders
96 );
97
98 assert(!suggestion1.isEqual(suggestion2));
99 });
100 });
101
102 describe('toCodeAction', () => {
103 it('should map a simple suggestion', () => {
104 const suggestion = new SuggestedFix(
105 'Replace me!',
106 location1,
107 'With this!'
108 );
109
110 const codeAction = suggestion.toCodeAction();
111 assert.strictEqual(codeAction.kind, vscode.CodeActionKind.QuickFix);
112 assert.strictEqual(codeAction.title, 'Replace me!');
113 assert.strictEqual(codeAction.isPreferred, false);
114
115 const edit = codeAction.edit;
116 if (!edit) {
117 return assert.fail('Code Action edit unexpectedly missing');
118 }
119
120 const editEntries = edit.entries();
121 assert.strictEqual(editEntries.length, 1);
122
123 const [[editUri, textEdits]] = editEntries;
124 assert.strictEqual(editUri.toString(), location1.uri.toString());
125
126 assert.strictEqual(textEdits.length, 1);
127 const [textEdit] = textEdits;
128
129 assert(textEdit.range.isEqual(location1.range));
130 assert.strictEqual(textEdit.newText, 'With this!');
131 });
132 });
133});
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 @@
1import * as assert from 'assert';
2import * as vscode from 'vscode';
3
4import SuggestedFix from '../../../utils/diagnostics/SuggestedFix';
5import SuggestedFixCollection from '../../../utils/diagnostics/SuggestedFixCollection';
6
7const uri1 = vscode.Uri.file('/file/1');
8const uri2 = vscode.Uri.file('/file/2');
9
10const mockDocument1 = ({
11 uri: uri1
12} as unknown) as vscode.TextDocument;
13
14const mockDocument2 = ({
15 uri: uri2
16} as unknown) as vscode.TextDocument;
17
18const range1 = new vscode.Range(
19 new vscode.Position(1, 2),
20 new vscode.Position(3, 4)
21);
22const range2 = new vscode.Range(
23 new vscode.Position(5, 6),
24 new vscode.Position(7, 8)
25);
26
27const diagnostic1 = new vscode.Diagnostic(range1, 'First diagnostic');
28const diagnostic2 = new vscode.Diagnostic(range2, 'Second diagnostic');
29
30// This is a mutable object so return a fresh instance every time
31function suggestion1(): SuggestedFix {
32 return new SuggestedFix(
33 'Replace me!',
34 new vscode.Location(uri1, range1),
35 'With this!'
36 );
37}
38
39describe('SuggestedFixCollection', () => {
40 it('should add a suggestion then return it as a code action', () => {
41 const suggestedFixes = new SuggestedFixCollection();
42 suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
43
44 // Specify the document and range that exactly matches
45 const codeActions = suggestedFixes.provideCodeActions(
46 mockDocument1,
47 range1
48 );
49
50 assert.strictEqual(codeActions.length, 1);
51 const [codeAction] = codeActions;
52 assert.strictEqual(codeAction.title, suggestion1().title);
53
54 const { diagnostics } = codeAction;
55 if (!diagnostics) {
56 return assert.fail('Diagnostics unexpectedly missing');
57 }
58
59 assert.strictEqual(diagnostics.length, 1);
60 assert.strictEqual(diagnostics[0], diagnostic1);
61 });
62
63 it('should not return code actions for different ranges', () => {
64 const suggestedFixes = new SuggestedFixCollection();
65 suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
66
67 const codeActions = suggestedFixes.provideCodeActions(
68 mockDocument1,
69 range2
70 );
71
72 assert(!codeActions || codeActions.length === 0);
73 });
74
75 it('should not return code actions for different documents', () => {
76 const suggestedFixes = new SuggestedFixCollection();
77 suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
78
79 const codeActions = suggestedFixes.provideCodeActions(
80 mockDocument2,
81 range1
82 );
83
84 assert(!codeActions || codeActions.length === 0);
85 });
86
87 it('should not return code actions that have been cleared', () => {
88 const suggestedFixes = new SuggestedFixCollection();
89 suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
90 suggestedFixes.clear();
91
92 const codeActions = suggestedFixes.provideCodeActions(
93 mockDocument1,
94 range1
95 );
96
97 assert(!codeActions || codeActions.length === 0);
98 });
99
100 it('should merge identical suggestions together', () => {
101 const suggestedFixes = new SuggestedFixCollection();
102
103 // Add the same suggestion for two diagnostics
104 suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1);
105 suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic2);
106
107 const codeActions = suggestedFixes.provideCodeActions(
108 mockDocument1,
109 range1
110 );
111
112 assert.strictEqual(codeActions.length, 1);
113 const [codeAction] = codeActions;
114 const { diagnostics } = codeAction;
115
116 if (!diagnostics) {
117 return assert.fail('Diagnostics unexpectedly missing');
118 }
119
120 // We should be associated with both diagnostics
121 assert.strictEqual(diagnostics.length, 2);
122 assert.strictEqual(diagnostics[0], diagnostic1);
123 assert.strictEqual(diagnostics[1], diagnostic2);
124 });
125});
diff --git a/editors/code/src/test/rust_diagnostics.test.ts b/editors/code/src/test/utils/diagnotics/rust.test.ts
index f27c58fe2..b555a4819 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';
5import { 5import {
6 MappedRustDiagnostic, 6 MappedRustDiagnostic,
7 mapRustDiagnosticToVsCode, 7 mapRustDiagnosticToVsCode,
8 RustDiagnostic 8 RustDiagnostic,
9} from '../utils/rust_diagnostics'; 9 SuggestionApplicability
10} from '../../../utils/diagnostics/rust';
10 11
11function loadDiagnosticFixture(name: string): RustDiagnostic { 12function loadDiagnosticFixture(name: string): RustDiagnostic {
12 const jsonText = fs 13 const jsonText = fs
13 .readFileSync( 14 .readFileSync(
14 // We're actually in our JavaScript output directory, climb out 15 // We're actually in our JavaScript output directory, climb out
15 `${__dirname}/../../src/test/fixtures/rust-diagnostics/${name}.json` 16 `${__dirname}/../../../../src/test/fixtures/rust-diagnostics/${name}.json`
16 ) 17 )
17 .toString(); 18 .toString();
18 19
@@ -31,7 +32,9 @@ function mapFixtureToVsCode(name: string): MappedRustDiagnostic {
31 32
32describe('mapRustDiagnosticToVsCode', () => { 33describe('mapRustDiagnosticToVsCode', () => {
33 it('should map an incompatible type for trait error', () => { 34 it('should map an incompatible type for trait error', () => {
34 const { diagnostic, codeActions } = mapFixtureToVsCode('error/E0053'); 35 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
36 'error/E0053'
37 );
35 38
36 assert.strictEqual( 39 assert.strictEqual(
37 diagnostic.severity, 40 diagnostic.severity,
@@ -52,12 +55,12 @@ describe('mapRustDiagnosticToVsCode', () => {
52 // No related information 55 // No related information
53 assert.deepStrictEqual(diagnostic.relatedInformation, []); 56 assert.deepStrictEqual(diagnostic.relatedInformation, []);
54 57
55 // There are no code actions available 58 // There are no suggested fixes
56 assert.strictEqual(codeActions.length, 0); 59 assert.strictEqual(suggestedFixes.length, 0);
57 }); 60 });
58 61
59 it('should map an unused variable warning', () => { 62 it('should map an unused variable warning', () => {
60 const { diagnostic, codeActions } = mapFixtureToVsCode( 63 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
61 'warning/unused_variables' 64 'warning/unused_variables'
62 ); 65 );
63 66
@@ -81,18 +84,23 @@ describe('mapRustDiagnosticToVsCode', () => {
81 // No related information 84 // No related information
82 assert.deepStrictEqual(diagnostic.relatedInformation, []); 85 assert.deepStrictEqual(diagnostic.relatedInformation, []);
83 86
84 // One code action available to prefix the variable 87 // One suggested fix available to prefix the variable
85 assert.strictEqual(codeActions.length, 1); 88 assert.strictEqual(suggestedFixes.length, 1);
86 const [codeAction] = codeActions; 89 const [suggestedFix] = suggestedFixes;
87 assert.strictEqual( 90 assert.strictEqual(
88 codeAction.title, 91 suggestedFix.title,
89 'consider prefixing with an underscore: `_foo`' 92 'consider prefixing with an underscore: `_foo`'
90 ); 93 );
91 assert(codeAction.isPreferred); 94 assert.strictEqual(
95 suggestedFix.applicability,
96 SuggestionApplicability.MachineApplicable
97 );
92 }); 98 });
93 99
94 it('should map a wrong number of parameters error', () => { 100 it('should map a wrong number of parameters error', () => {
95 const { diagnostic, codeActions } = mapFixtureToVsCode('error/E0061'); 101 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
102 'error/E0061'
103 );
96 104
97 assert.strictEqual( 105 assert.strictEqual(
98 diagnostic.severity, 106 diagnostic.severity,
@@ -115,12 +123,12 @@ describe('mapRustDiagnosticToVsCode', () => {
115 const [related] = relatedInformation; 123 const [related] = relatedInformation;
116 assert.strictEqual(related.message, 'defined here'); 124 assert.strictEqual(related.message, 'defined here');
117 125
118 // There are no actions available 126 // There are no suggested fixes
119 assert.strictEqual(codeActions.length, 0); 127 assert.strictEqual(suggestedFixes.length, 0);
120 }); 128 });
121 129
122 it('should map a Clippy copy pass by ref warning', () => { 130 it('should map a Clippy copy pass by ref warning', () => {
123 const { diagnostic, codeActions } = mapFixtureToVsCode( 131 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
124 'clippy/trivially_copy_pass_by_ref' 132 'clippy/trivially_copy_pass_by_ref'
125 ); 133 );
126 134
@@ -149,14 +157,17 @@ describe('mapRustDiagnosticToVsCode', () => {
149 const [related] = relatedInformation; 157 const [related] = relatedInformation;
150 assert.strictEqual(related.message, 'lint level defined here'); 158 assert.strictEqual(related.message, 'lint level defined here');
151 159
152 // One code action available to pass by value 160 // One suggested fix to pass by value
153 assert.strictEqual(codeActions.length, 1); 161 assert.strictEqual(suggestedFixes.length, 1);
154 const [codeAction] = codeActions; 162 const [suggestedFix] = suggestedFixes;
155 assert.strictEqual( 163 assert.strictEqual(
156 codeAction.title, 164 suggestedFix.title,
157 'consider passing by value instead: `self`' 165 'consider passing by value instead: `self`'
158 ); 166 );
159 // Clippy does not mark this as machine applicable 167 // Clippy does not mark this with any applicability
160 assert.strictEqual(codeAction.isPreferred, false); 168 assert.strictEqual(
169 suggestedFix.applicability,
170 SuggestionApplicability.Unspecified
171 );
161 }); 172 });
162}); 173});
diff --git a/editors/code/src/test/vscode_diagnostics.test.ts b/editors/code/src/test/utils/diagnotics/vscode.test.ts
index 9c5d812fa..542dec1f5 100644
--- a/editors/code/src/test/vscode_diagnostics.test.ts
+++ b/editors/code/src/test/utils/diagnotics/vscode.test.ts
@@ -1,12 +1,7 @@
1import * as assert from 'assert'; 1import * as assert from 'assert';
2import * as vscode from 'vscode'; 2import * as vscode from 'vscode';
3 3
4import { 4import { areDiagnosticsEqual } from '../../../utils/diagnostics/vscode';
5 areCodeActionsEqual,
6 areDiagnosticsEqual
7} from '../utils/vscode_diagnostics';
8
9const uri = vscode.Uri.file('/file/1');
10 5
11const range1 = new vscode.Range( 6const range1 = new vscode.Range(
12 new vscode.Position(1, 2), 7 new vscode.Position(1, 2),
@@ -101,82 +96,3 @@ describe('areDiagnosticsEqual', () => {
101 assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); 96 assert(!areDiagnosticsEqual(diagnostic1, diagnostic2));
102 }); 97 });
103}); 98});
104
105describe('areCodeActionsEqual', () => {
106 it('should treat identical actions as equal', () => {
107 const codeAction1 = new vscode.CodeAction(
108 'Fix me!',
109 vscode.CodeActionKind.QuickFix
110 );
111
112 const codeAction2 = new vscode.CodeAction(
113 'Fix me!',
114 vscode.CodeActionKind.QuickFix
115 );
116
117 const edit = new vscode.WorkspaceEdit();
118 edit.replace(uri, range1, 'Replace with this');
119 codeAction1.edit = edit;
120 codeAction2.edit = edit;
121
122 assert(areCodeActionsEqual(codeAction1, codeAction2));
123 });
124
125 it('should treat actions with different types as inequal', () => {
126 const codeAction1 = new vscode.CodeAction(
127 'Fix me!',
128 vscode.CodeActionKind.Refactor
129 );
130
131 const codeAction2 = new vscode.CodeAction(
132 'Fix me!',
133 vscode.CodeActionKind.QuickFix
134 );
135
136 const edit = new vscode.WorkspaceEdit();
137 edit.replace(uri, range1, 'Replace with this');
138 codeAction1.edit = edit;
139 codeAction2.edit = edit;
140
141 assert(!areCodeActionsEqual(codeAction1, codeAction2));
142 });
143
144 it('should treat actions with different titles as inequal', () => {
145 const codeAction1 = new vscode.CodeAction(
146 'Fix me!',
147 vscode.CodeActionKind.Refactor
148 );
149
150 const codeAction2 = new vscode.CodeAction(
151 'Do something different!',
152 vscode.CodeActionKind.Refactor
153 );
154
155 const edit = new vscode.WorkspaceEdit();
156 edit.replace(uri, range1, 'Replace with this');
157 codeAction1.edit = edit;
158 codeAction2.edit = edit;
159
160 assert(!areCodeActionsEqual(codeAction1, codeAction2));
161 });
162
163 it('should treat actions with different edits as inequal', () => {
164 const codeAction1 = new vscode.CodeAction(
165 'Fix me!',
166 vscode.CodeActionKind.Refactor
167 );
168 const edit1 = new vscode.WorkspaceEdit();
169 edit1.replace(uri, range1, 'Replace with this');
170 codeAction1.edit = edit1;
171
172 const codeAction2 = new vscode.CodeAction(
173 'Fix me!',
174 vscode.CodeActionKind.Refactor
175 );
176 const edit2 = new vscode.WorkspaceEdit();
177 edit2.replace(uri, range1, 'Replace with this other thing');
178 codeAction2.edit = edit2;
179
180 assert(!areCodeActionsEqual(codeAction1, codeAction2));
181 });
182});
diff --git a/editors/code/src/utils/diagnostics/SuggestedFix.ts b/editors/code/src/utils/diagnostics/SuggestedFix.ts
new file mode 100644
index 000000000..b1be2a225
--- /dev/null
+++ b/editors/code/src/utils/diagnostics/SuggestedFix.ts
@@ -0,0 +1,67 @@
1import * as vscode from 'vscode';
2
3import { SuggestionApplicability } from './rust';
4
5/**
6 * Model object for text replacements suggested by the Rust compiler
7 *
8 * This is an intermediate form between the raw `rustc` JSON and a
9 * `vscode.CodeAction`. It's optimised for the use-cases of
10 * `SuggestedFixCollection`.
11 */
12export default class SuggestedFix {
13 public readonly title: string;
14 public readonly location: vscode.Location;
15 public readonly replacement: string;
16 public readonly applicability: SuggestionApplicability;
17
18 /**
19 * Diagnostics this suggested fix could resolve
20 */
21 public diagnostics: vscode.Diagnostic[];
22
23 constructor(
24 title: string,
25 location: vscode.Location,
26 replacement: string,
27 applicability: SuggestionApplicability = SuggestionApplicability.Unspecified
28 ) {
29 this.title = title;
30 this.location = location;
31 this.replacement = replacement;
32 this.applicability = applicability;
33 this.diagnostics = [];
34 }
35
36 /**
37 * Determines if this suggested fix is equivalent to another instance
38 */
39 public isEqual(other: SuggestedFix): boolean {
40 return (
41 this.title === other.title &&
42 this.location.range.isEqual(other.location.range) &&
43 this.replacement === other.replacement &&
44 this.applicability === other.applicability
45 );
46 }
47
48 /**
49 * Converts this suggested fix to a VS Code Quick Fix code action
50 */
51 public toCodeAction(): vscode.CodeAction {
52 const codeAction = new vscode.CodeAction(
53 this.title,
54 vscode.CodeActionKind.QuickFix
55 );
56
57 const edit = new vscode.WorkspaceEdit();
58 edit.replace(this.location.uri, this.location.range, this.replacement);
59 codeAction.edit = edit;
60
61 codeAction.isPreferred =
62 this.applicability === SuggestionApplicability.MachineApplicable;
63
64 codeAction.diagnostics = [...this.diagnostics];
65 return codeAction;
66 }
67}
diff --git a/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts
new file mode 100644
index 000000000..3b0bf7468
--- /dev/null
+++ b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts
@@ -0,0 +1,74 @@
1import * as vscode from 'vscode';
2import SuggestedFix from './SuggestedFix';
3
4/**
5 * Collection of suggested fixes across multiple documents
6 *
7 * This stores `SuggestedFix` model objects and returns them via the
8 * `vscode.CodeActionProvider` interface.
9 */
10export default class SuggestedFixCollection
11 implements vscode.CodeActionProvider {
12 public static PROVIDED_CODE_ACTION_KINDS = [vscode.CodeActionKind.QuickFix];
13
14 private suggestedFixes: Map<string, SuggestedFix[]>;
15
16 constructor() {
17 this.suggestedFixes = new Map();
18 }
19
20 /**
21 * Clears all suggested fixes across all documents
22 */
23 public clear(): void {
24 this.suggestedFixes = new Map();
25 }
26
27 /**
28 * Adds a suggested fix for the given diagnostic
29 *
30 * Some suggested fixes will appear in multiple diagnostics. For example,
31 * forgetting a `mut` on a variable will suggest changing the delaration on
32 * every mutable usage site. If the suggested fix has already been added
33 * this method will instead associate the existing fix with the new
34 * diagnostic.
35 */
36 public addSuggestedFixForDiagnostic(
37 suggestedFix: SuggestedFix,
38 diagnostic: vscode.Diagnostic
39 ): void {
40 const fileUriString = suggestedFix.location.uri.toString();
41 const fileSuggestions = this.suggestedFixes.get(fileUriString) || [];
42
43 const existingSuggestion = fileSuggestions.find(s =>
44 s.isEqual(suggestedFix)
45 );
46
47 if (existingSuggestion) {
48 // The existing suggestion also applies to this new diagnostic
49 existingSuggestion.diagnostics.push(diagnostic);
50 } else {
51 // We haven't seen this suggestion before
52 suggestedFix.diagnostics.push(diagnostic);
53 fileSuggestions.push(suggestedFix);
54 }
55
56 this.suggestedFixes.set(fileUriString, fileSuggestions);
57 }
58
59 /**
60 * Filters suggested fixes by their document and range and converts them to
61 * code actions
62 */
63 public provideCodeActions(
64 document: vscode.TextDocument,
65 range: vscode.Range
66 ): vscode.CodeAction[] {
67 const documentUriString = document.uri.toString();
68
69 const suggestedFixes = this.suggestedFixes.get(documentUriString);
70 return (suggestedFixes || [])
71 .filter(({ location }) => location.range.intersection(range))
72 .map(suggestedEdit => suggestedEdit.toCodeAction());
73 }
74}
diff --git a/editors/code/src/utils/rust_diagnostics.ts b/editors/code/src/utils/diagnostics/rust.ts
index 3c524cb37..d16576eb1 100644
--- a/editors/code/src/utils/rust_diagnostics.ts
+++ b/editors/code/src/utils/diagnostics/rust.ts
@@ -1,6 +1,15 @@
1import * as path from 'path'; 1import * as path from 'path';
2import * as vscode from 'vscode'; 2import * as vscode from 'vscode';
3 3
4import SuggestedFix from './SuggestedFix';
5
6export enum SuggestionApplicability {
7 MachineApplicable = 'MachineApplicable',
8 HasPlaceholders = 'HasPlaceholders',
9 MaybeIncorrect = 'MaybeIncorrect',
10 Unspecified = 'Unspecified'
11}
12
4// Reference: 13// Reference:
5// https://github.com/rust-lang/rust/blob/master/src/libsyntax/json.rs 14// https://github.com/rust-lang/rust/blob/master/src/libsyntax/json.rs
6export interface RustDiagnosticSpan { 15export interface RustDiagnosticSpan {
@@ -12,11 +21,7 @@ export interface RustDiagnosticSpan {
12 file_name: string; 21 file_name: string;
13 label?: string; 22 label?: string;
14 suggested_replacement?: string; 23 suggested_replacement?: string;
15 suggestion_applicability?: 24 suggestion_applicability?: SuggestionApplicability;
16 | 'MachineApplicable'
17 | 'HasPlaceholders'
18 | 'MaybeIncorrect'
19 | 'Unspecified';
20} 25}
21 26
22export interface RustDiagnostic { 27export interface RustDiagnostic {
@@ -33,12 +38,12 @@ export interface RustDiagnostic {
33export interface MappedRustDiagnostic { 38export interface MappedRustDiagnostic {
34 location: vscode.Location; 39 location: vscode.Location;
35 diagnostic: vscode.Diagnostic; 40 diagnostic: vscode.Diagnostic;
36 codeActions: vscode.CodeAction[]; 41 suggestedFixes: SuggestedFix[];
37} 42}
38 43
39interface MappedRustChildDiagnostic { 44interface MappedRustChildDiagnostic {
40 related?: vscode.DiagnosticRelatedInformation; 45 related?: vscode.DiagnosticRelatedInformation;
41 codeAction?: vscode.CodeAction; 46 suggestedFix?: SuggestedFix;
42 messageLine?: string; 47 messageLine?: string;
43} 48}
44 49
@@ -130,24 +135,19 @@ function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic {
130 135
131 // We need to distinguish `null` from an empty string 136 // We need to distinguish `null` from an empty string
132 if (span && typeof span.suggested_replacement === 'string') { 137 if (span && typeof span.suggested_replacement === 'string') {
133 const edit = new vscode.WorkspaceEdit(); 138 // Include our replacement in the title unless it's empty
134 edit.replace(location.uri, location.range, span.suggested_replacement);
135
136 // Include our replacement in the label unless it's empty
137 const title = span.suggested_replacement 139 const title = span.suggested_replacement
138 ? `${rd.message}: \`${span.suggested_replacement}\`` 140 ? `${rd.message}: \`${span.suggested_replacement}\``
139 : rd.message; 141 : rd.message;
140 142
141 const codeAction = new vscode.CodeAction( 143 return {
142 title, 144 suggestedFix: new SuggestedFix(
143 vscode.CodeActionKind.QuickFix 145 title,
144 ); 146 location,
145 147 span.suggested_replacement,
146 codeAction.edit = edit; 148 span.suggestion_applicability
147 codeAction.isPreferred = 149 )
148 span.suggestion_applicability === 'MachineApplicable'; 150 };
149
150 return { codeAction };
151 } else { 151 } else {
152 const related = new vscode.DiagnosticRelatedInformation( 152 const related = new vscode.DiagnosticRelatedInformation(
153 location, 153 location,
@@ -165,7 +165,7 @@ function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic {
165 * 165 *
166 * 1. Creating a `vscode.Diagnostic` with the root message and primary span. 166 * 1. Creating a `vscode.Diagnostic` with the root message and primary span.
167 * 2. Adding any labelled secondary spans to `relatedInformation` 167 * 2. Adding any labelled secondary spans to `relatedInformation`
168 * 3. Categorising child diagnostics as either Quick Fix actions, 168 * 3. Categorising child diagnostics as either `SuggestedFix`es,
169 * `relatedInformation` or additional message lines. 169 * `relatedInformation` or additional message lines.
170 * 170 *
171 * If the diagnostic has no primary span this will return `undefined` 171 * If the diagnostic has no primary span this will return `undefined`
@@ -173,8 +173,6 @@ function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic {
173export function mapRustDiagnosticToVsCode( 173export function mapRustDiagnosticToVsCode(
174 rd: RustDiagnostic 174 rd: RustDiagnostic
175): MappedRustDiagnostic | undefined { 175): MappedRustDiagnostic | undefined {
176 const codeActions = [];
177
178 const primarySpan = rd.spans.find(s => s.is_primary); 176 const primarySpan = rd.spans.find(s => s.is_primary);
179 if (!primarySpan) { 177 if (!primarySpan) {
180 return; 178 return;
@@ -208,16 +206,17 @@ export function mapRustDiagnosticToVsCode(
208 } 206 }
209 } 207 }
210 208
209 const suggestedFixes = [];
211 for (const child of rd.children) { 210 for (const child of rd.children) {
212 const { related, codeAction, messageLine } = mapRustChildDiagnostic( 211 const { related, suggestedFix, messageLine } = mapRustChildDiagnostic(
213 child 212 child
214 ); 213 );
215 214
216 if (related) { 215 if (related) {
217 vd.relatedInformation.push(related); 216 vd.relatedInformation.push(related);
218 } 217 }
219 if (codeAction) { 218 if (suggestedFix) {
220 codeActions.push(codeAction); 219 suggestedFixes.push(suggestedFix);
221 } 220 }
222 if (messageLine) { 221 if (messageLine) {
223 vd.message += `\n${messageLine}`; 222 vd.message += `\n${messageLine}`;
@@ -231,6 +230,6 @@ export function mapRustDiagnosticToVsCode(
231 return { 230 return {
232 location, 231 location,
233 diagnostic: vd, 232 diagnostic: vd,
234 codeActions 233 suggestedFixes
235 }; 234 };
236} 235}
diff --git a/editors/code/src/utils/diagnostics/vscode.ts b/editors/code/src/utils/diagnostics/vscode.ts
new file mode 100644
index 000000000..d8b85b720
--- /dev/null
+++ b/editors/code/src/utils/diagnostics/vscode.ts
@@ -0,0 +1,14 @@
1import * as vscode from 'vscode';
2
3/** Compares two `vscode.Diagnostic`s for equality */
4export function areDiagnosticsEqual(
5 left: vscode.Diagnostic,
6 right: vscode.Diagnostic
7): boolean {
8 return (
9 left.source === right.source &&
10 left.severity === right.severity &&
11 left.range.isEqual(right.range) &&
12 left.message === right.message
13 );
14}
diff --git a/editors/code/src/utils/vscode_diagnostics.ts b/editors/code/src/utils/vscode_diagnostics.ts
deleted file mode 100644
index 9d763c8d6..000000000
--- a/editors/code/src/utils/vscode_diagnostics.ts
+++ /dev/null
@@ -1,73 +0,0 @@
1import * as vscode from 'vscode';
2
3/** Compares two `vscode.Diagnostic`s for equality */
4export function areDiagnosticsEqual(
5 left: vscode.Diagnostic,
6 right: vscode.Diagnostic
7): boolean {
8 return (
9 left.source === right.source &&
10 left.severity === right.severity &&
11 left.range.isEqual(right.range) &&
12 left.message === right.message
13 );
14}
15
16/** Compares two `vscode.TextEdit`s for equality */
17function areTextEditsEqual(
18 left: vscode.TextEdit,
19 right: vscode.TextEdit
20): boolean {
21 if (!left.range.isEqual(right.range)) {
22 return false;
23 }
24
25 if (left.newText !== right.newText) {
26 return false;
27 }
28
29 return true;
30}
31
32/** Compares two `vscode.CodeAction`s for equality */
33export function areCodeActionsEqual(
34 left: vscode.CodeAction,
35 right: vscode.CodeAction
36): boolean {
37 if (
38 left.kind !== right.kind ||
39 left.title !== right.title ||
40 !left.edit ||
41 !right.edit
42 ) {
43 return false;
44 }
45
46 const leftEditEntries = left.edit.entries();
47 const rightEditEntries = right.edit.entries();
48
49 if (leftEditEntries.length !== rightEditEntries.length) {
50 return false;
51 }
52
53 for (let i = 0; i < leftEditEntries.length; i++) {
54 const [leftUri, leftEdits] = leftEditEntries[i];
55 const [rightUri, rightEdits] = rightEditEntries[i];
56
57 if (leftUri.toString() !== rightUri.toString()) {
58 return false;
59 }
60
61 if (leftEdits.length !== rightEdits.length) {
62 return false;
63 }
64
65 for (let j = 0; j < leftEdits.length; j++) {
66 if (!areTextEditsEqual(leftEdits[j], rightEdits[j])) {
67 return false;
68 }
69 }
70 }
71
72 return true;
73}