Skip to content

Commit

Permalink
Use maybe_unused_trait_imports
Browse files Browse the repository at this point in the history
  • Loading branch information
@RuairidhWilliamson
RuairidhWilliamson committed Aug 30, 2024
1 parent 92f9e59 commit 20376b5
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 171 deletions.
159 changes: 7 additions & 152 deletions clippy_lints/src/anon_trait_import.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
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_hir::{Item, ItemKind, 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;
Expand Down Expand Up @@ -51,17 +45,15 @@ 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)
// 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 use if it is only visible to its module (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)
{
Expand All @@ -78,140 +70,3 @@ impl<'tcx> LateLintPass<'tcx> for AnonTraitImport {
}
}
}

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)
})
}
}
27 changes: 17 additions & 10 deletions tests/ui/anon_trait_import.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ mod nested_mod_used_good3 {
}
}

mod nested_mod_used_not_used_bad {
mod nested_mod_used_bad {
use std::any::Any as _;

fn bar() {
Expand All @@ -118,15 +118,18 @@ mod nested_mod_used_not_used_bad {
}
}

// Known limitation
// This is currently a false negative because if we find any `use crate::...` for the trait then we
// assume we can't anonymise the trait import
/*
// More complex example where `use std::any::Any;` should be anonymised but `use std::any::Any as
// MyAny;` should not as it is used by a sub module. Even though if you removed `use std::any::Any;`
// the code would still compile.
mod nested_mod_used_bad1 {
use std::any::Any;
use std::any::Any as _;

use std::any::Any as MyAny;

fn baz() {
println!("{:?}", "baz".type_id());
}

mod foo {
use crate::nested_mod_used_bad1::MyAny;

Expand All @@ -135,16 +138,20 @@ mod nested_mod_used_bad1 {
}
}
}
*/

mod nested_mod_used_bad2 {
use std::any::Any as _;
// Example of nested import with an unused import to try and trick it
mod nested_mod_used_good5 {
use std::any::Any;

mod foo {
use std::any::Any;

fn baz() {
println!("{:?}", "baz".type_id());
}

mod bar {
use crate::nested_mod_used_bad2::foo::Any;
use crate::nested_mod_used_good5::foo::Any;

fn foo() {
println!("{:?}", Any::type_id("foo"));
Expand Down
23 changes: 15 additions & 8 deletions tests/ui/anon_trait_import.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ mod nested_mod_used_good3 {
}
}

mod nested_mod_used_not_used_bad {
mod nested_mod_used_bad {
use std::any::Any;

fn bar() {
Expand All @@ -118,15 +118,18 @@ mod nested_mod_used_not_used_bad {
}
}

// Known limitation
// This is currently a false negative because if we find any `use crate::...` for the trait then we
// assume we can't anonymise the trait import
/*
// More complex example where `use std::any::Any;` should be anonymised but `use std::any::Any as
// MyAny;` should not as it is used by a sub module. Even though if you removed `use std::any::Any;`
// the code would still compile.
mod nested_mod_used_bad1 {
use std::any::Any;

use std::any::Any as MyAny;

fn baz() {
println!("{:?}", "baz".type_id());
}

mod foo {
use crate::nested_mod_used_bad1::MyAny;

Expand All @@ -135,16 +138,20 @@ mod nested_mod_used_bad1 {
}
}
}
*/

mod nested_mod_used_bad2 {
// Example of nested import with an unused import to try and trick it
mod nested_mod_used_good5 {
use std::any::Any;

mod foo {
use std::any::Any;

fn baz() {
println!("{:?}", "baz".type_id());
}

mod bar {
use crate::nested_mod_used_bad2::foo::Any;
use crate::nested_mod_used_good5::foo::Any;

fn foo() {
println!("{:?}", Any::type_id("foo"));
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/anon_trait_import.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ LL | use std::any::Any;
| ^^^ help: use: `Any as _`

error: importing trait that is only used anonymously
--> tests/ui/anon_trait_import.rs:141:19
--> tests/ui/anon_trait_import.rs:125:19
|
LL | use std::any::Any;
| ^^^ help: use: `Any as _`
Expand Down

0 comments on commit 20376b5

Please sign in to comment.