diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a98217f625a..87a46e086b39 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5fcb851dfebc..fba7faef90da 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 92eb3d7a7c59..e716dd40b8f0 100644 --- a/clippy_lints/src/lib.rs +++ b/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` } diff --git a/clippy_lints/src/std_wildcard_imports.rs b/clippy_lints/src/std_wildcard_imports.rs new file mode 100644 index 000000000000..e6a84c00a3d9 --- /dev/null +++ b/clippy_lints/src/std_wildcard_imports.rs @@ -0,0 +1,85 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::{import_span_and_sugg, is_prelude_import}; +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 (span, sugg, applicability) = import_span_and_sugg(cx, use_path, item); + + 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 +} diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 45a5dbabeb4e..bb381ed9de20 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -1,16 +1,13 @@ 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::{import_span_and_sugg, is_in_test, is_prelude_import}; use rustc_data_structures::fx::FxHashSet; -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::symbol::kw; -use rustc_span::{BytePos, sym}; declare_clippy_lint! { /// ### What it does @@ -134,40 +131,7 @@ impl LateLintPass<'_> for WildcardImports { && !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, sugg, applicability) = import_span_and_sugg(cx, use_path, item); // Glob imports always have a single resolution. let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] { @@ -184,20 +148,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 diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 3a3191765710..a0632566bb7b 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -93,6 +93,7 @@ use rustc_attr_data_structures::{AttributeKind, find_attr}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::packed::Pu128; use rustc_data_structures::unhash::UnindexMap; +use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId}; @@ -104,7 +105,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, UsePath, def, }; use rustc_lexer::{TokenKind, tokenize}; use rustc_lint::{LateContext, Level, Lint, LintContext}; @@ -121,12 +122,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, snippet_with_applicability}; 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; @@ -3471,3 +3473,55 @@ 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())) +} + +pub fn import_span_and_sugg( + cx: &LateContext<'_>, + use_path: &UsePath<'_>, + item: &Item<'_>, +) -> (Span, String, Applicability) { + let used_imports = cx.tcx.names_imported_by_glob_use(item.owner_id.def_id); + + 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}") + }; + + (span, sugg, applicability) +} diff --git a/tests/ui/crashes/ice-11422.fixed b/tests/ui/crashes/ice-11422.fixed index be07a9d8c1f9..73e5072e3630 100644 --- a/tests/ui/crashes/ice-11422.fixed +++ b/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 diff --git a/tests/ui/crashes/ice-11422.rs b/tests/ui/crashes/ice-11422.rs index 43de882caa11..9f2b38a0e029 100644 --- a/tests/ui/crashes/ice-11422.rs +++ b/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 diff --git a/tests/ui/enum_glob_use.fixed b/tests/ui/enum_glob_use.fixed index 881493f3d5ff..33fdc17eda54 100644 --- a/tests/ui/enum_glob_use.fixed +++ b/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; diff --git a/tests/ui/enum_glob_use.rs b/tests/ui/enum_glob_use.rs index a510462ecb2f..b45db229c9dc 100644 --- a/tests/ui/enum_glob_use.rs +++ b/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::*; diff --git a/tests/ui/explicit_iter_loop.fixed b/tests/ui/explicit_iter_loop.fixed index f246ec61800e..9cf181ef2bc9 100644 --- a/tests/ui/explicit_iter_loop.fixed +++ b/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]; diff --git a/tests/ui/explicit_iter_loop.rs b/tests/ui/explicit_iter_loop.rs index 35f4fb7097d8..457f3e3b33cd 100644 --- a/tests/ui/explicit_iter_loop.rs +++ b/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]; diff --git a/tests/ui/for_kv_map.fixed b/tests/ui/for_kv_map.fixed index 2a68b7443fbf..0d89e20fe58c 100644 --- a/tests/ui/for_kv_map.fixed +++ b/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() { diff --git a/tests/ui/for_kv_map.rs b/tests/ui/for_kv_map.rs index 485a97815e3c..36dc773e21c2 100644 --- a/tests/ui/for_kv_map.rs +++ b/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() { diff --git a/tests/ui/into_iter_on_ref.fixed b/tests/ui/into_iter_on_ref.fixed index 7626569fd422..b7f8bd2cf3c8 100644 --- a/tests/ui/into_iter_on_ref.fixed +++ b/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] {} diff --git a/tests/ui/into_iter_on_ref.rs b/tests/ui/into_iter_on_ref.rs index 286a62c69ce5..6dacf59b459b 100644 --- a/tests/ui/into_iter_on_ref.rs +++ b/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] {} diff --git a/tests/ui/std_wildcard_imports.fixed b/tests/ui/std_wildcard_imports.fixed new file mode 100644 index 000000000000..c0d38e91a4df --- /dev/null +++ b/tests/ui/std_wildcard_imports.fixed @@ -0,0 +1,53 @@ +//@aux-build:wildcard_imports_helper.rs + +#![feature(test)] +#![warn(clippy::std_wildcard_imports)] + +extern crate alloc; +extern crate proc_macro; +extern crate test; +extern crate wildcard_imports_helper; + +use alloc::boxed::Box; +//~^ std_wildcard_imports +use core::cell::Cell; +//~^ std_wildcard_imports +use proc_macro::is_available; +//~^ std_wildcard_imports +use std::any::type_name; +//~^ std_wildcard_imports +use std::mem::{swap, align_of}; +//~^ std_wildcard_imports +use test::bench::black_box; +//~^ std_wildcard_imports + +use crate::fn_mod::*; + +use wildcard_imports_helper::*; + +use std::io::prelude::*; +use wildcard_imports_helper::extern_prelude::v1::*; +use wildcard_imports_helper::prelude::v1::*; + +mod fn_mod { + pub fn foo() {} +} + +mod super_imports { + use super::*; + + fn test_super() { + fn_mod::foo(); + } +} + +fn main() { + foo(); + + let _ = Box::new(()); // imported from alloc::boxed module + let _ = Cell::new(()); // imported from core::cell module + let _ = is_available(); // imported from proc_macro crate + let _ = type_name::(); // imported from std::any module + let _ = align_of::(); // imported from std::mem module + black_box(()); // imported from test crate +} diff --git a/tests/ui/std_wildcard_imports.rs b/tests/ui/std_wildcard_imports.rs new file mode 100644 index 000000000000..b8e584ecaa9d --- /dev/null +++ b/tests/ui/std_wildcard_imports.rs @@ -0,0 +1,53 @@ +//@aux-build:wildcard_imports_helper.rs + +#![feature(test)] +#![warn(clippy::std_wildcard_imports)] + +extern crate alloc; +extern crate proc_macro; +extern crate test; +extern crate wildcard_imports_helper; + +use alloc::boxed::*; +//~^ std_wildcard_imports +use core::cell::*; +//~^ std_wildcard_imports +use proc_macro::*; +//~^ std_wildcard_imports +use std::any::*; +//~^ std_wildcard_imports +use std::mem::{swap, *}; +//~^ std_wildcard_imports +use test::bench::*; +//~^ std_wildcard_imports + +use crate::fn_mod::*; + +use wildcard_imports_helper::*; + +use std::io::prelude::*; +use wildcard_imports_helper::extern_prelude::v1::*; +use wildcard_imports_helper::prelude::v1::*; + +mod fn_mod { + pub fn foo() {} +} + +mod super_imports { + use super::*; + + fn test_super() { + fn_mod::foo(); + } +} + +fn main() { + foo(); + + let _ = Box::new(()); // imported from alloc::boxed module + let _ = Cell::new(()); // imported from core::cell module + let _ = is_available(); // imported from proc_macro crate + let _ = type_name::(); // imported from std::any module + let _ = align_of::(); // imported from std::mem module + black_box(()); // imported from test crate +} diff --git a/tests/ui/std_wildcard_imports.stderr b/tests/ui/std_wildcard_imports.stderr new file mode 100644 index 000000000000..3b91a31410c5 --- /dev/null +++ b/tests/ui/std_wildcard_imports.stderr @@ -0,0 +1,41 @@ +error: usage of wildcard import from `std` crates + --> tests/ui/std_wildcard_imports.rs:11:5 + | +LL | use alloc::boxed::*; + | ^^^^^^^^^^^^^^^ help: try: `alloc::boxed::Box` + | + = note: `-D clippy::std-wildcard-imports` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::std_wildcard_imports)]` + +error: usage of wildcard import from `std` crates + --> tests/ui/std_wildcard_imports.rs:13:5 + | +LL | use core::cell::*; + | ^^^^^^^^^^^^^ help: try: `core::cell::Cell` + +error: usage of wildcard import from `std` crates + --> tests/ui/std_wildcard_imports.rs:15:5 + | +LL | use proc_macro::*; + | ^^^^^^^^^^^^^ help: try: `proc_macro::is_available` + +error: usage of wildcard import from `std` crates + --> tests/ui/std_wildcard_imports.rs:17:5 + | +LL | use std::any::*; + | ^^^^^^^^^^^ help: try: `std::any::type_name` + +error: usage of wildcard import from `std` crates + --> tests/ui/std_wildcard_imports.rs:19:22 + | +LL | use std::mem::{swap, *}; + | ^ help: try: `align_of` + +error: usage of wildcard import from `std` crates + --> tests/ui/std_wildcard_imports.rs:21:5 + | +LL | use test::bench::*; + | ^^^^^^^^^^^^^^ help: try: `test::bench::black_box` + +error: aborting due to 6 previous errors +