diff options
author | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-03-25 11:38:46 +0000 |
---|---|---|
committer | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-03-25 11:38:46 +0000 |
commit | c4ead49361e4b8c0586b810399c8e96a468b891c (patch) | |
tree | 0b1ba767e34e3baef938f6b7672f95ce4572ec07 /crates/ra_ide_api | |
parent | 8aedf9603df1bc68eafcd8dcf3c14e5a6a2c8638 (diff) | |
parent | 309716cffe93d065bcad0344b0f332425576c1e5 (diff) |
Merge #1034
1034: HIR diagnostics API r=matklad a=matklad
This PR introduces diagnostics API for HIR, so we can now start issuing errors and warnings! Here are requirements that this solution aims to fulfill:
* structured diagnostics: rather than immediately rendering error to string, we provide a well-typed blob of data with error-description. These data is used by IDE to provide fixes
* open set diagnostics: there's no single enum with all possible diagnostics, which hopefully should result in better modularity
The `Diagnostic` trait describes "a diagnostic", which can be downcast to a specific diagnostic kind. Diagnostics are expressed in terms of macro-expanded syntax tree: they store pointers to syntax nodes. Diagnostics are self-contained: you don't need any context, besides `db`, to fully understand the meaning of a diagnostic.
Because diagnostics are tied to the source, we can't store them in salsa. So subsystems like type-checking produce subsystem-local diagnostic (which is a closed `enum`), which is expressed in therms of subsystem IR. A separate step converts these proto-diagnostics into `Diagnostic`, by merging them with source-maps.
Note that this PR stresses type-system quite a bit: we now type-check every function in open files to compute errors!
Discussion on Zulip: https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Diagnostics.20API
Co-authored-by: Aleksey Kladov <[email protected]>
Diffstat (limited to 'crates/ra_ide_api')
-rw-r--r-- | crates/ra_ide_api/src/diagnostics.rs | 87 | ||||
-rw-r--r-- | crates/ra_ide_api/tests/test/main.rs | 16 | ||||
-rw-r--r-- | crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap | 28 |
3 files changed, 57 insertions, 74 deletions
diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 156f28ca3..5a78e94d8 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs | |||
@@ -1,10 +1,11 @@ | |||
1 | use std::cell::RefCell; | ||
2 | |||
1 | use itertools::Itertools; | 3 | use itertools::Itertools; |
2 | use hir::{Problem, source_binder}; | 4 | use hir::{source_binder, diagnostics::{Diagnostic as _, DiagnosticSink}}; |
3 | use ra_db::SourceDatabase; | 5 | use ra_db::SourceDatabase; |
4 | use ra_syntax::{ | 6 | use ra_syntax::{ |
5 | Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, | 7 | Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, |
6 | ast::{self, AstNode}, | 8 | ast::{self, AstNode}, |
7 | |||
8 | }; | 9 | }; |
9 | use ra_text_edit::{TextEdit, TextEditBuilder}; | 10 | use ra_text_edit::{TextEdit, TextEditBuilder}; |
10 | 11 | ||
@@ -26,11 +27,31 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic> | |||
26 | check_unnecessary_braces_in_use_statement(&mut res, file_id, node); | 27 | check_unnecessary_braces_in_use_statement(&mut res, file_id, node); |
27 | check_struct_shorthand_initialization(&mut res, file_id, node); | 28 | check_struct_shorthand_initialization(&mut res, file_id, node); |
28 | } | 29 | } |
29 | 30 | let res = RefCell::new(res); | |
31 | let mut sink = DiagnosticSink::new(|d| { | ||
32 | res.borrow_mut().push(Diagnostic { | ||
33 | message: d.message(), | ||
34 | range: d.highlight_range(), | ||
35 | severity: Severity::Error, | ||
36 | fix: None, | ||
37 | }) | ||
38 | }) | ||
39 | .on::<hir::diagnostics::UnresolvedModule, _>(|d| { | ||
40 | let source_root = db.file_source_root(d.file().original_file(db)); | ||
41 | let create_file = FileSystemEdit::CreateFile { source_root, path: d.candidate.clone() }; | ||
42 | let fix = SourceChange::file_system_edit("create module", create_file); | ||
43 | res.borrow_mut().push(Diagnostic { | ||
44 | range: d.highlight_range(), | ||
45 | message: d.message(), | ||
46 | severity: Severity::Error, | ||
47 | fix: Some(fix), | ||
48 | }) | ||
49 | }); | ||
30 | if let Some(m) = source_binder::module_from_file_id(db, file_id) { | 50 | if let Some(m) = source_binder::module_from_file_id(db, file_id) { |
31 | check_module(&mut res, db, file_id, m); | 51 | m.diagnostics(db, &mut sink); |
32 | }; | 52 | }; |
33 | res | 53 | drop(sink); |
54 | res.into_inner() | ||
34 | } | 55 | } |
35 | 56 | ||
36 | fn syntax_errors(acc: &mut Vec<Diagnostic>, source_file: &SourceFile) { | 57 | fn syntax_errors(acc: &mut Vec<Diagnostic>, source_file: &SourceFile) { |
@@ -128,34 +149,12 @@ fn check_struct_shorthand_initialization( | |||
128 | Some(()) | 149 | Some(()) |
129 | } | 150 | } |
130 | 151 | ||
131 | fn check_module( | ||
132 | acc: &mut Vec<Diagnostic>, | ||
133 | db: &RootDatabase, | ||
134 | file_id: FileId, | ||
135 | module: hir::Module, | ||
136 | ) { | ||
137 | let source_root = db.file_source_root(file_id); | ||
138 | for (name_node, problem) in module.problems(db) { | ||
139 | let diag = match problem { | ||
140 | Problem::UnresolvedModule { candidate } => { | ||
141 | let create_file = | ||
142 | FileSystemEdit::CreateFile { source_root, path: candidate.clone() }; | ||
143 | let fix = SourceChange::file_system_edit("create module", create_file); | ||
144 | Diagnostic { | ||
145 | range: name_node.range(), | ||
146 | message: "unresolved module".to_string(), | ||
147 | severity: Severity::Error, | ||
148 | fix: Some(fix), | ||
149 | } | ||
150 | } | ||
151 | }; | ||
152 | acc.push(diag) | ||
153 | } | ||
154 | } | ||
155 | |||
156 | #[cfg(test)] | 152 | #[cfg(test)] |
157 | mod tests { | 153 | mod tests { |
158 | use test_utils::assert_eq_text; | 154 | use test_utils::assert_eq_text; |
155 | use insta::assert_debug_snapshot_matches; | ||
156 | |||
157 | use crate::mock_analysis::single_file; | ||
159 | 158 | ||
160 | use super::*; | 159 | use super::*; |
161 | 160 | ||
@@ -185,6 +184,34 @@ mod tests { | |||
185 | } | 184 | } |
186 | 185 | ||
187 | #[test] | 186 | #[test] |
187 | fn test_unresolved_module_diagnostic() { | ||
188 | let (analysis, file_id) = single_file("mod foo;"); | ||
189 | let diagnostics = analysis.diagnostics(file_id).unwrap(); | ||
190 | assert_debug_snapshot_matches!(diagnostics, @r####"[ | ||
191 | Diagnostic { | ||
192 | message: "unresolved module", | ||
193 | range: [0; 8), | ||
194 | fix: Some( | ||
195 | SourceChange { | ||
196 | label: "create module", | ||
197 | source_file_edits: [], | ||
198 | file_system_edits: [ | ||
199 | CreateFile { | ||
200 | source_root: SourceRootId( | ||
201 | 0 | ||
202 | ), | ||
203 | path: "foo.rs" | ||
204 | } | ||
205 | ], | ||
206 | cursor_position: None | ||
207 | } | ||
208 | ), | ||
209 | severity: Error | ||
210 | } | ||
211 | ]"####); | ||
212 | } | ||
213 | |||
214 | #[test] | ||
188 | fn test_check_unnecessary_braces_in_use_statement() { | 215 | fn test_check_unnecessary_braces_in_use_statement() { |
189 | check_not_applicable( | 216 | check_not_applicable( |
190 | " | 217 | " |
diff --git a/crates/ra_ide_api/tests/test/main.rs b/crates/ra_ide_api/tests/test/main.rs index 0f0766f62..d4ff21c09 100644 --- a/crates/ra_ide_api/tests/test/main.rs +++ b/crates/ra_ide_api/tests/test/main.rs | |||
@@ -1,4 +1,3 @@ | |||
1 | use insta::assert_debug_snapshot_matches; | ||
2 | use ra_ide_api::{ | 1 | use ra_ide_api::{ |
3 | mock_analysis::{single_file, single_file_with_position, single_file_with_range, MockAnalysis}, | 2 | mock_analysis::{single_file, single_file_with_position, single_file_with_range, MockAnalysis}, |
4 | AnalysisChange, CrateGraph, Edition::Edition2018, Query, NavigationTarget, | 3 | AnalysisChange, CrateGraph, Edition::Edition2018, Query, NavigationTarget, |
@@ -7,21 +6,6 @@ use ra_ide_api::{ | |||
7 | use ra_syntax::SmolStr; | 6 | use ra_syntax::SmolStr; |
8 | 7 | ||
9 | #[test] | 8 | #[test] |
10 | fn test_unresolved_module_diagnostic() { | ||
11 | let (analysis, file_id) = single_file("mod foo;"); | ||
12 | let diagnostics = analysis.diagnostics(file_id).unwrap(); | ||
13 | assert_debug_snapshot_matches!("unresolved_module_diagnostic", &diagnostics); | ||
14 | } | ||
15 | |||
16 | // FIXME: move this test to hir | ||
17 | #[test] | ||
18 | fn test_unresolved_module_diagnostic_no_diag_for_inline_mode() { | ||
19 | let (analysis, file_id) = single_file("mod foo {}"); | ||
20 | let diagnostics = analysis.diagnostics(file_id).unwrap(); | ||
21 | assert!(diagnostics.is_empty()); | ||
22 | } | ||
23 | |||
24 | #[test] | ||
25 | fn test_resolve_crate_root() { | 9 | fn test_resolve_crate_root() { |
26 | let mock = MockAnalysis::with_files( | 10 | let mock = MockAnalysis::with_files( |
27 | " | 11 | " |
diff --git a/crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap b/crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap deleted file mode 100644 index 5bb953892..000000000 --- a/crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap +++ /dev/null | |||
@@ -1,28 +0,0 @@ | |||
1 | --- | ||
2 | created: "2019-01-22T14:45:01.486985900+00:00" | ||
3 | creator: [email protected] | ||
4 | expression: "&diagnostics" | ||
5 | source: "crates\\ra_ide_api\\tests\\test\\main.rs" | ||
6 | --- | ||
7 | [ | ||
8 | Diagnostic { | ||
9 | message: "unresolved module", | ||
10 | range: [0; 8), | ||
11 | fix: Some( | ||
12 | SourceChange { | ||
13 | label: "create module", | ||
14 | source_file_edits: [], | ||
15 | file_system_edits: [ | ||
16 | CreateFile { | ||
17 | source_root: SourceRootId( | ||
18 | 0 | ||
19 | ), | ||
20 | path: "foo.rs" | ||
21 | } | ||
22 | ], | ||
23 | cursor_position: None | ||
24 | } | ||
25 | ), | ||
26 | severity: Error | ||
27 | } | ||
28 | ] | ||