Add new lint: std_wildcard_imports

Author: alanbondarun

CHANGELOG.md

@@ -6289,6 +6289,7 @@ Released 2018-09-13
 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
 [`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc
 [`std_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core
+[`std_wildcard_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_wildcard_imports
 [`str_split_at_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_split_at_newline
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add

clippy_lints/src/declared_lints.rs

@@ -676,6 +676,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,
+    crate::std_wildcard_imports::STD_WILDCARD_IMPORTS_INFO,
     crate::string_patterns::MANUAL_PATTERN_CHAR_COMPARISON_INFO,
     crate::string_patterns::SINGLE_CHAR_PATTERN_INFO,
     crate::strings::STRING_ADD_INFO,

clippy_lints/src/lib.rs

@@ -345,6 +345,7 @@ mod size_of_in_element_count;
 mod size_of_ref;
 mod slow_vector_initialization;
 mod std_instead_of_core;
+mod std_wildcard_imports;
 mod string_patterns;
 mod strings;
 mod strlen_on_c_strings;
@@ -946,5 +947,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
     store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix));
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
+    store.register_late_pass(|_| Box::new(std_wildcard_imports::StdWildcardImports));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/std_wildcard_imports.rs

@@ -0,0 +1,92 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{is_prelude_import, sugg_glob_import, whole_glob_import_span};
+use rustc_errors::Applicability;
+use rustc_hir::{Item, ItemKind, PathSegment, UseKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+use rustc_span::sym;
+use rustc_span::symbol::{STDLIB_STABLE_CRATES, kw};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for wildcard imports `use _::*` from the standard library crates.
+    ///
+    /// ### Why is this bad?
+    /// Wildcard imports can pollute the namespace. This is especially bad when importing from the
+    /// standard library through wildcards:
+    ///
+    /// ```no_run
+    /// use foo::bar; // Imports a function named bar
+    /// use std::rc::*; // Does not have a function named bar initially
+    ///
+    /// # mod foo { pub fn bar() {} }
+    /// bar();
+    /// ```
+    ///
+    /// When the `std::rc` module later adds a function named `bar`, the compiler cannot decide
+    /// which function to call, causing a compilation error.
+    ///
+    /// ### Exceptions
+    /// Wildcard imports are allowed from modules whose names contain `prelude`. Many crates
+    /// (including the standard library) provide modules named "prelude" specifically designed
+    /// for wildcard import.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// use std::rc::*;
+    ///
+    /// let _ = Rc::new(5);
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// use std::rc::Rc;
+    ///
+    /// let _ = Rc::new(5);
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub STD_WILDCARD_IMPORTS,
+    style,
+    "lint `use _::*` from the standard library crates"
+}
+
+declare_lint_pass!(StdWildcardImports => [STD_WILDCARD_IMPORTS]);
+
+impl LateLintPass<'_> for StdWildcardImports {
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        if let ItemKind::Use(use_path, UseKind::Glob) = item.kind
+            && !is_prelude_import(use_path.segments)
+            && is_std_import(use_path.segments)
+            && let used_imports = cx.tcx.names_imported_by_glob_use(item.owner_id.def_id)
+            && !used_imports.is_empty() // Already handled by `unused_imports`
+            && !used_imports.contains(&kw::Underscore)
+        {
+            let mut applicability = Applicability::MachineApplicable;
+            let import_source_snippet = snippet_with_applicability(cx, use_path.span, "..", &mut applicability);
+
+            let span = whole_glob_import_span(cx, item, import_source_snippet.is_empty())
+                .expect("Not a glob import statement");
+            let sugg = sugg_glob_import(&import_source_snippet, used_imports);
+
+            span_lint_and_sugg(
+                cx,
+                STD_WILDCARD_IMPORTS,
+                span,
+                "usage of wildcard import from `std` crates",
+                "try",
+                sugg,
+                applicability,
+            );
+        }
+    }
+}
+
+// Checks for the standard libraries, including `test` crate.
+fn is_std_import(segments: &[PathSegment<'_>]) -> bool {
+    let Some(first_segment_name) = segments.first().map(|ps| ps.ident.name) else {
+        return false;
+    };
+
+    STDLIB_STABLE_CRATES.contains(&first_segment_name) || first_segment_name == sym::test
+}

clippy_lints/src/wildcard_imports.rs

@@ -1,7 +1,7 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::is_in_test;
-use clippy_utils::source::{snippet, snippet_with_applicability};
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{is_in_test, is_prelude_import, sugg_glob_import, whole_glob_import_span};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
@@ -10,7 +10,6 @@ use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::ty;
 use rustc_session::impl_lint_pass;
 use rustc_span::symbol::kw;
-use rustc_span::{BytePos, sym};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -136,38 +135,10 @@ impl LateLintPass<'_> for WildcardImports {
         {
             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 {
-                // 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
-                // formatting 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)
-            };
 
-            let mut imports: Vec<_> = used_imports.iter().map(ToString::to_string).collect();
-            let imports_string = if imports.len() == 1 {
-                imports.pop().unwrap()
-            } else if braced_glob {
-                imports.join(", ")
-            } else {
-                format!("{{{}}}", imports.join(", "))
-            };
-
-            let sugg = if braced_glob {
-                imports_string
-            } else {
-                format!("{import_source_snippet}::{imports_string}")
-            };
+            let span = whole_glob_import_span(cx, item, import_source_snippet.is_empty())
+                .expect("Not a glob import statement");
+            let sugg = sugg_glob_import(&import_source_snippet, used_imports);
 
             // Glob imports always have a single resolution.
             let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] {
@@ -184,20 +155,12 @@ impl LateLintPass<'_> for WildcardImports {
 impl WildcardImports {
     fn check_exceptions(&self, cx: &LateContext<'_>, item: &Item<'_>, segments: &[PathSegment<'_>]) -> bool {
         item.span.from_expansion()
-            || is_prelude_import(segments)
+            || is_prelude_import(segments) // Many crates have a prelude, and it is imported as a glob by design.
             || is_allowed_via_config(segments, &self.allowed_segments)
             || (is_super_only_import(segments) && is_in_test(cx.tcx, item.hir_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.as_str().contains(sym::prelude.as_str()))
-}
-
 // Allow "super::*" imports in tests.
 fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool {
     segments.len() == 1 && segments[0].ident.name == kw::Super

clippy_utils/src/lib.rs

@@ -90,7 +90,7 @@ use itertools::Itertools;
 use rustc_abi::Integer;
 use rustc_ast::ast::{self, LitKind, RangeLimits};
 use rustc_attr_data_structures::{AttributeKind, find_attr};
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
 use rustc_data_structures::packed::Pu128;
 use rustc_data_structures::unhash::UnindexMap;
 use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
@@ -104,7 +104,7 @@ use rustc_hir::{
     CoroutineKind, Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArg, GenericArgs, HirId, Impl,
     ImplItem, ImplItemKind, Item, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, OwnerNode,
     Param, Pat, PatExpr, PatExprKind, PatKind, Path, PathSegment, QPath, Stmt, StmtKind, TraitFn, TraitItem,
-    TraitItemKind, TraitRef, TyKind, UnOp, def,
+    TraitItemKind, TraitRef, TyKind, UnOp, UseKind, def,
 };
 use rustc_lexer::{TokenKind, tokenize};
 use rustc_lint::{LateContext, Level, Lint, LintContext};
@@ -121,12 +121,13 @@ use rustc_middle::ty::{
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::SourceMap;
 use rustc_span::symbol::{Ident, Symbol, kw};
-use rustc_span::{InnerSpan, Span};
+use rustc_span::{BytePos, InnerSpan, Span};
 use source::walk_span_to_context;
 use visitors::{Visitable, for_each_unconsumed_temporary};
 
 use crate::consts::{ConstEvalCtxt, Constant, mir_to_const};
 use crate::higher::Range;
+use crate::source::snippet;
 use crate::ty::{adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type};
 use crate::visitors::for_each_expr_without_closures;
 
@@ -3473,3 +3474,53 @@ pub fn desugar_await<'tcx>(expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
         None
     }
 }
+
+/// Returns true for `...prelude::...` imports.
+pub fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool {
+    segments
+        .iter()
+        .any(|ps| ps.ident.as_str().contains(sym::prelude.as_str()))
+}
+
+/// Returns the entire span for a given glob import statement, including the `*` symbol.
+pub fn whole_glob_import_span(cx: &LateContext<'_>, item: &Item<'_>, braced_glob: bool) -> Option<Span> {
+    let ItemKind::Use(use_path, UseKind::Glob) = item.kind else {
+        return None;
+    };
+
+    if braced_glob {
+        // 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.
+        Some(use_path.span.with_hi(use_path.span.hi() + BytePos(1)))
+    } else {
+        // 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
+        // formatting 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));
+        }
+        Some(span)
+    }
+}
+
+/// Generates a suggestion for a glob import using only the actually used items.
+pub fn sugg_glob_import(import_source_snippet: &str, used_imports: &FxIndexSet<Symbol>) -> String {
+    let mut imports: Vec<_> = used_imports.iter().map(ToString::to_string).collect();
+    let imports_string = if imports.len() == 1 {
+        imports.pop().unwrap()
+    } else if import_source_snippet.is_empty() {
+        imports.join(", ")
+    } else {
+        format!("{{{}}}", imports.join(", "))
+    };
+
+    if import_source_snippet.is_empty() {
+        imports_string
+    } else {
+        format!("{import_source_snippet}::{imports_string}")
+    }
+}

tests/ui/crashes/ice-11422.fixed

@@ -1,7 +1,7 @@
 #![warn(clippy::implied_bounds_in_impls)]
 
 use std::fmt::Debug;
-use std::ops::*;
+use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
 
 fn r#gen() -> impl PartialOrd + Debug {}
 //~^ implied_bounds_in_impls

tests/ui/crashes/ice-11422.rs

@@ -1,7 +1,7 @@
 #![warn(clippy::implied_bounds_in_impls)]
 
 use std::fmt::Debug;
-use std::ops::*;
+use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
 
 fn r#gen() -> impl PartialOrd + PartialEq + Debug {}
 //~^ implied_bounds_in_impls

tests/ui/enum_glob_use.fixed

@@ -1,5 +1,5 @@
 #![warn(clippy::enum_glob_use)]
-#![allow(unused)]
+#![allow(unused, clippy::std_wildcard_imports)]
 #![warn(unused_imports)]
 
 use std::cmp::Ordering::Less;

tests/ui/enum_glob_use.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::enum_glob_use)]
-#![allow(unused)]
+#![allow(unused, clippy::std_wildcard_imports)]
 #![warn(unused_imports)]
 
 use std::cmp::Ordering::*;

tests/ui/explicit_iter_loop.fixed

@@ -10,7 +10,7 @@
 )]
 
 use core::slice;
