From 2a9a66d25485036455eac54747a83ac7c6557d44 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 16 Sep 2020 14:52:39 +0200 Subject: Add diagnostic types for unresolved crates/imports --- crates/hir_def/src/nameres/collector.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'crates/hir_def/src/nameres') diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 3e99c8773..bc286a8a3 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -1051,13 +1051,11 @@ impl ModCollector<'_, '_> { self.import_all_legacy_macros(module_id); } } - Err(candidate) => self.def_collector.def_map.diagnostics.push( - DefDiagnostic::UnresolvedModule { - module: self.module_id, - declaration: ast_id, - candidate, - }, - ), + Err(candidate) => { + self.def_collector.def_map.diagnostics.push( + DefDiagnostic::unresolved_module(self.module_id, ast_id, candidate), + ); + } }; } } -- cgit v1.2.3 From 4ac9a2e5d3c43401450b812786ab1551d535420c Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 16 Sep 2020 15:46:56 +0200 Subject: Leave extern crate items unresolved if they are --- crates/hir_def/src/nameres/collector.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'crates/hir_def/src/nameres') diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index bc286a8a3..952a04b35 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -420,7 +420,11 @@ impl DefCollector<'_> { .as_ident() .expect("extern crate should have been desugared to one-element path"), ); - PartialResolvedImport::Resolved(res) + if res.is_none() { + PartialResolvedImport::Unresolved + } else { + PartialResolvedImport::Resolved(res) + } } else { let res = self.def_map.resolve_path_fp_with_macro( self.db, -- cgit v1.2.3 From 4785162b080acf3b5e711f49b2df399b11ee5cb0 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 16 Sep 2020 15:47:58 +0200 Subject: Track import sources and emit diagnostics --- crates/hir_def/src/nameres/collector.rs | 61 +++++++++++++++++----- crates/hir_def/src/nameres/tests/mod_resolution.rs | 20 +++---- 2 files changed, 60 insertions(+), 21 deletions(-) (limited to 'crates/hir_def/src/nameres') diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 952a04b35..deb3885f9 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -5,6 +5,7 @@ use base_db::{CrateId, FileId, ProcMacroId}; use cfg::CfgOptions; +use hir_expand::InFile; use hir_expand::{ ast_id_map::FileAstId, builtin_derive::find_builtin_derive, @@ -21,9 +22,7 @@ use crate::{ attr::Attrs, db::DefDatabase, item_scope::{ImportType, PerNsGlobImports}, - item_tree::{ - self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind, - }, + item_tree::{self, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind}, nameres::{ diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint, BuiltinShadowMode, CrateDefMap, ModuleData, ModuleOrigin, ResolveMode, @@ -111,6 +110,12 @@ impl PartialResolvedImport { } } +#[derive(Clone, Debug, Eq, PartialEq)] +enum ImportSource { + Import(ItemTreeId), + ExternCrate(ItemTreeId), +} + #[derive(Clone, Debug, Eq, PartialEq)] struct Import { pub path: ModPath, @@ -120,11 +125,12 @@ struct Import { pub is_prelude: bool, pub is_extern_crate: bool, pub is_macro_use: bool, + source: ImportSource, } impl Import { - fn from_use(tree: &ItemTree, id: FileItemTreeId) -> Self { - let it = &tree[id]; + fn from_use(tree: &ItemTree, id: ItemTreeId) -> Self { + let it = &tree[id.value]; let visibility = &tree[it.visibility]; Self { path: it.path.clone(), @@ -134,11 +140,12 @@ impl Import { is_prelude: it.is_prelude, is_extern_crate: false, is_macro_use: false, + source: ImportSource::Import(id), } } - fn from_extern_crate(tree: &ItemTree, id: FileItemTreeId) -> Self { - let it = &tree[id]; + fn from_extern_crate(tree: &ItemTree, id: ItemTreeId) -> Self { + let it = &tree[id.value]; let visibility = &tree[it.visibility]; Self { path: it.path.clone(), @@ -148,6 +155,7 @@ impl Import { is_prelude: false, is_extern_crate: true, is_macro_use: it.is_macro_use, + source: ImportSource::ExternCrate(id), } } } @@ -245,9 +253,10 @@ impl DefCollector<'_> { let unresolved_imports = std::mem::replace(&mut self.unresolved_imports, Vec::new()); // show unresolved imports in completion, etc - for directive in unresolved_imports { - self.record_resolved_import(&directive) + for directive in &unresolved_imports { + self.record_resolved_import(directive) } + self.unresolved_imports = unresolved_imports; // Record proc-macros self.collect_proc_macro(); @@ -778,7 +787,29 @@ impl DefCollector<'_> { .collect(item_tree.top_level_items()); } - fn finish(self) -> CrateDefMap { + fn finish(mut self) -> CrateDefMap { + for directive in &self.unresolved_imports { + match directive.import.source { + ImportSource::Import(import) => { + let item_tree = self.db.item_tree(import.file_id); + let import_data = &item_tree[import.value]; + self.def_map.diagnostics.push(DefDiagnostic::unresolved_import( + directive.module_id, + InFile::new(import.file_id, import_data.ast_id), + import_data.index, + )); + } + ImportSource::ExternCrate(krate) => { + let item_tree = self.db.item_tree(krate.file_id); + let extern_crate = &item_tree[krate.value]; + self.def_map.diagnostics.push(DefDiagnostic::unresolved_extern_crate( + directive.module_id, + InFile::new(krate.file_id, extern_crate.ast_id), + )); + } + } + } + self.def_map } } @@ -834,14 +865,20 @@ impl ModCollector<'_, '_> { ModItem::Import(import_id) => { self.def_collector.unresolved_imports.push(ImportDirective { module_id: self.module_id, - import: Import::from_use(&self.item_tree, import_id), + import: Import::from_use( + &self.item_tree, + InFile::new(self.file_id, import_id), + ), status: PartialResolvedImport::Unresolved, }) } ModItem::ExternCrate(import_id) => { self.def_collector.unresolved_imports.push(ImportDirective { module_id: self.module_id, - import: Import::from_extern_crate(&self.item_tree, import_id), + import: Import::from_extern_crate( + &self.item_tree, + InFile::new(self.file_id, import_id), + ), status: PartialResolvedImport::Unresolved, }) } diff --git a/crates/hir_def/src/nameres/tests/mod_resolution.rs b/crates/hir_def/src/nameres/tests/mod_resolution.rs index 1f619787e..3b9f79544 100644 --- a/crates/hir_def/src/nameres/tests/mod_resolution.rs +++ b/crates/hir_def/src/nameres/tests/mod_resolution.rs @@ -688,19 +688,21 @@ fn unresolved_module_diagnostics() { expect![[r#" [ - UnresolvedModule { - module: Idx::(0), - declaration: InFile { - file_id: HirFileId( - FileId( + DefDiagnostic { + in_module: Idx::(0), + kind: UnresolvedModule { + declaration: InFile { + file_id: HirFileId( FileId( - 0, + FileId( + 0, + ), ), ), - ), - value: FileAstId::(1), + value: FileAstId::(1), + }, + candidate: "bar.rs", }, - candidate: "bar.rs", }, ] "#]] -- cgit v1.2.3 From f792bc7ddd2616c0bb1fcdffda204151fc40b3d6 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 16 Sep 2020 17:26:16 +0200 Subject: Add annotation-based nameres diagnostic tests --- crates/hir_def/src/nameres/tests.rs | 1 + crates/hir_def/src/nameres/tests/diagnostics.rs | 107 +++++++++++++++++++++ crates/hir_def/src/nameres/tests/mod_resolution.rs | 38 -------- 3 files changed, 108 insertions(+), 38 deletions(-) create mode 100644 crates/hir_def/src/nameres/tests/diagnostics.rs (limited to 'crates/hir_def/src/nameres') diff --git a/crates/hir_def/src/nameres/tests.rs b/crates/hir_def/src/nameres/tests.rs index 5ca30dac9..11d84f808 100644 --- a/crates/hir_def/src/nameres/tests.rs +++ b/crates/hir_def/src/nameres/tests.rs @@ -2,6 +2,7 @@ mod globs; mod incremental; mod macros; mod mod_resolution; +mod diagnostics; mod primitives; use std::sync::Arc; diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs new file mode 100644 index 000000000..cd0eb1a4b --- /dev/null +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs @@ -0,0 +1,107 @@ +use base_db::fixture::WithFixture; +use base_db::FileId; +use base_db::SourceDatabaseExt; +use hir_expand::db::AstDatabase; +use rustc_hash::FxHashMap; +use syntax::TextRange; +use syntax::TextSize; + +use crate::test_db::TestDB; + +fn check_diagnostics(ra_fixture: &str) { + let db: TestDB = TestDB::with_files(ra_fixture); + let annotations = db.extract_annotations(); + assert!(!annotations.is_empty()); + + let mut actual: FxHashMap> = FxHashMap::default(); + db.diagnostics(|d| { + let src = d.display_source(); + let root = db.parse_or_expand(src.file_id).unwrap(); + // FIXME: macros... + let file_id = src.file_id.original_file(&db); + let range = src.value.to_node(&root).text_range(); + let message = d.message().to_owned(); + actual.entry(file_id).or_default().push((range, message)); + }); + + for (file_id, diags) in actual.iter_mut() { + diags.sort_by_key(|it| it.0.start()); + let text = db.file_text(*file_id); + // For multiline spans, place them on line start + for (range, content) in diags { + if text[*range].contains('\n') { + *range = TextRange::new(range.start(), range.start() + TextSize::from(1)); + *content = format!("... {}", content); + } + } + } + + assert_eq!(annotations, actual); +} + +#[test] +fn unresolved_import() { + check_diagnostics( + r" + use does_exist; + use does_not_exist; + //^^^^^^^^^^^^^^ unresolved import + + mod does_exist {} + ", + ); +} + +#[test] +fn unresolved_import_in_use_tree() { + // Only the relevant part of a nested `use` item should be highlighted. + check_diagnostics( + r" + use does_exist::{Exists, DoesntExist}; + //^^^^^^^^^^^ unresolved import + + use {does_not_exist::*, does_exist}; + //^^^^^^^^^^^^^^^^^ unresolved import + + use does_not_exist::{ + a, + //^ unresolved import + b, + //^ unresolved import + c, + //^ unresolved import + }; + + mod does_exist { + pub struct Exists; + } + ", + ); +} + +#[test] +fn unresolved_extern_crate() { + check_diagnostics( + r" + //- /main.rs crate:main deps:core + extern crate core; + extern crate doesnotexist; + //^^^^^^^^^^^^^^^^^^^^^^^^^^ unresolved extern crate + //- /lib.rs crate:core + ", + ); +} + +#[test] +fn unresolved_module() { + check_diagnostics( + r" + //- /lib.rs + mod foo; + mod bar; + //^^^^^^^^ unresolved module + mod baz {} + //- /foo.rs + ", + ); +} diff --git a/crates/hir_def/src/nameres/tests/mod_resolution.rs b/crates/hir_def/src/nameres/tests/mod_resolution.rs index 3b9f79544..f93337a6e 100644 --- a/crates/hir_def/src/nameres/tests/mod_resolution.rs +++ b/crates/hir_def/src/nameres/tests/mod_resolution.rs @@ -671,44 +671,6 @@ pub struct Baz; ); } -#[test] -fn unresolved_module_diagnostics() { - let db = TestDB::with_files( - r" - //- /lib.rs - mod foo; - mod bar; - mod baz {} - //- /foo.rs - ", - ); - let krate = db.test_crate(); - - let crate_def_map = db.crate_def_map(krate); - - expect![[r#" - [ - DefDiagnostic { - in_module: Idx::(0), - kind: UnresolvedModule { - declaration: InFile { - file_id: HirFileId( - FileId( - FileId( - 0, - ), - ), - ), - value: FileAstId::(1), - }, - candidate: "bar.rs", - }, - }, - ] - "#]] - .assert_debug_eq(&crate_def_map.diagnostics); -} - #[test] fn module_resolution_decl_inside_module_in_non_crate_root_2() { check( -- cgit v1.2.3 From 0dca7acf0fb65545f0c46f0c604bb15400aa6d91 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 17 Sep 2020 14:48:17 +0200 Subject: Don't diagnose imports whose base crate is missing --- crates/hir_def/src/nameres/collector.rs | 57 +++++++++++++++++-------- crates/hir_def/src/nameres/tests/diagnostics.rs | 24 +++++++++++ 2 files changed, 64 insertions(+), 17 deletions(-) (limited to 'crates/hir_def/src/nameres') diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index deb3885f9..818008169 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -15,6 +15,7 @@ use hir_expand::{ HirFileId, MacroCallId, MacroDefId, MacroDefKind, }; use rustc_hash::FxHashMap; +use rustc_hash::FxHashSet; use syntax::ast; use test_utils::mark; @@ -788,25 +789,47 @@ impl DefCollector<'_> { } fn finish(mut self) -> CrateDefMap { + // Emit diagnostics for all remaining unresolved imports. + + // We'd like to avoid emitting a diagnostics avalanche when some `extern crate` doesn't + // resolve. We first emit diagnostics for unresolved extern crates and collect the missing + // crate names. Then we emit diagnostics for unresolved imports, but only if the import + // doesn't start with an unresolved crate's name. Due to renaming and reexports, this is a + // heuristic, but it works in practice. + let mut diagnosed_extern_crates = FxHashSet::default(); for directive in &self.unresolved_imports { - match directive.import.source { - ImportSource::Import(import) => { - let item_tree = self.db.item_tree(import.file_id); - let import_data = &item_tree[import.value]; - self.def_map.diagnostics.push(DefDiagnostic::unresolved_import( - directive.module_id, - InFile::new(import.file_id, import_data.ast_id), - import_data.index, - )); - } - ImportSource::ExternCrate(krate) => { - let item_tree = self.db.item_tree(krate.file_id); - let extern_crate = &item_tree[krate.value]; - self.def_map.diagnostics.push(DefDiagnostic::unresolved_extern_crate( - directive.module_id, - InFile::new(krate.file_id, extern_crate.ast_id), - )); + if let ImportSource::ExternCrate(krate) = directive.import.source { + let item_tree = self.db.item_tree(krate.file_id); + let extern_crate = &item_tree[krate.value]; + + diagnosed_extern_crates.insert(extern_crate.path.segments[0].clone()); + + self.def_map.diagnostics.push(DefDiagnostic::unresolved_extern_crate( + directive.module_id, + InFile::new(krate.file_id, extern_crate.ast_id), + )); + } + } + + for directive in &self.unresolved_imports { + if let ImportSource::Import(import) = &directive.import.source { + let item_tree = self.db.item_tree(import.file_id); + let import_data = &item_tree[import.value]; + + match (import_data.path.segments.first(), &import_data.path.kind) { + (Some(krate), PathKind::Plain) | (Some(krate), PathKind::Abs) => { + if diagnosed_extern_crates.contains(krate) { + continue; + } + } + _ => {} } + + self.def_map.diagnostics.push(DefDiagnostic::unresolved_import( + directive.module_id, + InFile::new(import.file_id, import_data.ast_id), + import_data.index, + )); } } diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs index cd0eb1a4b..576b813d2 100644 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs @@ -92,6 +92,30 @@ fn unresolved_extern_crate() { ); } +#[test] +fn dedup_unresolved_import_from_unresolved_crate() { + check_diagnostics( + r" + //- /main.rs crate:main + mod a { + extern crate doesnotexist; + //^^^^^^^^^^^^^^^^^^^^^^^^^^ unresolved extern crate + + // Should not error, since we already errored for the missing crate. + use doesnotexist::{self, bla, *}; + + use crate::doesnotexist; + //^^^^^^^^^^^^^^^^^^^ unresolved import + } + + mod m { + use super::doesnotexist; + //^^^^^^^^^^^^^^^^^^^ unresolved import + } + ", + ); +} + #[test] fn unresolved_module() { check_diagnostics( -- cgit v1.2.3