Auto merge of #13322 - RuairidhWilliamson:anon-trait-import, r=y21

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`

Author: bors

CHANGELOG.md

@@ -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

book/src/lint_configuration.md

@@ -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)
 
 

clippy_config/src/conf.rs

@@ -563,6 +563,7 @@ define_Conf! {
         uninlined_format_args,
         unnecessary_lazy_evaluations,
         unnested_or_patterns,
+        unused_trait_names,
         use_self,
     )]
     msrv: Msrv = Msrv::empty(),

clippy_config/src/msrvs.rs

@@ -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 }

clippy_lints/src/attrs/useless_attribute.rs

@@ -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;

clippy_lints/src/declared_lints.rs

@@ -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,

clippy_lints/src/lib.rs

@@ -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;
@@ -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`
 }

clippy_lints/src/unused_trait_names.rs

@@ -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);
+}