Skip to content

Commit

Permalink
Auto merge of #13322 - RuairidhWilliamson:anon-trait-import, r=y21
Browse files Browse the repository at this point in the history
Unused trait imports (formerly anonymous trait import)

For #11969

I'm looking for help and feedback on implementing a new lint for suggesting `use ... as _` for traits where possible.

I have had a go at implementing this but I don't know if this is the best way to do it as I am new to clippy.

There are some edge cases I can think of where this doesn't work but have aired on the side of false negatives instead of false positives.

An example of a false negative. I couldn't figure out the best way to resolve an import from within clippy. The sub module imports MyAny so that cannot be anonymized but `use std::any::Any` could be. In this case it is not caught because `Any` and `MyAny` have the same DefId.
```rust
mod nested_mod_used_bad1 {
    use std::any::Any;
    use std::any::Any as MyAny;
    mod foo {
        use crate::nested_mod_used_bad1::MyAny;
        fn foo() {
            println!("{:?}", MyAny::type_id("foo"));
        }
    }
}
```

Any feedback is much appreciated.

-------
changelog: new lint: `unused_trait_names`
  • Loading branch information
bors committed Sep 22, 2024
2 parents 612ae35 + 739ef7b commit abdf173
Show file tree
Hide file tree
Showing 11 changed files with 747 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6063,6 +6063,7 @@ Released 2018-09-13
[`unused_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_result_ok
[`unused_rounding`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_rounding
[`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self
[`unused_trait_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_trait_names
[`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit
[`unusual_byte_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusual_byte_groupings
[`unwrap_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_in_result
Expand Down
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
* [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args)
* [`unnecessary_lazy_evaluations`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations)
* [`unnested_or_patterns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns)
* [`unused_trait_names`](https://rust-lang.github.io/rust-clippy/master/index.html#unused_trait_names)
* [`use_self`](https://rust-lang.github.io/rust-clippy/master/index.html#use_self)


Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ define_Conf! {
uninlined_format_args,
unnecessary_lazy_evaluations,
unnested_or_patterns,
unused_trait_names,
use_self,
)]
msrv: Msrv = Msrv::empty(),
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ msrv_aliases! {
1,36,0 { ITERATOR_COPIED }
1,35,0 { OPTION_COPIED, RANGE_CONTAINS }
1,34,0 { TRY_FROM }
1,33,0 { UNDERSCORE_IMPORTS }
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
1,29,0 { ITER_FLATTEN }
1,28,0 { FROM_BOOL }
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/attrs/useless_attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub(super) fn check(cx: &LateContext<'_>, item: &Item<'_>, attrs: &[Attribute])
| "module_name_repetitions"
| "single_component_path_imports"
| "disallowed_types"
| "unused_trait_names"
)
}) {
return;
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 @@ -748,6 +748,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::unused_result_ok::UNUSED_RESULT_OK_INFO,
crate::unused_rounding::UNUSED_ROUNDING_INFO,
crate::unused_self::UNUSED_SELF_INFO,
crate::unused_trait_names::UNUSED_TRAIT_NAMES_INFO,
crate::unused_unit::UNUSED_UNIT_INFO,
crate::unwrap::PANICKING_UNWRAP_INFO,
crate::unwrap::UNNECESSARY_UNWRAP_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 @@ -376,6 +376,7 @@ mod unused_peekable;
mod unused_result_ok;
mod unused_rounding;
mod unused_self;
mod unused_trait_names;
mod unused_unit;
mod unwrap;
mod unwrap_in_result;
Expand Down Expand Up @@ -942,5 +943,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
// add lints here, do not remove this comment, it's used in `new_lint`
}
94 changes: 94 additions & 0 deletions clippy_lints/src/unused_trait_names.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_from_proc_macro;
use clippy_utils::source::snippet_opt;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Item, ItemKind, UseKind};
use rustc_lint::{LateContext, LateLintPass, LintContext as _};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::Visibility;
use rustc_session::impl_lint_pass;
use rustc_span::symbol::kw;

declare_clippy_lint! {
/// ### What it does
/// Checks for `use Trait` where the Trait is only used for its methods and not referenced by a path directly.
///
/// ### Why is this bad?
/// Traits imported that aren't used directly can be imported anonymously with `use Trait as _`.
/// It is more explicit, avoids polluting the current scope with unused names and can be useful to show which imports are required for traits.
///
/// ### Example
/// ```no_run
/// use std::fmt::Write;
///
/// fn main() {
/// let mut s = String::new();
/// let _ = write!(s, "hello, world!");
/// println!("{s}");
/// }
/// ```
/// Use instead:
/// ```no_run
/// use std::fmt::Write as _;
///
/// fn main() {
/// let mut s = String::new();
/// let _ = write!(s, "hello, world!");
/// println!("{s}");
/// }
/// ```
#[clippy::version = "1.83.0"]
pub UNUSED_TRAIT_NAMES,
restriction,
"use items that import a trait but only use it anonymously"
}

pub struct UnusedTraitNames {
msrv: Msrv,
}

impl UnusedTraitNames {
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}

impl_lint_pass!(UnusedTraitNames => [UNUSED_TRAIT_NAMES]);

impl<'tcx> LateLintPass<'tcx> for UnusedTraitNames {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if self.msrv.meets(msrvs::UNDERSCORE_IMPORTS)
&& !in_external_macro(cx.sess(), item.span)
&& let ItemKind::Use(path, UseKind::Single) = item.kind
// Ignore imports that already use Underscore
&& item.ident.name != kw::Underscore
// Only check traits
&& let Some(Res::Def(DefKind::Trait, _)) = path.res.first()
&& cx.tcx.maybe_unused_trait_imports(()).contains(&item.owner_id.def_id)
// Only check this import if it is visible to its module only (no pub, pub(crate), ...)
&& let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id)
&& cx.tcx.visibility(item.owner_id.def_id) == Visibility::Restricted(module.to_def_id())
&& let Some(last_segment) = path.segments.last()
&& let Some(snip) = snippet_opt(cx, last_segment.ident.span)
&& !is_from_proc_macro(cx, &last_segment.ident)
{
let complete_span = last_segment.ident.span.to(item.ident.span);
span_lint_and_sugg(
cx,
UNUSED_TRAIT_NAMES,
complete_span,
"importing trait that is only used anonymously",
"use",
format!("{snip} as _"),
Applicability::MachineApplicable,
);
}
}

extract_msrv_attr!(LateContext);
}
Loading

0 comments on commit abdf173

Please sign in to comment.