Skip to content

[WIP] Add new lint: std_wildcard_imports #14868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}
85 changes: 85 additions & 0 deletions clippy_lints/src/std_wildcard_imports.rs
Original file line number Diff line number Diff line change
@@ -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
}
50 changes: 3 additions & 47 deletions clippy_lints/src/wildcard_imports.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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] {
Expand All @@ -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
Expand Down
58 changes: 56 additions & 2 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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};
Expand All @@ -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;

Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-11422.fixed
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-11422.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/enum_glob_use.fixed
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/enum_glob_use.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::enum_glob_use)]
#![allow(unused)]
#![allow(unused, clippy::std_wildcard_imports)]
#![warn(unused_imports)]

use std::cmp::Ordering::*;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/explicit_iter_loop.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/explicit_iter_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/for_kv_map.fixed
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/for_kv_map.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/into_iter_on_ref.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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] {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/into_iter_on_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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] {}
Expand Down
Loading
Loading