aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAleksey Kladov <[email protected]>2021-06-13 12:41:19 +0100
committerAleksey Kladov <[email protected]>2021-06-13 12:55:45 +0100
commitefa069d28818dd074afd2c7cee776907b63ca012 (patch)
tree39f3ff2d5154bb62df5e4611f7054e1f7e96eb2f
parent546be18e3a91e4844b0dacc76c9f055397b6d89e (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.rs39
-rw-r--r--crates/hir/src/lib.rs32
-rw-r--r--crates/hir_def/src/nameres/tests/diagnostics.rs14
-rw-r--r--crates/ide/src/diagnostics.rs41
-rw-r--r--crates/ide/src/diagnostics/fixes.rs1
-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.rs5
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 18macro_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
34diagnostics![UnresolvedModule];
35
21#[derive(Debug)] 36#[derive(Debug)]
22pub struct UnresolvedModule { 37pub 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
28impl 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
81use crate::{ 81use 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
93pub use crate::{ 86pub 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]
82fn 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]
96fn inactive_item() { 82fn 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
7mod unresolved_module;
8
7mod fixes; 9mod fixes;
8mod field_shorthand; 10mod field_shorthand;
9mod unlinked_file; 11mod unlinked_file;
@@ -12,7 +14,7 @@ use std::cell::RefCell;
12 14
13use hir::{ 15use 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};
18use ide_assists::AssistResolveStrategy; 20use ide_assists::AssistResolveStrategy;
@@ -42,6 +44,12 @@ pub struct Diagnostic {
42} 44}
43 45
44impl Diagnostic { 46impl 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
93struct DiagnosticsContext<'a> {
94 config: &'a DiagnosticsConfig,
95 sema: Semantics<'a, RootDatabase>,
96 #[allow(unused)]
97 resolve: &'a AssistResolveStrategy,
98}
99
85pub(crate) fn diagnostics( 100pub(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
219fn diagnostic_with_fix<D: DiagnosticWithFixes>( 248fn 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;
5mod fill_missing_fields; 5mod fill_missing_fields;
6mod remove_semicolon; 6mod remove_semicolon;
7mod replace_with_find_map; 7mod replace_with_find_map;
8mod unresolved_module;
9mod wrap_tail_expr; 8mod wrap_tail_expr;
10 9
11use hir::{diagnostics::Diagnostic, Semantics}; 10use 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 @@
1use hir::{db::AstDatabase, diagnostics::UnresolvedModule, Semantics}; 1use hir::db::AstDatabase;
2use ide_assists::{Assist, AssistResolveStrategy}; 2use ide_assists::Assist;
3use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit, RootDatabase}; 3use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit};
4use syntax::AstNode; 4use syntax::AstNode;
5 5
6use crate::diagnostics::{fix, DiagnosticWithFixes}; 6use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext};
7 7
8impl 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>, 11pub(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 { 20fn 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)]
33mod tests { 39mod 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
49mod foo;
50 mod bar;
51//^^^^^^^^ unresolved module
52mod 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());