-use crate::utils::{in_macro, snippet_with_applicability, span_lint_and_sugg};
+use crate::utils::{in_macro, snippet, 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_hir::{
+ def::{DefKind, Res},
+ Item, ItemKind, PathSegment, UseKind,
+};
use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::{symbol::Symbol, BytePos};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::symbol::kw;
+use rustc_span::{sym, BytePos};
+
+declare_clippy_lint! {
+ /// **What it does:** Checks for `use Enum::*`.
+ ///
+ /// **Why is this bad?** It is usually better style to use the prefixed name of
+ /// an enumeration variant, rather than importing variants.
+ ///
+ /// **Known problems:** Old-style enumerations that prefix the variants are
+ /// still around.
+ ///
+ /// **Example:**
+ /// ```rust,ignore
+ /// // Bad
+ /// use std::cmp::Ordering::*;
+ /// foo(Less);
+ ///
+ /// // Good
+ /// use std::cmp::Ordering;
+ /// foo(Ordering::Less)
+ /// ```
+ pub ENUM_GLOB_USE,
+ pedantic,
+ "use items that import all variants of an enum"
+}
declare_clippy_lint! {
/// **What it does:** Checks for wildcard imports `use _::*`.
///
- /// **Why is this bad?** wildcard imports can polute the namespace. This is especially bad if
+ /// **Why is this bad?** wildcard imports can pollute the namespace. This is especially bad if
/// you try to import something through a wildcard, that already has been imported by name from
/// a different source:
///
///
/// This can lead to confusing error messages at best and to unexpected behavior at worst.
///
+ /// **Exceptions:**
+ ///
+ /// Wildcard imports are allowed from modules named `prelude`. Many crates (including the standard library)
+ /// provide modules named "prelude" specifically designed for wildcard import.
+ ///
+ /// `use super::*` is allowed in test modules. This is defined as any module with "test" in the name.
+ ///
+ /// These exceptions can be disabled using the `warn-on-all-wildcard-imports` configuration flag.
+ ///
/// **Known problems:** If macros are imported through the wildcard, this macro is not included
/// by the suggestion and has to be added by hand.
///
+ /// Applying the suggestion when explicit imports of the things imported with a glob import
+ /// exist, may result in `unused_imports` warnings.
+ ///
/// **Example:**
///
- /// Bad:
/// ```rust,ignore
+ /// // Bad
/// use crate1::*;
///
/// foo();
/// ```
///
- /// Good:
/// ```rust,ignore
+ /// // Good
/// use crate1::foo;
///
/// foo();
"lint `use _::*` statements"
}
-declare_lint_pass!(WildcardImports => [WILDCARD_IMPORTS]);
+#[derive(Default)]
+pub struct WildcardImports {
+ warn_on_all: bool,
+ test_modules_deep: u32,
+}
+
+impl WildcardImports {
+ pub fn new(warn_on_all: bool) -> Self {
+ Self {
+ warn_on_all,
+ test_modules_deep: 0,
+ }
+ }
+}
+
+impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]);
-impl LateLintPass<'_, '_> for WildcardImports {
- fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &Item<'_>) {
+impl LateLintPass<'_> for WildcardImports {
+ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+ if is_test_module_or_function(item) {
+ self.test_modules_deep = self.test_modules_deep.saturating_add(1);
+ }
if item.vis.node.is_pub() || item.vis.node.is_pub_restricted() {
return;
}
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();
+ if self.warn_on_all || !self.check_exceptions(item, use_path.segments);
+ let used_imports = cx.tcx.names_imported_by_glob_use(item.hir_id.owner);
+ 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 = snippet_with_applicability(cx, use_path.span, "..", &mut applicability);
+ let (span, braced_glob) = if import_source_snippet.is_empty() {
+ // This is a `_::{_, *}` import
+ // In this case `use_path.span` is empty and ends directly in front of the `*`,
+ // so we need to extend it by one byte.
+ (
+ use_path.span.with_hi(use_path.span.hi() + BytePos(1)),
+ true,
+ )
} else {
- (vec![], false)
- };
-
- let mut import_used_visitor = ImportsUsedVisitor {
- cx,
- wildcard_def_id: def_id,
- in_module,
- used_imports: FxHashSet::default(),
+ // In this case, the `use_path.span` ends right before the `::*`, so we need to
+ // extend it up to the `*`. Since it is hard to find the `*` in weird
+ // formattings like `use _ :: *;`, we extend it up to, but not including the
+ // `;`. In nested imports, like `use _::{inner::*, _}` there is no `;` and we
+ // can just use the end of the item span
+ let mut span = use_path.span.with_hi(item.span.hi());
+ if snippet(cx, span, "").ends_with(';') {
+ span = use_path.span.with_hi(item.span.hi() - BytePos(1));
+ }
+ (
+ span, false,
+ )
};
- 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::<Vec<_>>();
+ imports.sort();
+ if braced_glob {
+ imports.join(", ")
} else {
- (
- use_path.span.with_hi(use_path.span.hi() + BytePos(3)),
- false,
- )
- };
+ format!("{{{}}}", imports.join(", "))
+ }
+ };
- 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::<Vec<_>>();
- imports.sort();
- if braced_glob {
- imports.join(", ")
- } else {
- format!("{{{}}}", imports.join(", "))
- }
- };
+ let sugg = if braced_glob {
+ imports_string
+ } else {
+ format!("{}::{}", import_source_snippet, imports_string)
+ };
- let sugg = if import_source.is_empty() {
- imports_string
- } else {
- format!("{}::{}", import_source, imports_string)
- };
+ let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res {
+ (ENUM_GLOB_USE, "usage of wildcard import for enum variants")
+ } else {
+ (WILDCARD_IMPORTS, "usage of wildcard import")
+ };
- span_lint_and_sugg(
- cx,
- WILDCARD_IMPORTS,
- span,
- "usage of wildcard import",
- "try",
- sugg,
- applicability,
- );
- }
+ span_lint_and_sugg(
+ cx,
+ lint,
+ span,
+ message,
+ "try",
+ sugg,
+ applicability,
+ );
}
}
}
-}
-
-struct ImportsUsedVisitor<'a, 'tcx> {
- cx: &'a LateContext<'a, 'tcx>,
- wildcard_def_id: def_id::DefId,
- in_module: bool,
- used_imports: FxHashSet<Symbol>,
-}
-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;
- }
- }
- }
- }
-
- // 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,
- );
- }
- }
+ fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
+ if is_test_module_or_function(item) {
+ self.test_modules_deep = self.test_modules_deep.saturating_sub(1);
}
}
+}
- fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> {
- NestedVisitorMap::All(&self.cx.tcx.hir())
+impl WildcardImports {
+ fn check_exceptions(&self, item: &Item<'_>, segments: &[PathSegment<'_>]) -> bool {
+ in_macro(item.span)
+ || is_prelude_import(segments)
+ || (is_super_only_import(segments) && self.test_modules_deep > 0)
}
}
-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,
- }
- }
+// Allow "...prelude::..::*" imports.
+// Many crates have a prelude, and it is imported as a glob by design.
+fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool {
+ segments.iter().any(|ps| ps.ident.name == sym::prelude)
+}
- fn first_path_segment_def_id(&self, path: &Path<'_>) -> Option<DefId> {
- 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;
- }
- }
+// Allow "super::*" imports in tests.
+fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool {
+ segments.len() == 1 && segments[0].ident.name == kw::Super
+}
- Some(def_id)
- })
- }
+fn is_test_module_or_function(item: &Item<'_>) -> bool {
+ matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test")
}
projects / rust.git / blobdiff