From 44f4510caa6becafc3621253e8115d94b6bd4423 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 16 Sep 2020 12:35:09 +0200 Subject: Store `Import` indices for later reconstruction --- crates/hir_def/src/item_tree.rs | 6 +++++- crates/hir_def/src/item_tree/lower.rs | 3 ++- crates/hir_def/src/item_tree/tests.rs | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) (limited to 'crates/hir_def/src') diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs index e14722cae..f02cfb0c6 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs @@ -291,7 +291,6 @@ pub enum AttrOwner { Variant(Idx), Field(Idx), - // FIXME: Store variant and field attrs, and stop reparsing them in `attrs_query`. } macro_rules! from_attrs { @@ -483,6 +482,11 @@ pub struct Import { /// AST ID of the `use` or `extern crate` item this import was derived from. Note that many /// `Import`s can map to the same `use` item. pub ast_id: FileAstId, + /// Index of this `Import` when the containing `Use` is visited via `ModPath::expand_use_item`. + /// + /// This can be used to get the `UseTree` this `Import` corresponds to and allows emitting + /// precise diagnostics. + pub index: usize, } #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs index 6a503d785..9160bfafe 100644 --- a/crates/hir_def/src/item_tree/lower.rs +++ b/crates/hir_def/src/item_tree/lower.rs @@ -482,7 +482,7 @@ impl Ctx { ModPath::expand_use_item( InFile::new(self.file, use_item.clone()), &self.hygiene, - |path, _tree, is_glob, alias| { + |path, _use_tree, is_glob, alias| { imports.push(id(tree.imports.alloc(Import { path, alias, @@ -490,6 +490,7 @@ impl Ctx { is_glob, is_prelude, ast_id, + index: imports.len(), }))); }, ); diff --git a/crates/hir_def/src/item_tree/tests.rs b/crates/hir_def/src/item_tree/tests.rs index 620e697d4..d26c032b7 100644 --- a/crates/hir_def/src/item_tree/tests.rs +++ b/crates/hir_def/src/item_tree/tests.rs @@ -228,9 +228,9 @@ fn smoke() { top-level items: #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_on_use"))] }, input: None }]) }] - Import { path: ModPath { kind: Plain, segments: [Name(Text("a"))] }, alias: None, visibility: RawVisibilityId("pub(self)"), is_glob: false, is_prelude: false, ast_id: FileAstId::(0) } + Import { path: ModPath { kind: Plain, segments: [Name(Text("a"))] }, alias: None, visibility: RawVisibilityId("pub(self)"), is_glob: false, is_prelude: false, ast_id: FileAstId::(0), index: 0 } #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_on_use"))] }, input: None }]) }] - Import { path: ModPath { kind: Plain, segments: [Name(Text("b"))] }, alias: None, visibility: RawVisibilityId("pub(self)"), is_glob: true, is_prelude: false, ast_id: FileAstId::(0) } + Import { path: ModPath { kind: Plain, segments: [Name(Text("b"))] }, alias: None, visibility: RawVisibilityId("pub(self)"), is_glob: true, is_prelude: false, ast_id: FileAstId::(0), index: 1 } #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("ext_crate"))] }, input: None }]) }] ExternCrate { path: ModPath { kind: Plain, segments: [Name(Text("krate"))] }, alias: None, visibility: RawVisibilityId("pub(self)"), is_macro_use: false, ast_id: FileAstId::(1) } #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("on_trait"))] }, input: None }]) }] -- cgit v1.2.3 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/diagnostics.rs | 42 +++++++++++++++ crates/hir_def/src/nameres.rs | 93 ++++++++++++++++++++++++++++----- crates/hir_def/src/nameres/collector.rs | 12 ++--- 3 files changed, 128 insertions(+), 19 deletions(-) (limited to 'crates/hir_def/src') diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index 3e19d9117..2ec0fd3fb 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -28,3 +28,45 @@ impl Diagnostic for UnresolvedModule { self } } + +#[derive(Debug)] +pub struct UnresolvedExternCrate { + pub file: HirFileId, + pub item: AstPtr, +} + +impl Diagnostic for UnresolvedExternCrate { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("unresolved-extern-crate") + } + fn message(&self) -> String { + "unresolved extern crate".to_string() + } + fn display_source(&self) -> InFile { + InFile::new(self.file, self.item.clone().into()) + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +#[derive(Debug)] +pub struct UnresolvedImport { + pub file: HirFileId, + pub node: AstPtr, +} + +impl Diagnostic for UnresolvedImport { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("unresolved-import") + } + fn message(&self) -> String { + "unresolved import".to_string() + } + fn display_source(&self) -> InFile { + InFile::new(self.file, self.node.clone().into()) + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index bf302172d..5e4d73c1f 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -288,31 +288,70 @@ pub enum ModuleSource { mod diagnostics { use hir_expand::diagnostics::DiagnosticSink; + use hir_expand::hygiene::Hygiene; + use hir_expand::InFile; use syntax::{ast, AstPtr}; - use crate::{db::DefDatabase, diagnostics::UnresolvedModule, nameres::LocalModuleId, AstId}; + use crate::path::ModPath; + use crate::{db::DefDatabase, diagnostics::*, nameres::LocalModuleId, AstId}; #[derive(Debug, PartialEq, Eq)] - pub(super) enum DefDiagnostic { - UnresolvedModule { - module: LocalModuleId, - declaration: AstId, - candidate: String, - }, + enum DiagnosticKind { + UnresolvedModule { declaration: AstId, candidate: String }, + + UnresolvedExternCrate { ast: AstId }, + + UnresolvedImport { ast: AstId, index: usize }, + } + + #[derive(Debug, PartialEq, Eq)] + pub(super) struct DefDiagnostic { + in_module: LocalModuleId, + kind: DiagnosticKind, } impl DefDiagnostic { + pub(super) fn unresolved_module( + container: LocalModuleId, + declaration: AstId, + candidate: String, + ) -> Self { + Self { + in_module: container, + kind: DiagnosticKind::UnresolvedModule { declaration, candidate }, + } + } + + pub(super) fn unresolved_extern_crate( + container: LocalModuleId, + declaration: AstId, + ) -> Self { + Self { + in_module: container, + kind: DiagnosticKind::UnresolvedExternCrate { ast: declaration }, + } + } + + pub(super) fn unresolved_import( + container: LocalModuleId, + ast: AstId, + index: usize, + ) -> Self { + Self { in_module: container, kind: DiagnosticKind::UnresolvedImport { ast, index } } + } + pub(super) fn add_to( &self, db: &dyn DefDatabase, target_module: LocalModuleId, sink: &mut DiagnosticSink, ) { - match self { - DefDiagnostic::UnresolvedModule { module, declaration, candidate } => { - if *module != target_module { - return; - } + if self.in_module != target_module { + return; + } + + match &self.kind { + DiagnosticKind::UnresolvedModule { declaration, candidate } => { let decl = declaration.to_node(db.upcast()); sink.push(UnresolvedModule { file: declaration.file_id, @@ -320,6 +359,36 @@ mod diagnostics { candidate: candidate.clone(), }) } + + DiagnosticKind::UnresolvedExternCrate { ast } => { + let item = ast.to_node(db.upcast()); + sink.push(UnresolvedExternCrate { + file: ast.file_id, + item: AstPtr::new(&item), + }); + } + + DiagnosticKind::UnresolvedImport { ast, index } => { + let use_item = ast.to_node(db.upcast()); + let hygiene = Hygiene::new(db.upcast(), ast.file_id); + let mut cur = 0; + let mut tree = None; + ModPath::expand_use_item( + InFile::new(ast.file_id, use_item), + &hygiene, + |_mod_path, use_tree, _is_glob, _alias| { + if cur == *index { + tree = Some(use_tree.clone()); + } + + cur += 1; + }, + ); + + if let Some(tree) = tree { + sink.push(UnresolvedImport { file: ast.file_id, node: AstPtr::new(&tree) }); + } + } } } } 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') 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') 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 -------- crates/hir_def/src/test_db.rs | 42 ++++++++ 4 files changed, 150 insertions(+), 38 deletions(-) create mode 100644 crates/hir_def/src/nameres/tests/diagnostics.rs (limited to 'crates/hir_def/src') 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( diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index 42a762936..fb1d3c974 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -5,9 +5,15 @@ use std::{ sync::{Arc, Mutex}, }; +use base_db::SourceDatabase; use base_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, Upcast}; use hir_expand::db::AstDatabase; +use hir_expand::diagnostics::Diagnostic; +use hir_expand::diagnostics::DiagnosticSinkBuilder; +use rustc_hash::FxHashMap; use rustc_hash::FxHashSet; +use syntax::TextRange; +use test_utils::extract_annotations; use crate::db::DefDatabase; @@ -98,4 +104,40 @@ impl TestDB { }) .collect() } + + pub fn extract_annotations(&self) -> FxHashMap> { + let mut files = Vec::new(); + let crate_graph = self.crate_graph(); + for krate in crate_graph.iter() { + let crate_def_map = self.crate_def_map(krate); + for (module_id, _) in crate_def_map.modules.iter() { + let file_id = crate_def_map[module_id].origin.file_id(); + files.extend(file_id) + } + } + assert!(!files.is_empty()); + files + .into_iter() + .filter_map(|file_id| { + let text = self.file_text(file_id); + let annotations = extract_annotations(&text); + if annotations.is_empty() { + return None; + } + Some((file_id, annotations)) + }) + .collect() + } + + pub fn diagnostics(&self, mut cb: F) { + let crate_graph = self.crate_graph(); + for krate in crate_graph.iter() { + let crate_def_map = self.crate_def_map(krate); + + let mut sink = DiagnosticSinkBuilder::new().build(&mut cb); + for (module_id, _) in crate_def_map.modules.iter() { + crate_def_map.add_diagnostics(self, module_id, &mut sink); + } + } + } } -- 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') 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