aboutsummaryrefslogtreecommitdiff
path: root/editors/code/src/test
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 /editors/code/src/test
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.
Diffstat (limited to 'editors/code/src/test')
-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
4 files changed, 292 insertions, 107 deletions
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});