-use std::collections::*;
+use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, VecDeque};
 
 fn main() {
     let mut vec = vec![1, 2, 3, 4];

tests/ui/explicit_iter_loop.rs

@@ -10,7 +10,7 @@
 )]
 
 use core::slice;
-use std::collections::*;
+use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, VecDeque};
 
 fn main() {
     let mut vec = vec![1, 2, 3, 4];

tests/ui/for_kv_map.fixed

@@ -1,7 +1,7 @@
 #![warn(clippy::for_kv_map)]
 #![allow(clippy::used_underscore_binding)]
 
-use std::collections::*;
+use std::collections::HashMap;
 use std::rc::Rc;
 
 fn main() {

tests/ui/for_kv_map.rs

@@ -1,7 +1,7 @@
 #![warn(clippy::for_kv_map)]
 #![allow(clippy::used_underscore_binding)]
 
-use std::collections::*;
+use std::collections::HashMap;
 use std::rc::Rc;
 
 fn main() {

tests/ui/into_iter_on_ref.fixed

@@ -2,7 +2,7 @@
 #![warn(clippy::into_iter_on_ref)]
 
 struct X;
-use std::collections::*;
+use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, VecDeque};
 
 fn main() {
     for _ in &[1, 2, 3] {}

tests/ui/into_iter_on_ref.rs

@@ -2,7 +2,7 @@
 #![warn(clippy::into_iter_on_ref)]
 
 struct X;
-use std::collections::*;
+use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, VecDeque};
 
 fn main() {
     for _ in &[1, 2, 3] {}