diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 70098f31a..597552f03 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -1,14 +1,10 @@ use crate::utils::{in_macro, snippet_with_applicability, span_lint_and_sugg}; use if_chain::if_chain; -use rustc::ty::DefIdTree; -use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; -use rustc_hir::def_id::DefId; -use rustc_hir::intravisit::{walk_item, NestedVisitorMap, Visitor}; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::{symbol::Symbol, BytePos}; +use rustc_span::BytePos; declare_clippy_lint! { /// **What it does:** Checks for wildcard imports `use _::*`. @@ -59,213 +55,55 @@ impl LateLintPass<'_, '_> for WildcardImports { if_chain! { if !in_macro(item.span); if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind; - if let Some(def_id) = use_path.res.opt_def_id(); + let used_imports = cx.tcx.names_imported_by_glob_use(item.hir_id.owner_def_id()); + if !used_imports.is_empty(); // Already handled by `unused_imports` then { - let hir = cx.tcx.hir(); - let parent_id = hir.get_parent_item(item.hir_id); - let (items, in_module) = if parent_id == CRATE_HIR_ID { - let items = hir - .krate() - .module - .item_ids - .iter() - .map(|item_id| hir.get(item_id.id)) - .filter_map(|node| { - if let Node::Item(item) = node { - Some(item) - } else { - None - } - }) - .collect(); - (items, true) - } else if let Node::Item(item) = hir.get(parent_id) { - (vec![item], false) + let mut applicability = Applicability::MachineApplicable; + let import_source = snippet_with_applicability(cx, use_path.span, "..", &mut applicability); + let (span, braced_glob) = if import_source.is_empty() { + // This is a `_::{_, *}` import + ( + use_path.span.with_hi(use_path.span.hi() + BytePos(1)), + true, + ) } else { - (vec![], false) + ( + use_path.span.with_hi(use_path.span.hi() + BytePos(3)), + false, + ) }; - let mut import_used_visitor = ImportsUsedVisitor { - cx, - wildcard_def_id: def_id, - in_module, - used_imports: FxHashSet::default(), - }; - for item in items { - import_used_visitor.visit_item(item); - } - - if !import_used_visitor.used_imports.is_empty() { - let module_name = use_path - .segments + let imports_string = if used_imports.len() == 1 { + used_imports.iter().next().unwrap().to_string() + } else { + let mut imports = used_imports .iter() - .last() - .expect("path has at least one segment") - .ident - .name; - - let mut applicability = Applicability::MachineApplicable; - let import_source = snippet_with_applicability(cx, use_path.span, "..", &mut applicability); - let (span, braced_glob) = if import_source.is_empty() { - // This is a `_::{_, *}` import - // Probably it's `_::{self, *}`, in that case we don't want to suggest to - // import `self`. - // If it is something else, we also don't want to include `self` in the - // suggestion, since either it was imported through another use statement: - // ``` - // use foo::bar; - // use foo::bar::{baz, *}; - // ``` - // or it couldn't be used anywhere. - ( - use_path.span.with_hi(use_path.span.hi() + BytePos(1)), - true, - ) + .map(ToString::to_string) + .collect::>(); + imports.sort(); + if braced_glob { + imports.join(", ") } else { - ( - use_path.span.with_hi(use_path.span.hi() + BytePos(3)), - false, - ) - }; - - let imports_string = if import_used_visitor.used_imports.len() == 1 { - // We don't need to check for accidental suggesting the module name instead - // of `self` here, since if `used_imports.len() == 1`, and the only usage - // is `self`, then it's not through a `*` and if there is a `*`, it gets - // already linted by `unused_imports` of rustc. - import_used_visitor.used_imports.iter().next().unwrap().to_string() - } else { - let mut imports = import_used_visitor - .used_imports - .iter() - .filter_map(|import_name| { - if braced_glob && *import_name == module_name { - None - } else if *import_name == module_name { - Some("self".to_string()) - } else { - Some(import_name.to_string()) - } - }) - .collect::>(); - imports.sort(); - if braced_glob { - imports.join(", ") - } else { - format!("{{{}}}", imports.join(", ")) - } - }; - - let sugg = if import_source.is_empty() { - imports_string - } else { - format!("{}::{}", import_source, imports_string) - }; - - span_lint_and_sugg( - cx, - WILDCARD_IMPORTS, - span, - "usage of wildcard import", - "try", - sugg, - applicability, - ); - } - } - } - } -} - -struct ImportsUsedVisitor<'a, 'tcx> { - cx: &'a LateContext<'a, 'tcx>, - wildcard_def_id: def_id::DefId, - in_module: bool, - used_imports: FxHashSet, -} - -impl<'a, 'tcx> Visitor<'tcx> for ImportsUsedVisitor<'a, 'tcx> { - type Map = Map<'tcx>; - - fn visit_item(&mut self, item: &'tcx Item<'_>) { - match item.kind { - ItemKind::Use(..) => {}, - ItemKind::Mod(..) if self.in_module => {}, - ItemKind::Mod(..) => self.in_module = true, - _ => walk_item(self, item), - } - } - - fn visit_path(&mut self, path: &Path<'_>, _: HirId) { - if let Some(def_id) = self.first_path_segment_def_id(path) { - // Check if the function/enum/... was exported - if let Some(exports) = self.cx.tcx.module_exports(self.wildcard_def_id) { - for export in exports { - if let Some(export_def_id) = export.res.opt_def_id() { - if export_def_id == def_id { - self.used_imports.insert( - path.segments - .iter() - .next() - .expect("path has at least one segment") - .ident - .name, - ); - return; - } + format!("{{{}}}", imports.join(", ")) } - } - } + }; - // Check if it is directly in the module - if let Some(parent_def_id) = self.cx.tcx.parent(def_id) { - if self.wildcard_def_id == parent_def_id { - self.used_imports.insert( - path.segments - .iter() - .next() - .expect("path has at least one segment") - .ident - .name, - ); - } + let sugg = if import_source.is_empty() { + imports_string + } else { + format!("{}::{}", import_source, imports_string) + }; + + span_lint_and_sugg( + cx, + WILDCARD_IMPORTS, + span, + "usage of wildcard import", + "try", + sugg, + applicability, + ); } } } - - fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> { - NestedVisitorMap::All(&self.cx.tcx.hir()) - } -} - -impl ImportsUsedVisitor<'_, '_> { - fn skip_def_id(&self, def_id: DefId) -> DefId { - let def_key = self.cx.tcx.def_key(def_id); - match def_key.disambiguated_data.data { - DefPathData::Ctor => { - if let Some(def_id) = self.cx.tcx.parent(def_id) { - self.skip_def_id(def_id) - } else { - def_id - } - }, - _ => def_id, - } - } - - fn first_path_segment_def_id(&self, path: &Path<'_>) -> Option { - path.res.opt_def_id().and_then(|mut def_id| { - def_id = self.skip_def_id(def_id); - for _ in path.segments.iter().skip(1) { - def_id = self.skip_def_id(def_id); - if let Some(parent_def_id) = self.cx.tcx.parent(def_id) { - def_id = parent_def_id; - } else { - return None; - } - } - - Some(def_id) - }) - } }