diff options
author | Aleksey Kladov <[email protected]> | 2021-06-13 12:41:19 +0100 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2021-06-13 12:55:45 +0100 |
commit | efa069d28818dd074afd2c7cee776907b63ca012 (patch) | |
tree | 39f3ff2d5154bb62df5e4611f7054e1f7e96eb2f | |
parent | 546be18e3a91e4844b0dacc76c9f055397b6d89e (diff) |
internal: start new diagnostics API
At the moment, this moves only a single diagnostic, but the idea is
reafactor the rest to use the same pattern. We are going to have a
single file per diagnostic. This file will define diagnostics code,
rendering range and fixes, if any. It'll also have all of the tests.
This is similar to how we deal with assists.
After we refactor all diagnostics to follow this pattern, we'll probably
move them to a new `ide_diagnostics` crate.
Not that we intentionally want to test all diagnostics on this layer,
despite the fact that they are generally emitted in the guts on the
compiler. Diagnostics care to much about the end presentation
details/fixes to be worth-while "unit" testing. So, we'll unit-test only
the primary output of compilation process (types and name res tables),
and will use integrated UI tests for diagnostics.
-rw-r--r-- | crates/hir/src/diagnostics.rs | 39 | ||||
-rw-r--r-- | crates/hir/src/lib.rs | 32 | ||||
-rw-r--r-- | crates/hir_def/src/nameres/tests/diagnostics.rs | 14 | ||||
-rw-r--r-- | crates/ide/src/diagnostics.rs | 41 | ||||
-rw-r--r-- | crates/ide/src/diagnostics/fixes.rs | 1 | ||||
-rw-r--r-- | crates/ide/src/diagnostics/unresolved_module.rs (renamed from crates/ide/src/diagnostics/fixes/unresolved_module.rs) | 74 | ||||
-rw-r--r-- | xtask/src/tidy.rs | 5 |
7 files changed, 123 insertions, 83 deletions
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 8a7c3a4fd..5e2f94698 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs | |||
@@ -15,31 +15,30 @@ pub use crate::diagnostics_sink::{ | |||
15 | Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder, | 15 | Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder, |
16 | }; | 16 | }; |
17 | 17 | ||
18 | // Diagnostic: unresolved-module | 18 | macro_rules! diagnostics { |
19 | // | 19 | ($($diag:ident)*) => { |
20 | // This diagnostic is triggered if rust-analyzer is unable to discover referred module. | 20 | pub enum AnyDiagnostic {$( |
21 | $diag(Box<$diag>), | ||
22 | )*} | ||
23 | |||
24 | $( | ||
25 | impl From<$diag> for AnyDiagnostic { | ||
26 | fn from(d: $diag) -> AnyDiagnostic { | ||
27 | AnyDiagnostic::$diag(Box::new(d)) | ||
28 | } | ||
29 | } | ||
30 | )* | ||
31 | }; | ||
32 | } | ||
33 | |||
34 | diagnostics![UnresolvedModule]; | ||
35 | |||
21 | #[derive(Debug)] | 36 | #[derive(Debug)] |
22 | pub struct UnresolvedModule { | 37 | pub struct UnresolvedModule { |
23 | pub file: HirFileId, | 38 | pub decl: InFile<AstPtr<ast::Module>>, |
24 | pub decl: AstPtr<ast::Module>, | ||
25 | pub candidate: String, | 39 | pub candidate: String, |
26 | } | 40 | } |
27 | 41 | ||
28 | impl Diagnostic for UnresolvedModule { | ||
29 | fn code(&self) -> DiagnosticCode { | ||
30 | DiagnosticCode("unresolved-module") | ||
31 | } | ||
32 | fn message(&self) -> String { | ||
33 | "unresolved module".to_string() | ||
34 | } | ||
35 | fn display_source(&self) -> InFile<SyntaxNodePtr> { | ||
36 | InFile::new(self.file, self.decl.clone().into()) | ||
37 | } | ||
38 | fn as_any(&self) -> &(dyn Any + Send + 'static) { | ||
39 | self | ||
40 | } | ||
41 | } | ||
42 | |||
43 | // Diagnostic: unresolved-extern-crate | 42 | // Diagnostic: unresolved-extern-crate |
44 | // | 43 | // |
45 | // This diagnostic is triggered if rust-analyzer is unable to discover referred extern crate. | 44 | // This diagnostic is triggered if rust-analyzer is unable to discover referred extern crate. |
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index bd923cba8..ff6c68dbc 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs | |||
@@ -80,18 +80,18 @@ use tt::{Ident, Leaf, Literal, TokenTree}; | |||
80 | 80 | ||
81 | use crate::{ | 81 | use crate::{ |
82 | db::{DefDatabase, HirDatabase}, | 82 | db::{DefDatabase, HirDatabase}, |
83 | diagnostics::{ | ||
84 | BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, MismatchedArgCount, | ||
85 | MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, MissingPatFields, | ||
86 | MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, | ||
87 | UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, | ||
88 | UnresolvedModule, UnresolvedProcMacro, | ||
89 | }, | ||
90 | diagnostics_sink::DiagnosticSink, | 83 | diagnostics_sink::DiagnosticSink, |
91 | }; | 84 | }; |
92 | 85 | ||
93 | pub use crate::{ | 86 | pub use crate::{ |
94 | attrs::{HasAttrs, Namespace}, | 87 | attrs::{HasAttrs, Namespace}, |
88 | diagnostics::{ | ||
89 | AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, | ||
90 | MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, | ||
91 | MissingPatFields, MissingUnsafe, NoSuchField, RemoveThisSemicolon, | ||
92 | ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate, | ||
93 | UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro, | ||
94 | }, | ||
95 | has_source::HasSource, | 95 | has_source::HasSource, |
96 | semantics::{PathResolution, Semantics, SemanticsScope}, | 96 | semantics::{PathResolution, Semantics, SemanticsScope}, |
97 | }; | 97 | }; |
@@ -460,10 +460,11 @@ impl Module { | |||
460 | db: &dyn HirDatabase, | 460 | db: &dyn HirDatabase, |
461 | sink: &mut DiagnosticSink, | 461 | sink: &mut DiagnosticSink, |
462 | internal_diagnostics: bool, | 462 | internal_diagnostics: bool, |
463 | ) { | 463 | ) -> Vec<AnyDiagnostic> { |
464 | let _p = profile::span("Module::diagnostics").detail(|| { | 464 | let _p = profile::span("Module::diagnostics").detail(|| { |
465 | format!("{:?}", self.name(db).map_or("<unknown>".into(), |name| name.to_string())) | 465 | format!("{:?}", self.name(db).map_or("<unknown>".into(), |name| name.to_string())) |
466 | }); | 466 | }); |
467 | let mut acc: Vec<AnyDiagnostic> = Vec::new(); | ||
467 | let def_map = self.id.def_map(db.upcast()); | 468 | let def_map = self.id.def_map(db.upcast()); |
468 | for diag in def_map.diagnostics() { | 469 | for diag in def_map.diagnostics() { |
469 | if diag.in_module != self.id.local_id { | 470 | if diag.in_module != self.id.local_id { |
@@ -473,11 +474,13 @@ impl Module { | |||
473 | match &diag.kind { | 474 | match &diag.kind { |
474 | DefDiagnosticKind::UnresolvedModule { ast: declaration, candidate } => { | 475 | DefDiagnosticKind::UnresolvedModule { ast: declaration, candidate } => { |
475 | let decl = declaration.to_node(db.upcast()); | 476 | let decl = declaration.to_node(db.upcast()); |
476 | sink.push(UnresolvedModule { | 477 | acc.push( |
477 | file: declaration.file_id, | 478 | UnresolvedModule { |
478 | decl: AstPtr::new(&decl), | 479 | decl: InFile::new(declaration.file_id, AstPtr::new(&decl)), |
479 | candidate: candidate.clone(), | 480 | candidate: candidate.clone(), |
480 | }) | 481 | } |
482 | .into(), | ||
483 | ) | ||
481 | } | 484 | } |
482 | DefDiagnosticKind::UnresolvedExternCrate { ast } => { | 485 | DefDiagnosticKind::UnresolvedExternCrate { ast } => { |
483 | let item = ast.to_node(db.upcast()); | 486 | let item = ast.to_node(db.upcast()); |
@@ -610,7 +613,7 @@ impl Module { | |||
610 | crate::ModuleDef::Module(m) => { | 613 | crate::ModuleDef::Module(m) => { |
611 | // Only add diagnostics from inline modules | 614 | // Only add diagnostics from inline modules |
612 | if def_map[m.id.local_id].origin.is_inline() { | 615 | if def_map[m.id.local_id].origin.is_inline() { |
613 | m.diagnostics(db, sink, internal_diagnostics) | 616 | acc.extend(m.diagnostics(db, sink, internal_diagnostics)) |
614 | } | 617 | } |
615 | } | 618 | } |
616 | _ => { | 619 | _ => { |
@@ -626,6 +629,7 @@ impl Module { | |||
626 | } | 629 | } |
627 | } | 630 | } |
628 | } | 631 | } |
632 | acc | ||
629 | } | 633 | } |
630 | 634 | ||
631 | pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> { | 635 | pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> { |
diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs index ec6670952..c82deca9c 100644 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs | |||
@@ -79,20 +79,6 @@ fn dedup_unresolved_import_from_unresolved_crate() { | |||
79 | } | 79 | } |
80 | 80 | ||
81 | #[test] | 81 | #[test] |
82 | fn unresolved_module() { | ||
83 | check_diagnostics( | ||
84 | r" | ||
85 | //- /lib.rs | ||
86 | mod foo; | ||
87 | mod bar; | ||
88 | //^^^^^^^^ UnresolvedModule | ||
89 | mod baz {} | ||
90 | //- /foo.rs | ||
91 | ", | ||
92 | ); | ||
93 | } | ||
94 | |||
95 | #[test] | ||
96 | fn inactive_item() { | 82 | fn inactive_item() { |
97 | // Additional tests in `cfg` crate. This only tests disabled cfgs. | 83 | // Additional tests in `cfg` crate. This only tests disabled cfgs. |
98 | 84 | ||
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 337a904b6..075aae8d5 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs | |||
@@ -4,6 +4,8 @@ | |||
4 | //! macro-expanded files, but we need to present them to the users in terms of | 4 | //! macro-expanded files, but we need to present them to the users in terms of |
5 | //! original files. So we need to map the ranges. | 5 | //! original files. So we need to map the ranges. |
6 | 6 | ||
7 | mod unresolved_module; | ||
8 | |||
7 | mod fixes; | 9 | mod fixes; |
8 | mod field_shorthand; | 10 | mod field_shorthand; |
9 | mod unlinked_file; | 11 | mod unlinked_file; |
@@ -12,7 +14,7 @@ use std::cell::RefCell; | |||
12 | 14 | ||
13 | use hir::{ | 15 | use hir::{ |
14 | db::AstDatabase, | 16 | db::AstDatabase, |
15 | diagnostics::{Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder}, | 17 | diagnostics::{AnyDiagnostic, Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder}, |
16 | InFile, Semantics, | 18 | InFile, Semantics, |
17 | }; | 19 | }; |
18 | use ide_assists::AssistResolveStrategy; | 20 | use ide_assists::AssistResolveStrategy; |
@@ -42,6 +44,12 @@ pub struct Diagnostic { | |||
42 | } | 44 | } |
43 | 45 | ||
44 | impl Diagnostic { | 46 | impl Diagnostic { |
47 | fn new(code: &'static str, message: impl Into<String>, range: TextRange) -> Diagnostic { | ||
48 | let message = message.into(); | ||
49 | let code = Some(DiagnosticCode(code)); | ||
50 | Self { message, range, severity: Severity::Error, fixes: None, unused: false, code } | ||
51 | } | ||
52 | |||
45 | fn error(range: TextRange, message: String) -> Self { | 53 | fn error(range: TextRange, message: String) -> Self { |
46 | Self { message, range, severity: Severity::Error, fixes: None, unused: false, code: None } | 54 | Self { message, range, severity: Severity::Error, fixes: None, unused: false, code: None } |
47 | } | 55 | } |
@@ -82,6 +90,13 @@ pub struct DiagnosticsConfig { | |||
82 | pub disabled: FxHashSet<String>, | 90 | pub disabled: FxHashSet<String>, |
83 | } | 91 | } |
84 | 92 | ||
93 | struct DiagnosticsContext<'a> { | ||
94 | config: &'a DiagnosticsConfig, | ||
95 | sema: Semantics<'a, RootDatabase>, | ||
96 | #[allow(unused)] | ||
97 | resolve: &'a AssistResolveStrategy, | ||
98 | } | ||
99 | |||
85 | pub(crate) fn diagnostics( | 100 | pub(crate) fn diagnostics( |
86 | db: &RootDatabase, | 101 | db: &RootDatabase, |
87 | config: &DiagnosticsConfig, | 102 | config: &DiagnosticsConfig, |
@@ -108,9 +123,6 @@ pub(crate) fn diagnostics( | |||
108 | } | 123 | } |
109 | let res = RefCell::new(res); | 124 | let res = RefCell::new(res); |
110 | let sink_builder = DiagnosticSinkBuilder::new() | 125 | let sink_builder = DiagnosticSinkBuilder::new() |
111 | .on::<hir::diagnostics::UnresolvedModule, _>(|d| { | ||
112 | res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); | ||
113 | }) | ||
114 | .on::<hir::diagnostics::MissingFields, _>(|d| { | 126 | .on::<hir::diagnostics::MissingFields, _>(|d| { |
115 | res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); | 127 | res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); |
116 | }) | 128 | }) |
@@ -204,16 +216,33 @@ pub(crate) fn diagnostics( | |||
204 | ); | 216 | ); |
205 | }); | 217 | }); |
206 | 218 | ||
219 | let mut diags = Vec::new(); | ||
207 | let internal_diagnostics = cfg!(test); | 220 | let internal_diagnostics = cfg!(test); |
208 | match sema.to_module_def(file_id) { | 221 | match sema.to_module_def(file_id) { |
209 | Some(m) => m.diagnostics(db, &mut sink, internal_diagnostics), | 222 | Some(m) => diags = m.diagnostics(db, &mut sink, internal_diagnostics), |
210 | None => { | 223 | None => { |
211 | sink.push(UnlinkedFile { file_id, node: SyntaxNodePtr::new(parse.tree().syntax()) }); | 224 | sink.push(UnlinkedFile { file_id, node: SyntaxNodePtr::new(parse.tree().syntax()) }); |
212 | } | 225 | } |
213 | } | 226 | } |
214 | 227 | ||
215 | drop(sink); | 228 | drop(sink); |
216 | res.into_inner() | 229 | |
230 | let mut res = res.into_inner(); | ||
231 | |||
232 | let ctx = DiagnosticsContext { config, sema, resolve }; | ||
233 | for diag in diags { | ||
234 | let d = match diag { | ||
235 | AnyDiagnostic::UnresolvedModule(d) => unresolved_module::render(&ctx, &d), | ||
236 | }; | ||
237 | if let Some(code) = d.code { | ||
238 | if ctx.config.disabled.contains(code.as_str()) { | ||
239 | continue; | ||
240 | } | ||
241 | } | ||
242 | res.push(d) | ||
243 | } | ||
244 | |||
245 | res | ||
217 | } | 246 | } |
218 | 247 | ||
219 | fn diagnostic_with_fix<D: DiagnosticWithFixes>( | 248 | fn diagnostic_with_fix<D: DiagnosticWithFixes>( |
diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 258ac6974..8640d7231 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs | |||
@@ -5,7 +5,6 @@ mod create_field; | |||
5 | mod fill_missing_fields; | 5 | mod fill_missing_fields; |
6 | mod remove_semicolon; | 6 | mod remove_semicolon; |
7 | mod replace_with_find_map; | 7 | mod replace_with_find_map; |
8 | mod unresolved_module; | ||
9 | mod wrap_tail_expr; | 8 | mod wrap_tail_expr; |
10 | 9 | ||
11 | use hir::{diagnostics::Diagnostic, Semantics}; | 10 | use hir::{diagnostics::Diagnostic, Semantics}; |
diff --git a/crates/ide/src/diagnostics/fixes/unresolved_module.rs b/crates/ide/src/diagnostics/unresolved_module.rs index b3d0283bb..abf53a57c 100644 --- a/crates/ide/src/diagnostics/fixes/unresolved_module.rs +++ b/crates/ide/src/diagnostics/unresolved_module.rs | |||
@@ -1,39 +1,59 @@ | |||
1 | use hir::{db::AstDatabase, diagnostics::UnresolvedModule, Semantics}; | 1 | use hir::db::AstDatabase; |
2 | use ide_assists::{Assist, AssistResolveStrategy}; | 2 | use ide_assists::Assist; |
3 | use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit, RootDatabase}; | 3 | use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit}; |
4 | use syntax::AstNode; | 4 | use syntax::AstNode; |
5 | 5 | ||
6 | use crate::diagnostics::{fix, DiagnosticWithFixes}; | 6 | use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext}; |
7 | 7 | ||
8 | impl DiagnosticWithFixes for UnresolvedModule { | 8 | // Diagnostic: unresolved-module |
9 | fn fixes( | 9 | // |
10 | &self, | 10 | // This diagnostic is triggered if rust-analyzer is unable to discover referred module. |
11 | sema: &Semantics<RootDatabase>, | 11 | pub(super) fn render(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedModule) -> Diagnostic { |
12 | _resolve: &AssistResolveStrategy, | 12 | Diagnostic::new( |
13 | ) -> Option<Vec<Assist>> { | 13 | "unresolved-module", |
14 | let root = sema.db.parse_or_expand(self.file)?; | 14 | "unresolved module", |
15 | let unresolved_module = self.decl.to_node(&root); | 15 | ctx.sema.diagnostics_display_range(d.decl.clone().map(|it| it.into())).range, |
16 | Some(vec![fix( | 16 | ) |
17 | "create_module", | 17 | .with_fixes(fixes(ctx, d)) |
18 | "Create module", | 18 | } |
19 | FileSystemEdit::CreateFile { | 19 | |
20 | dst: AnchoredPathBuf { | 20 | fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedModule) -> Option<Vec<Assist>> { |
21 | anchor: self.file.original_file(sema.db), | 21 | let root = ctx.sema.db.parse_or_expand(d.decl.file_id)?; |
22 | path: self.candidate.clone(), | 22 | let unresolved_module = d.decl.value.to_node(&root); |
23 | }, | 23 | Some(vec![fix( |
24 | initial_contents: "".to_string(), | 24 | "create_module", |
25 | } | 25 | "Create module", |
26 | .into(), | 26 | FileSystemEdit::CreateFile { |
27 | unresolved_module.syntax().text_range(), | 27 | dst: AnchoredPathBuf { |
28 | )]) | 28 | anchor: d.decl.file_id.original_file(ctx.sema.db), |
29 | } | 29 | path: d.candidate.clone(), |
30 | }, | ||
31 | initial_contents: "".to_string(), | ||
32 | } | ||
33 | .into(), | ||
34 | unresolved_module.syntax().text_range(), | ||
35 | )]) | ||
30 | } | 36 | } |
31 | 37 | ||
32 | #[cfg(test)] | 38 | #[cfg(test)] |
33 | mod tests { | 39 | mod tests { |
34 | use expect_test::expect; | 40 | use expect_test::expect; |
35 | 41 | ||
36 | use crate::diagnostics::tests::check_expect; | 42 | use crate::diagnostics::tests::{check_diagnostics, check_expect}; |
43 | |||
44 | #[test] | ||
45 | fn unresolved_module() { | ||
46 | check_diagnostics( | ||
47 | r#" | ||
48 | //- /lib.rs | ||
49 | mod foo; | ||
50 | mod bar; | ||
51 | //^^^^^^^^ unresolved module | ||
52 | mod baz {} | ||
53 | //- /foo.rs | ||
54 | "#, | ||
55 | ); | ||
56 | } | ||
37 | 57 | ||
38 | #[test] | 58 | #[test] |
39 | fn test_unresolved_module_diagnostic() { | 59 | fn test_unresolved_module_diagnostic() { |
diff --git a/xtask/src/tidy.rs b/xtask/src/tidy.rs index e6fa5868d..f2ba8efef 100644 --- a/xtask/src/tidy.rs +++ b/xtask/src/tidy.rs | |||
@@ -372,7 +372,10 @@ impl TidyDocs { | |||
372 | self.contains_fixme.push(path.to_path_buf()); | 372 | self.contains_fixme.push(path.to_path_buf()); |
373 | } | 373 | } |
374 | } else { | 374 | } else { |
375 | if text.contains("// Feature:") || text.contains("// Assist:") { | 375 | if text.contains("// Feature:") |
376 | || text.contains("// Assist:") | ||
377 | || text.contains("// Diagnostic:") | ||
378 | { | ||
376 | return; | 379 | return; |
377 | } | 380 | } |
378 | self.missing_docs.push(path.display().to_string()); | 381 | self.missing_docs.push(path.display().to_string()); |