wip: anon_trait_import

Author: RuairidhWilliamson

CHANGELOG.md

@@ -5241,6 +5241,7 @@ Released 2018-09-13
 [`almost_complete_letter_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_letter_range
 [`almost_complete_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range
 [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
+[`anon_trait_import`]: https://rust-lang.github.io/rust-clippy/master/index.html#anon_trait_import
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
 [`arc_with_non_send_sync`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
 [`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects

clippy_lints/src/anon_trait_import.rs

@@ -0,0 +1,217 @@
+use std::ops::ControlFlow;
+
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_opt;
+use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::def_id::DefId;
+use rustc_hir::definitions::DefPathData;
+use rustc_hir::intravisit::{walk_item, walk_mod, walk_path, Visitor};
+use rustc_hir::{HirId, Item, ItemKind, Node, Path, UseKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::hir::nested_filter;
+use rustc_middle::ty::Visibility;
+use rustc_session::declare_lint_pass;
+use rustc_span::symbol::kw;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `use Trait` where the Trait is not used directly.
+    ///
+    /// ### Why is this bad?
+    /// Traits imported that aren't used directly can be imported anonymously `use Trait as _`. It is more explicit and can be usedful 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.80.0"]
+    pub ANON_TRAIT_IMPORT,
+    nursery,
+    "use items that import a trait but only use it anonymously"
+}
+
+declare_lint_pass!(AnonTraitImport => [ANON_TRAIT_IMPORT]);
+
+impl<'tcx> LateLintPass<'tcx> for AnonTraitImport {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id);
+        if cx.tcx.visibility(item.owner_id.def_id) != Visibility::Restricted(module.to_def_id()) {
+            return;
+        }
+        if let ItemKind::Use(path, UseKind::Single) = item.kind
+            // Ignore imports that already use Underscore
+            && item.ident.name != kw::Underscore
+            && let Some(Res::Def(DefKind::Trait, def_id)) = path.res.first()
+            && let parent_id = cx.tcx.hir().get_parent_item(item.hir_id())
+            && let Node::Item(parent_item) = cx.tcx.hir_node_by_def_id(parent_id.def_id)
+            && !is_import_used_by_name_in_item(cx, *def_id, parent_item)
+            && let Some(last_segment) = path.segments.last()
+            && let Some(snip) = snippet_opt(cx, last_segment.ident.span)
+        {
+            let complete_span = last_segment.ident.span.to(item.ident.span);
+            span_lint_and_sugg(
+                cx,
+                ANON_TRAIT_IMPORT,
+                complete_span,
+                "importing trait that is only used anonymously",
+                "use",
+                format!("{snip} as _"),
+                Applicability::MaybeIncorrect,
+            );
+        }
+    }
+}
+
+fn is_import_used_by_name_in_item<'tcx>(cx: &LateContext<'tcx>, def_id: DefId, parent_item: &'tcx Item<'_>) -> bool {
+    let module = find_module(cx, parent_item);
+    let mut visitor = TraitUsedByNameVisitor {
+        cx,
+        id: def_id,
+        module,
+        module_depth: 0,
+    };
+    let result = if let ItemKind::Mod(parent_mod) = parent_item.kind {
+        visitor.visit_mod(parent_mod, parent_item.span, parent_item.hir_id())
+    } else {
+        visitor.visit_item(parent_item)
+    };
+    matches!(result, ControlFlow::Break(()))
+}
+
+fn find_module<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> HirId {
+    if let ItemKind::Mod(_) = &item.kind {
+        return item.hir_id();
+    }
+    let parent = cx.tcx.hir().get_parent_item(item.hir_id());
+    let mut node = cx.tcx.hir_node_by_def_id(parent.def_id);
+    loop {
+        match node {
+            Node::Crate(r#mod) => return r#mod.item_ids[0].hir_id(),
+            Node::Item(item) => {
+                if let ItemKind::Mod(_) = &item.kind {
+                    return item.hir_id();
+                }
+                let hir_id = item.hir_id();
+                let parent = cx.tcx.hir().get_parent_item(hir_id);
+                node = cx.tcx.hir_node_by_def_id(parent.def_id);
+            },
+            _ => panic!("not an item or crate: {node:?}"),
+        };
+    }
+}
+
+struct TraitUsedByNameVisitor<'a, 'hir> {
+    cx: &'a LateContext<'hir>,
+    id: DefId,
+    module: HirId,
+    module_depth: usize,
+}
+
+impl<'a, 'hir> Visitor<'hir> for TraitUsedByNameVisitor<'a, 'hir> {
+    type NestedFilter = nested_filter::All;
+    type Result = ControlFlow<()>;
+
+    fn nested_visit_map(&mut self) -> Self::Map {
+        self.cx.tcx.hir()
+    }
+
+    fn visit_item(&mut self, item: &'hir Item<'_>) -> Self::Result {
+        match item.kind {
+            ItemKind::Mod(m) => {
+                self.module_depth += 1;
+                let result = walk_mod(self, m, item.hir_id());
+                self.module_depth -= 1;
+                result
+            },
+            _ => walk_item(self, item),
+        }
+    }
+
+    fn visit_path(&mut self, path: &Path<'hir>, _: HirId) -> Self::Result {
+        if self.module_depth > 0 {
+            if let Some(segment) = path.segments.first()
+                && segment.ident.name == kw::Crate
+            {
+                if let Some(mod_segment) = path.segments.get(path.segments.len() - 2)
+                    && Some(self.module.owner.to_def_id()) == mod_segment.res.mod_def_id()
+                    && path.res.opt_def_id() == Some(self.id)
+                {
+                    return ControlFlow::Break(());
+                }
+            } else {
+                let mut super_count = 0;
+                for segment in path.segments {
+                    if segment.ident.name == kw::Super {
+                        super_count += 1;
+                        if super_count > self.module_depth {
+                            break;
+                        }
+                    } else {
+                        if super_count == self.module_depth
+                            && (path.res.opt_def_id() == Some(self.id) || segment.res.opt_def_id() == Some(self.id))
+                        {
+                            return ControlFlow::Break(());
+                        }
+                        break;
+                    }
+                }
+            }
+        }
+        if let Some(def_id) = self.first_path_segment_def_id(path)
+            && def_id == self.id
+            && self.module_depth == 0
+        {
+            return ControlFlow::Break(());
+        }
+        walk_path(self, path)
+    }
+}
+
+impl<'hir> TraitUsedByNameVisitor<'_, 'hir> {
+    fn skip_def_id(&self, def_id: DefId) -> DefId {
+        let def_key = self.cx.tcx.def_key(def_id);
+        match def_key.disambiguated_data.data {
+            DefPathData::Ctor => {
+                if let Some(def_id) = self.cx.tcx.opt_parent(def_id) {
+                    self.skip_def_id(def_id)
+                } else {
+                    def_id
+                }
+            },
+            _ => def_id,
+        }
+    }
+
+    fn first_path_segment_def_id(&self, path: &Path<'_>) -> Option<DefId> {
+        path.res.opt_def_id().and_then(|mut def_id| {
+            def_id = self.skip_def_id(def_id);
+            for _ in path.segments.iter().skip(1) {
+                def_id = self.skip_def_id(def_id);
+                if let Some(parent_def_id) = self.cx.tcx.opt_parent(def_id) {
+                    def_id = parent_def_id;
+                } else {
+                    return None;
+                }
+            }
+
+            Some(def_id)
+        })
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -33,6 +33,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::utils::internal_lints::unsorted_clippy_utils_paths::UNSORTED_CLIPPY_UTILS_PATHS_INFO,
     crate::absolute_paths::ABSOLUTE_PATHS_INFO,
     crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
+    crate::anon_trait_import::ANON_TRAIT_IMPORT_INFO,
     crate::approx_const::APPROX_CONSTANT_INFO,
     crate::arc_with_non_send_sync::ARC_WITH_NON_SEND_SYNC_INFO,
     crate::as_conversions::AS_CONVERSIONS_INFO,

clippy_lints/src/lib.rs

@@ -72,6 +72,7 @@ pub mod deprecated_lints;
 // begin lints modules, do not remove this comment, it’s used in `update_lints`
 mod absolute_paths;
 mod almost_complete_range;
+mod anon_trait_import;
 mod approx_const;
 mod arc_with_non_send_sync;
 mod as_conversions;
@@ -935,5 +936,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_early_pass(|| Box::new(byte_char_slices::ByteCharSlice));
     store.register_early_pass(|| Box::new(cfg_not_test::CfgNotTest));
     store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
+    store.register_late_pass(|_| Box::new(anon_trait_import::AnonTraitImport));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }