aboutsummaryrefslogtreecommitdiff
path: root/editors/code/src/test/utils/diagnotics
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/utils/diagnotics
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/utils/diagnotics')
-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.ts173
-rw-r--r--editors/code/src/test/utils/diagnotics/vscode.test.ts98
4 files changed, 529 insertions, 0 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/utils/diagnotics/rust.test.ts b/editors/code/src/test/utils/diagnotics/rust.test.ts
new file mode 100644
index 000000000..b555a4819
--- /dev/null
+++ b/editors/code/src/test/utils/diagnotics/rust.test.ts
@@ -0,0 +1,173 @@
1import * as assert from 'assert';
2import * as fs from 'fs';
3import * as vscode from 'vscode';
4
5import {
6 MappedRustDiagnostic,
7 mapRustDiagnosticToVsCode,
8 RustDiagnostic,
9 SuggestionApplicability
10} from '../../../utils/diagnostics/rust';
11
12function loadDiagnosticFixture(name: string): RustDiagnostic {
13 const jsonText = fs
14 .readFileSync(
15 // We're actually in our JavaScript output directory, climb out
16 `${__dirname}/../../../../src/test/fixtures/rust-diagnostics/${name}.json`
17 )
18 .toString();
19
20 return JSON.parse(jsonText);
21}
22
23function mapFixtureToVsCode(name: string): MappedRustDiagnostic {
24 const rd = loadDiagnosticFixture(name);
25 const mapResult = mapRustDiagnosticToVsCode(rd);
26
27 if (!mapResult) {
28 return assert.fail('Mapping unexpectedly failed');
29 }
30 return mapResult;
31}
32
33describe('mapRustDiagnosticToVsCode', () => {
34 it('should map an incompatible type for trait error', () => {
35 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
36 'error/E0053'
37 );
38
39 assert.strictEqual(
40 diagnostic.severity,
41 vscode.DiagnosticSeverity.Error
42 );
43 assert.strictEqual(diagnostic.source, 'rustc');
44 assert.strictEqual(
45 diagnostic.message,
46 [
47 `method \`next\` has an incompatible type for trait`,
48 `expected type \`fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref<M>>\``,
49 ` found type \`fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref<M>>\``
50 ].join('\n')
51 );
52 assert.strictEqual(diagnostic.code, 'E0053');
53 assert.strictEqual(diagnostic.tags, undefined);
54
55 // No related information
56 assert.deepStrictEqual(diagnostic.relatedInformation, []);
57
58 // There are no suggested fixes
59 assert.strictEqual(suggestedFixes.length, 0);
60 });
61
62 it('should map an unused variable warning', () => {
63 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
64 'warning/unused_variables'
65 );
66
67 assert.strictEqual(
68 diagnostic.severity,
69 vscode.DiagnosticSeverity.Warning
70 );
71 assert.strictEqual(
72 diagnostic.message,
73 [
74 'unused variable: `foo`',
75 '#[warn(unused_variables)] on by default'
76 ].join('\n')
77 );
78 assert.strictEqual(diagnostic.code, 'unused_variables');
79 assert.strictEqual(diagnostic.source, 'rustc');
80 assert.deepStrictEqual(diagnostic.tags, [
81 vscode.DiagnosticTag.Unnecessary
82 ]);
83
84 // No related information
85 assert.deepStrictEqual(diagnostic.relatedInformation, []);
86
87 // One suggested fix available to prefix the variable
88 assert.strictEqual(suggestedFixes.length, 1);
89 const [suggestedFix] = suggestedFixes;
90 assert.strictEqual(
91 suggestedFix.title,
92 'consider prefixing with an underscore: `_foo`'
93 );
94 assert.strictEqual(
95 suggestedFix.applicability,
96 SuggestionApplicability.MachineApplicable
97 );
98 });
99
100 it('should map a wrong number of parameters error', () => {
101 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
102 'error/E0061'
103 );
104
105 assert.strictEqual(
106 diagnostic.severity,
107 vscode.DiagnosticSeverity.Error
108 );
109 assert.strictEqual(
110 diagnostic.message,
111 'this function takes 2 parameters but 3 parameters were supplied'
112 );
113 assert.strictEqual(diagnostic.code, 'E0061');
114 assert.strictEqual(diagnostic.source, 'rustc');
115 assert.strictEqual(diagnostic.tags, undefined);
116
117 // One related information for the original definition
118 const relatedInformation = diagnostic.relatedInformation;
119 if (!relatedInformation) {
120 return assert.fail('Related information unexpectedly undefined');
121 }
122 assert.strictEqual(relatedInformation.length, 1);
123 const [related] = relatedInformation;
124 assert.strictEqual(related.message, 'defined here');
125
126 // There are no suggested fixes
127 assert.strictEqual(suggestedFixes.length, 0);
128 });
129
130 it('should map a Clippy copy pass by ref warning', () => {
131 const { diagnostic, suggestedFixes } = mapFixtureToVsCode(
132 'clippy/trivially_copy_pass_by_ref'
133 );
134
135 assert.strictEqual(
136 diagnostic.severity,
137 vscode.DiagnosticSeverity.Warning
138 );
139 assert.strictEqual(diagnostic.source, 'clippy');
140 assert.strictEqual(
141 diagnostic.message,
142 [
143 'this argument is passed by reference, but would be more efficient if passed by value',
144 '#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]',
145 'for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref'
146 ].join('\n')
147 );
148 assert.strictEqual(diagnostic.code, 'trivially_copy_pass_by_ref');
149 assert.strictEqual(diagnostic.tags, undefined);
150
151 // One related information for the lint definition
152 const relatedInformation = diagnostic.relatedInformation;
153 if (!relatedInformation) {
154 return assert.fail('Related information unexpectedly undefined');
155 }
156 assert.strictEqual(relatedInformation.length, 1);
157 const [related] = relatedInformation;
158 assert.strictEqual(related.message, 'lint level defined here');
159
160 // One suggested fix to pass by value
161 assert.strictEqual(suggestedFixes.length, 1);
162 const [suggestedFix] = suggestedFixes;
163 assert.strictEqual(
164 suggestedFix.title,
165 'consider passing by value instead: `self`'
166 );
167 // Clippy does not mark this with any applicability
168 assert.strictEqual(
169 suggestedFix.applicability,
170 SuggestionApplicability.Unspecified
171 );
172 });
173});
diff --git a/editors/code/src/test/utils/diagnotics/vscode.test.ts b/editors/code/src/test/utils/diagnotics/vscode.test.ts
new file mode 100644
index 000000000..542dec1f5
--- /dev/null
+++ b/editors/code/src/test/utils/diagnotics/vscode.test.ts
@@ -0,0 +1,98 @@
1import * as assert from 'assert';
2import * as vscode from 'vscode';
3
4import { areDiagnosticsEqual } from '../../../utils/diagnostics/vscode';
5
6const range1 = new vscode.Range(
7 new vscode.Position(1, 2),
8 new vscode.Position(3, 4)
9);
10
11const range2 = new vscode.Range(
12 new vscode.Position(5, 6),
13 new vscode.Position(7, 8)
14);
15
16describe('areDiagnosticsEqual', () => {
17 it('should treat identical diagnostics as equal', () => {
18 const diagnostic1 = new vscode.Diagnostic(
19 range1,
20 'Hello, world!',
21 vscode.DiagnosticSeverity.Error
22 );
23
24 const diagnostic2 = new vscode.Diagnostic(
25 range1,
26 'Hello, world!',
27 vscode.DiagnosticSeverity.Error
28 );
29
30 assert(areDiagnosticsEqual(diagnostic1, diagnostic2));
31 });
32
33 it('should treat diagnostics with different sources as inequal', () => {
34 const diagnostic1 = new vscode.Diagnostic(
35 range1,
36 'Hello, world!',
37 vscode.DiagnosticSeverity.Error
38 );
39 diagnostic1.source = 'rustc';
40
41 const diagnostic2 = new vscode.Diagnostic(
42 range1,
43 'Hello, world!',
44 vscode.DiagnosticSeverity.Error
45 );
46 diagnostic2.source = 'clippy';
47
48 assert(!areDiagnosticsEqual(diagnostic1, diagnostic2));
49 });
50
51 it('should treat diagnostics with different ranges as inequal', () => {
52 const diagnostic1 = new vscode.Diagnostic(
53 range1,
54 'Hello, world!',
55 vscode.DiagnosticSeverity.Error
56 );
57
58 const diagnostic2 = new vscode.Diagnostic(
59 range2,
60 'Hello, world!',
61 vscode.DiagnosticSeverity.Error
62 );
63
64 assert(!areDiagnosticsEqual(diagnostic1, diagnostic2));
65 });
66
67 it('should treat diagnostics with different messages as inequal', () => {
68 const diagnostic1 = new vscode.Diagnostic(
69 range1,
70 'Hello, world!',
71 vscode.DiagnosticSeverity.Error
72 );
73
74 const diagnostic2 = new vscode.Diagnostic(
75 range1,
76 'Goodbye!, world!',
77 vscode.DiagnosticSeverity.Error
78 );
79
80 assert(!areDiagnosticsEqual(diagnostic1, diagnostic2));
81 });
82
83 it('should treat diagnostics with different severities as inequal', () => {
84 const diagnostic1 = new vscode.Diagnostic(
85 range1,
86 'Hello, world!',
87 vscode.DiagnosticSeverity.Warning
88 );
89
90 const diagnostic2 = new vscode.Diagnostic(
91 range1,
92 'Hello, world!',
93 vscode.DiagnosticSeverity.Error
94 );
95
96 assert(!areDiagnosticsEqual(diagnostic1, diagnostic2));
97 });
98});