diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-09-17 14:00:25 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-09-17 14:00:25 +0100 |
commit | 933fc1eb1837a902def314d400c0f3a6dd2c1283 (patch) | |
tree | 95325c5d387fbbf13ac1caca9717e7ab0f3cdede /crates/hir_def/src/nameres/collector.rs | |
parent | af92bdb8270f238fbb8af8c749bf6e9fc6a06710 (diff) | |
parent | 0dca7acf0fb65545f0c46f0c604bb15400aa6d91 (diff) |
Merge #6016
6016: Emit diagnostics for unresolved imports and extern crates r=jonas-schievink a=jonas-schievink
AFAIK, we don't have any major bugs in name resolution that would cause a lot of false positives here (except procedural attribute macro support and some rare issues around `#[path]` on module files), so these are *not* marked as experimental diagnostics right now.
I noticed that diagnostics in a file sometimes don't get displayed after opening, but require some edit to be performed. This seems like a preexisting issue though.
Co-authored-by: Jonas Schievink <[email protected]>
Diffstat (limited to 'crates/hir_def/src/nameres/collector.rs')
-rw-r--r-- | crates/hir_def/src/nameres/collector.rs | 102 |
1 files changed, 82 insertions, 20 deletions
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 3e99c8773..818008169 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs | |||
@@ -5,6 +5,7 @@ | |||
5 | 5 | ||
6 | use base_db::{CrateId, FileId, ProcMacroId}; | 6 | use base_db::{CrateId, FileId, ProcMacroId}; |
7 | use cfg::CfgOptions; | 7 | use cfg::CfgOptions; |
8 | use hir_expand::InFile; | ||
8 | use hir_expand::{ | 9 | use hir_expand::{ |
9 | ast_id_map::FileAstId, | 10 | ast_id_map::FileAstId, |
10 | builtin_derive::find_builtin_derive, | 11 | builtin_derive::find_builtin_derive, |
@@ -14,6 +15,7 @@ use hir_expand::{ | |||
14 | HirFileId, MacroCallId, MacroDefId, MacroDefKind, | 15 | HirFileId, MacroCallId, MacroDefId, MacroDefKind, |
15 | }; | 16 | }; |
16 | use rustc_hash::FxHashMap; | 17 | use rustc_hash::FxHashMap; |
18 | use rustc_hash::FxHashSet; | ||
17 | use syntax::ast; | 19 | use syntax::ast; |
18 | use test_utils::mark; | 20 | use test_utils::mark; |
19 | 21 | ||
@@ -21,9 +23,7 @@ use crate::{ | |||
21 | attr::Attrs, | 23 | attr::Attrs, |
22 | db::DefDatabase, | 24 | db::DefDatabase, |
23 | item_scope::{ImportType, PerNsGlobImports}, | 25 | item_scope::{ImportType, PerNsGlobImports}, |
24 | item_tree::{ | 26 | item_tree::{self, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind}, |
25 | self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind, | ||
26 | }, | ||
27 | nameres::{ | 27 | nameres::{ |
28 | diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint, | 28 | diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint, |
29 | BuiltinShadowMode, CrateDefMap, ModuleData, ModuleOrigin, ResolveMode, | 29 | BuiltinShadowMode, CrateDefMap, ModuleData, ModuleOrigin, ResolveMode, |
@@ -112,6 +112,12 @@ impl PartialResolvedImport { | |||
112 | } | 112 | } |
113 | 113 | ||
114 | #[derive(Clone, Debug, Eq, PartialEq)] | 114 | #[derive(Clone, Debug, Eq, PartialEq)] |
115 | enum ImportSource { | ||
116 | Import(ItemTreeId<item_tree::Import>), | ||
117 | ExternCrate(ItemTreeId<item_tree::ExternCrate>), | ||
118 | } | ||
119 | |||
120 | #[derive(Clone, Debug, Eq, PartialEq)] | ||
115 | struct Import { | 121 | struct Import { |
116 | pub path: ModPath, | 122 | pub path: ModPath, |
117 | pub alias: Option<ImportAlias>, | 123 | pub alias: Option<ImportAlias>, |
@@ -120,11 +126,12 @@ struct Import { | |||
120 | pub is_prelude: bool, | 126 | pub is_prelude: bool, |
121 | pub is_extern_crate: bool, | 127 | pub is_extern_crate: bool, |
122 | pub is_macro_use: bool, | 128 | pub is_macro_use: bool, |
129 | source: ImportSource, | ||
123 | } | 130 | } |
124 | 131 | ||
125 | impl Import { | 132 | impl Import { |
126 | fn from_use(tree: &ItemTree, id: FileItemTreeId<item_tree::Import>) -> Self { | 133 | fn from_use(tree: &ItemTree, id: ItemTreeId<item_tree::Import>) -> Self { |
127 | let it = &tree[id]; | 134 | let it = &tree[id.value]; |
128 | let visibility = &tree[it.visibility]; | 135 | let visibility = &tree[it.visibility]; |
129 | Self { | 136 | Self { |
130 | path: it.path.clone(), | 137 | path: it.path.clone(), |
@@ -134,11 +141,12 @@ impl Import { | |||
134 | is_prelude: it.is_prelude, | 141 | is_prelude: it.is_prelude, |
135 | is_extern_crate: false, | 142 | is_extern_crate: false, |
136 | is_macro_use: false, | 143 | is_macro_use: false, |
144 | source: ImportSource::Import(id), | ||
137 | } | 145 | } |
138 | } | 146 | } |
139 | 147 | ||
140 | fn from_extern_crate(tree: &ItemTree, id: FileItemTreeId<item_tree::ExternCrate>) -> Self { | 148 | fn from_extern_crate(tree: &ItemTree, id: ItemTreeId<item_tree::ExternCrate>) -> Self { |
141 | let it = &tree[id]; | 149 | let it = &tree[id.value]; |
142 | let visibility = &tree[it.visibility]; | 150 | let visibility = &tree[it.visibility]; |
143 | Self { | 151 | Self { |
144 | path: it.path.clone(), | 152 | path: it.path.clone(), |
@@ -148,6 +156,7 @@ impl Import { | |||
148 | is_prelude: false, | 156 | is_prelude: false, |
149 | is_extern_crate: true, | 157 | is_extern_crate: true, |
150 | is_macro_use: it.is_macro_use, | 158 | is_macro_use: it.is_macro_use, |
159 | source: ImportSource::ExternCrate(id), | ||
151 | } | 160 | } |
152 | } | 161 | } |
153 | } | 162 | } |
@@ -245,9 +254,10 @@ impl DefCollector<'_> { | |||
245 | 254 | ||
246 | let unresolved_imports = std::mem::replace(&mut self.unresolved_imports, Vec::new()); | 255 | let unresolved_imports = std::mem::replace(&mut self.unresolved_imports, Vec::new()); |
247 | // show unresolved imports in completion, etc | 256 | // show unresolved imports in completion, etc |
248 | for directive in unresolved_imports { | 257 | for directive in &unresolved_imports { |
249 | self.record_resolved_import(&directive) | 258 | self.record_resolved_import(directive) |
250 | } | 259 | } |
260 | self.unresolved_imports = unresolved_imports; | ||
251 | 261 | ||
252 | // Record proc-macros | 262 | // Record proc-macros |
253 | self.collect_proc_macro(); | 263 | self.collect_proc_macro(); |
@@ -420,7 +430,11 @@ impl DefCollector<'_> { | |||
420 | .as_ident() | 430 | .as_ident() |
421 | .expect("extern crate should have been desugared to one-element path"), | 431 | .expect("extern crate should have been desugared to one-element path"), |
422 | ); | 432 | ); |
423 | PartialResolvedImport::Resolved(res) | 433 | if res.is_none() { |
434 | PartialResolvedImport::Unresolved | ||
435 | } else { | ||
436 | PartialResolvedImport::Resolved(res) | ||
437 | } | ||
424 | } else { | 438 | } else { |
425 | let res = self.def_map.resolve_path_fp_with_macro( | 439 | let res = self.def_map.resolve_path_fp_with_macro( |
426 | self.db, | 440 | self.db, |
@@ -774,7 +788,51 @@ impl DefCollector<'_> { | |||
774 | .collect(item_tree.top_level_items()); | 788 | .collect(item_tree.top_level_items()); |
775 | } | 789 | } |
776 | 790 | ||
777 | fn finish(self) -> CrateDefMap { | 791 | fn finish(mut self) -> CrateDefMap { |
792 | // Emit diagnostics for all remaining unresolved imports. | ||
793 | |||
794 | // We'd like to avoid emitting a diagnostics avalanche when some `extern crate` doesn't | ||
795 | // resolve. We first emit diagnostics for unresolved extern crates and collect the missing | ||
796 | // crate names. Then we emit diagnostics for unresolved imports, but only if the import | ||
797 | // doesn't start with an unresolved crate's name. Due to renaming and reexports, this is a | ||
798 | // heuristic, but it works in practice. | ||
799 | let mut diagnosed_extern_crates = FxHashSet::default(); | ||
800 | for directive in &self.unresolved_imports { | ||
801 | if let ImportSource::ExternCrate(krate) = directive.import.source { | ||
802 | let item_tree = self.db.item_tree(krate.file_id); | ||
803 | let extern_crate = &item_tree[krate.value]; | ||
804 | |||
805 | diagnosed_extern_crates.insert(extern_crate.path.segments[0].clone()); | ||
806 | |||
807 | self.def_map.diagnostics.push(DefDiagnostic::unresolved_extern_crate( | ||
808 | directive.module_id, | ||
809 | InFile::new(krate.file_id, extern_crate.ast_id), | ||
810 | )); | ||
811 | } | ||
812 | } | ||
813 | |||
814 | for directive in &self.unresolved_imports { | ||
815 | if let ImportSource::Import(import) = &directive.import.source { | ||
816 | let item_tree = self.db.item_tree(import.file_id); | ||
817 | let import_data = &item_tree[import.value]; | ||
818 | |||
819 | match (import_data.path.segments.first(), &import_data.path.kind) { | ||
820 | (Some(krate), PathKind::Plain) | (Some(krate), PathKind::Abs) => { | ||
821 | if diagnosed_extern_crates.contains(krate) { | ||
822 | continue; | ||
823 | } | ||
824 | } | ||
825 | _ => {} | ||
826 | } | ||
827 | |||
828 | self.def_map.diagnostics.push(DefDiagnostic::unresolved_import( | ||
829 | directive.module_id, | ||
830 | InFile::new(import.file_id, import_data.ast_id), | ||
831 | import_data.index, | ||
832 | )); | ||
833 | } | ||
834 | } | ||
835 | |||
778 | self.def_map | 836 | self.def_map |
779 | } | 837 | } |
780 | } | 838 | } |
@@ -830,14 +888,20 @@ impl ModCollector<'_, '_> { | |||
830 | ModItem::Import(import_id) => { | 888 | ModItem::Import(import_id) => { |
831 | self.def_collector.unresolved_imports.push(ImportDirective { | 889 | self.def_collector.unresolved_imports.push(ImportDirective { |
832 | module_id: self.module_id, | 890 | module_id: self.module_id, |
833 | import: Import::from_use(&self.item_tree, import_id), | 891 | import: Import::from_use( |
892 | &self.item_tree, | ||
893 | InFile::new(self.file_id, import_id), | ||
894 | ), | ||
834 | status: PartialResolvedImport::Unresolved, | 895 | status: PartialResolvedImport::Unresolved, |
835 | }) | 896 | }) |
836 | } | 897 | } |
837 | ModItem::ExternCrate(import_id) => { | 898 | ModItem::ExternCrate(import_id) => { |
838 | self.def_collector.unresolved_imports.push(ImportDirective { | 899 | self.def_collector.unresolved_imports.push(ImportDirective { |
839 | module_id: self.module_id, | 900 | module_id: self.module_id, |
840 | import: Import::from_extern_crate(&self.item_tree, import_id), | 901 | import: Import::from_extern_crate( |
902 | &self.item_tree, | ||
903 | InFile::new(self.file_id, import_id), | ||
904 | ), | ||
841 | status: PartialResolvedImport::Unresolved, | 905 | status: PartialResolvedImport::Unresolved, |
842 | }) | 906 | }) |
843 | } | 907 | } |
@@ -1051,13 +1115,11 @@ impl ModCollector<'_, '_> { | |||
1051 | self.import_all_legacy_macros(module_id); | 1115 | self.import_all_legacy_macros(module_id); |
1052 | } | 1116 | } |
1053 | } | 1117 | } |
1054 | Err(candidate) => self.def_collector.def_map.diagnostics.push( | 1118 | Err(candidate) => { |
1055 | DefDiagnostic::UnresolvedModule { | 1119 | self.def_collector.def_map.diagnostics.push( |
1056 | module: self.module_id, | 1120 | DefDiagnostic::unresolved_module(self.module_id, ast_id, candidate), |
1057 | declaration: ast_id, | 1121 | ); |
1058 | candidate, | 1122 | } |
1059 | }, | ||
1060 | ), | ||
1061 | }; | 1123 | }; |
1062 | } | 1124 | } |
1063 | } | 1125 | } |