Add new lint: std_wildcard_imports

Author: alanbondarun

CHANGELOG.md

@@ -6296,6 +6296,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

@@ -779,6 +779,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::visibility::PUB_WITHOUT_SHORTHAND_INFO,
     crate::visibility::PUB_WITH_SHORTHAND_INFO,
     crate::wildcard_imports::ENUM_GLOB_USE_INFO,
+    crate::wildcard_imports::STD_WILDCARD_IMPORTS_INFO,
     crate::wildcard_imports::WILDCARD_IMPORTS_INFO,
     crate::write::PRINTLN_EMPTY_STRING_INFO,
     crate::write::PRINT_LITERAL_INFO,

clippy_lints/src/wildcard_imports.rs

@@ -2,15 +2,15 @@ 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 rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{Item, ItemKind, PathSegment, UseKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::ty;
 use rustc_session::impl_lint_pass;
-use rustc_span::BytePos;
-use rustc_span::symbol::kw;
+use rustc_span::symbol::{STDLIB_STABLE_CRATES, Symbol, kw};
+use rustc_span::{BytePos, Span, sym};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -100,6 +100,45 @@ declare_clippy_lint! {
     "lint `use _::*` statements"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for wildcard imports `use _::*` from the standard library crates.
+    ///
+    /// ### Why is this bad?
+    /// Wildcard imports from the standard library crates can lead to breakages due to name
+    /// resolution ambiguities when the standard library introduces new items with the same names
+    /// as locally defined items.
+    ///
+    /// ### 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 imports.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// use foo::bar;
+    /// use std::rc::*;
+    ///
+    /// # mod foo { pub fn bar() {} }
+    /// bar();
+    /// let _ = Rc::new(5);
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// use foo::bar;
+    /// use std::rc::Rc;
+    ///
+    /// # mod foo { pub fn bar() {} }
+    /// bar();
+    /// let _ = Rc::new(5);
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub STD_WILDCARD_IMPORTS,
+    pedantic,
+    "lint `use _::*` from the standard library crates"
+}
+
 pub struct WildcardImports {
     warn_on_all: bool,
     allowed_segments: FxHashSet<String>,
@@ -114,7 +153,7 @@ impl WildcardImports {
     }
 }
 
-impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]);
+impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS, STD_WILDCARD_IMPORTS]);
 
 impl LateLintPass<'_> for WildcardImports {
     fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
@@ -129,51 +168,33 @@ impl LateLintPass<'_> for WildcardImports {
             return;
         }
         if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind
-            && (self.warn_on_all || !self.check_exceptions(cx, item, use_path.segments))
+            && (self.warn_on_all
+                || !self.check_exceptions(cx, item, use_path.segments)
+                || (!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, 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. Enums are in the value namespace.
             let (lint, message) = if let Some(Res::Def(DefKind::Enum, _)) = use_path.res.value_ns {
-                (ENUM_GLOB_USE, "usage of wildcard import for enum variants")
+                (
+                    ENUM_GLOB_USE,
+                    String::from("usage of wildcard import for enum variants"),
+                )
+            } else if is_std_import(use_path.segments) {
+                (
+                    STD_WILDCARD_IMPORTS,
+                    format!("usage of wildcard import from `{}`", use_path.segments[0].ident),
+                )
             } else {
-                (WILDCARD_IMPORTS, "usage of wildcard import")
+                (WILDCARD_IMPORTS, String::from("usage of wildcard import"))
             };
 
             span_lint_and_sugg(cx, lint, span, message, "try", sugg, applicability);
@@ -201,10 +222,62 @@ fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool {
     segments.len() == 1 && segments[0].ident.name == kw::Super
 }
 
+// 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
+}
+
 // Allow skipping imports containing user configured segments,
 // i.e. "...::utils::...::*" if user put `allowed-wildcard-imports = ["utils"]` in `Clippy.toml`
 fn is_allowed_via_config(segments: &[PathSegment<'_>], allowed_segments: &FxHashSet<String>) -> bool {
     // segment matching need to be exact instead of using 'contains', in case user unintentionally put
     // a single character in the config thus skipping most of the warnings.
     segments.iter().any(|seg| allowed_segments.contains(seg.ident.as_str()))
 }
+
+// Returns the entire span for a given glob import statement, including the `*` symbol.
+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.
+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() {