Skip to content

Commit 3e012b0

Browse files
sjwang05sjwang05
committed
Use check_item and check_impl_item 
1 parent d7ef001 commit 3e012b0

File tree

3 files changed

+144
-99
lines changed

3 files changed

+144
-99
lines changed

‎clippy_lints/src/ambiguous_method_names.rs

Lines changed: 41 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
use clippy_utils::diagnostics::span_lint_and_note;
22
use clippy_utils::is_trait_impl_item;
3-
use hir::intravisit::FnKind;
4-
use hir::{Body, FnDecl};
5-
use rustc_data_structures::fx::FxHashMap;
3+
use clippy_utils::ty::implements_trait;
4+
use hir::{ImplItem, Item};
65
use rustc_hir as hir;
76
use rustc_lint::{LateContext, LateLintPass};
8-
use rustc_middle::ty::Ty;
97
use rustc_session::{declare_tool_lint, impl_lint_pass};
108
use rustc_span::def_id::LocalDefId;
119
use rustc_span::symbol::Ident;
12-
use rustc_span::{Span, Symbol};
1310

1411
declare_clippy_lint! {
1512
/// ### What it does
@@ -73,69 +70,61 @@ declare_clippy_lint! {
7370
}
7471

7572
#[derive(Clone)]
76-
pub struct AmbiguousMethodNames<'tcx> {
77-
trait_methods: FxHashMap<(Ty<'tcx>, Symbol), Span>,
78-
inherent_methods: Vec<(Ty<'tcx>, Symbol, Span)>,
73+
pub struct AmbiguousMethodNames {
74+
// Keeps track of trait methods
75+
trait_methods: Vec<(LocalDefId, Ident)>,
76+
// Keeps track of inherent methods
77+
inherent_methods: Vec<(LocalDefId, Ident)>,
7978
}
8079

81-
impl<'tcx> AmbiguousMethodNames<'tcx> {
80+
impl AmbiguousMethodNames {
8281
pub fn new() -> Self {
8382
Self {
84-
trait_methods: FxHashMap::default(),
83+
trait_methods: Vec::default(),
8584
inherent_methods: Vec::default(),
8685
}
8786
}
88-
89-
fn insert_method(&mut self, is_trait_impl: bool, ty: Ty<'tcx>, ident: Ident) {
90-
if is_trait_impl {
91-
self.trait_methods.insert((ty, ident.name), ident.span);
92-
} else {
93-
self.inherent_methods.push((ty, ident.name, ident.span));
94-
}
95-
}
9687
}
9788

98-
impl_lint_pass!(AmbiguousMethodNames<'_> => [AMBIGUOUS_METHOD_NAMES]);
99-
100-
impl<'tcx> LateLintPass<'tcx> for AmbiguousMethodNames<'tcx> {
101-
fn check_fn(
102-
&mut self,
103-
cx: &LateContext<'tcx>,
104-
kind: FnKind<'tcx>,
105-
_: &'tcx FnDecl<'_>,
106-
_: &'tcx Body<'_>,
107-
_: Span,
108-
def_id: LocalDefId,
109-
) {
110-
if let FnKind::Method(ident, _) = kind {
111-
let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id);
112-
let is_trait_impl = is_trait_impl_item(cx, hir_id);
113-
let parent_id = cx.tcx.hir().get_parent_item(hir_id);
89+
impl_lint_pass!(AmbiguousMethodNames => [AMBIGUOUS_METHOD_NAMES]);
11490

115-
// Calling type_of on a method's default impl causes an ICE
116-
if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(hir::HirId::from(parent_id))
117-
&& let hir::ItemKind::Trait(..) = item.kind
118-
{
119-
return;
91+
impl<'tcx> LateLintPass<'tcx> for AmbiguousMethodNames {
92+
// Check trait impls
93+
fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'_>) {
94+
if let hir::ItemKind::Trait(_, _, _, _, tr_items) = item.kind {
95+
for tr_item in tr_items {
96+
if let hir::AssocItemKind::Fn { .. } = tr_item.kind {
97+
self.trait_methods.push((item.owner_id.def_id, tr_item.ident))
98+
}
12099
}
100+
}
101+
}
121102

122-
let parent_ty = cx.tcx.type_of(parent_id.to_def_id()).skip_binder();
123-
self.insert_method(is_trait_impl, parent_ty, ident);
103+
// Check inherent methods
104+
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
105+
if let hir::ImplItemKind::Fn(..) = impl_item.kind {
106+
let hir_id = cx.tcx.hir().local_def_id_to_hir_id(impl_item.owner_id.def_id);
107+
if !is_trait_impl_item(cx, hir_id) {
108+
let struct_id = cx.tcx.hir().get_parent_item(hir_id);
109+
self.inherent_methods.push((struct_id.def_id, impl_item.ident))
110+
}
124111
}
125112
}
126113

127114
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
128-
for (ty, name, span) in &self.inherent_methods {
129-
let k = &(*ty, *name);
130-
if let Some(tm_span) = self.trait_methods.get(k) {
131-
span_lint_and_note(
132-
cx,
133-
AMBIGUOUS_METHOD_NAMES,
134-
*span,
135-
"ambiguous inherent method name",
136-
Some(*tm_span),
137-
"trait method defined here",
138-
);
115+
for (r#trait, ident) in &self.trait_methods {
116+
for (r#struct, inherent_ident) in &self.inherent_methods {
117+
let struct_ty = cx.tcx.type_of(r#struct.to_def_id()).skip_binder();
118+
if implements_trait(cx, struct_ty, r#trait.to_def_id(), &[]) && ident.name == inherent_ident.name {
119+
span_lint_and_note(
120+
cx,
121+
AMBIGUOUS_METHOD_NAMES,
122+
inherent_ident.span,
123+
"ambiguous inherent method name",
124+
Some(ident.span),
125+
"trait method defined here",
126+
);
127+
}
139128
}
140129
}
141130
}

‎tests/ui/ambiguous_method_names.rs

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,69 @@
11
#![allow(dead_code)]
22
#![warn(clippy::ambiguous_method_names)]
33

4-
fn main() {
5-
Base.ambiguous();
6-
Base.ambiguous();
7-
Base.also_ambiguous();
8-
9-
Base.unambiguous();
10-
11-
Other.ambiguous();
12-
Other.also_ambiguous();
13-
14-
Base.another();
15-
16-
ambiguous();
17-
18-
S::<u64>(42).f();
19-
S::<i32>(-42).f();
20-
}
4+
fn main() {}
215

226
fn ambiguous() {}
237

248
trait MyTrait {
259
fn ambiguous(&self);
2610
fn also_ambiguous(&self);
11+
fn ambiguous_default(&self) {}
2712
}
2813

2914
trait Another {
3015
fn another(&self);
3116
}
3217

33-
struct Base;
34-
35-
impl Base {
36-
fn ambiguous(&self) {
37-
println!("ambiguous struct impl");
38-
}
18+
struct A;
3919

20+
impl A {
21+
fn ambiguous(&self) {}
4022
fn also_ambiguous(&self) {}
23+
fn ambiguous_default(&self) {}
24+
fn unambiguous(&self) {}
25+
fn another(&self) {}
26+
}
4127

42-
fn unambiguous(&self) {
43-
println!("unambiguous struct impl");
44-
}
28+
impl MyTrait for A {
29+
fn ambiguous(&self) {}
30+
fn also_ambiguous(&self) {}
31+
}
4532

33+
impl Another for A {
4634
fn another(&self) {}
4735
}
4836

49-
impl MyTrait for Base {
50-
fn ambiguous(&self) {
51-
println!("ambiguous trait impl");
52-
}
37+
struct B;
38+
39+
impl B {
40+
fn ambiguous(&self) {}
41+
fn also_ambiguous(&self) {}
42+
fn ambiguous_default(&self) {}
43+
fn another(&self) {}
44+
}
5345

46+
impl MyTrait for B {
47+
fn ambiguous(&self) {}
5448
fn also_ambiguous(&self) {}
5549
}
5650

57-
impl Another for Base {
51+
impl Another for B {
5852
fn another(&self) {}
5953
}
6054

61-
struct Other;
55+
struct C;
6256

63-
impl MyTrait for Other {
64-
fn ambiguous(&self) {
65-
println!("not actually ambiguous")
66-
}
57+
impl MyTrait for C {
58+
fn ambiguous(&self) {}
59+
fn also_ambiguous(&self) {}
60+
}
6761

68-
fn also_ambiguous(&self) {
69-
println!("not actually ambiguous either")
70-
}
62+
struct D;
63+
64+
impl D {
65+
fn ambiguous(&self) {}
66+
fn also_ambiguous(&self) {}
7167
}
7268

7369
struct S<T>(T);

‎tests/ui/ambiguous_method_names.stderr

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,100 @@
11
error: ambiguous inherent method name
2-
--> $DIR/ambiguous_method_names.rs:36:8
2+
--> $DIR/ambiguous_method_names.rs:21:8
33
|
4-
LL | fn ambiguous(&self) {
4+
LL | fn ambiguous(&self) {}
55
| ^^^^^^^^^
66
|
77
note: trait method defined here
8-
--> $DIR/ambiguous_method_names.rs:50:8
8+
--> $DIR/ambiguous_method_names.rs:9:8
99
|
10-
LL | fn ambiguous(&self) {
10+
LL | fn ambiguous(&self);
1111
| ^^^^^^^^^
1212
= note: `-D clippy::ambiguous-method-names` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::ambiguous_method_names)]`
1414

1515
error: ambiguous inherent method name
1616
--> $DIR/ambiguous_method_names.rs:40:8
1717
|
18+
LL | fn ambiguous(&self) {}
19+
| ^^^^^^^^^
20+
|
21+
note: trait method defined here
22+
--> $DIR/ambiguous_method_names.rs:9:8
23+
|
24+
LL | fn ambiguous(&self);
25+
| ^^^^^^^^^
26+
27+
error: ambiguous inherent method name
28+
--> $DIR/ambiguous_method_names.rs:22:8
29+
|
1830
LL | fn also_ambiguous(&self) {}
1931
| ^^^^^^^^^^^^^^
2032
|
2133
note: trait method defined here
22-
--> $DIR/ambiguous_method_names.rs:54:8
34+
--> $DIR/ambiguous_method_names.rs:10:8
35+
|
36+
LL | fn also_ambiguous(&self);
37+
| ^^^^^^^^^^^^^^
38+
39+
error: ambiguous inherent method name
40+
--> $DIR/ambiguous_method_names.rs:41:8
2341
|
2442
LL | fn also_ambiguous(&self) {}
2543
| ^^^^^^^^^^^^^^
44+
|
45+
note: trait method defined here
46+
--> $DIR/ambiguous_method_names.rs:10:8
47+
|
48+
LL | fn also_ambiguous(&self);
49+
| ^^^^^^^^^^^^^^
50+
51+
error: ambiguous inherent method name
52+
--> $DIR/ambiguous_method_names.rs:23:8
53+
|
54+
LL | fn ambiguous_default(&self) {}
55+
| ^^^^^^^^^^^^^^^^^
56+
|
57+
note: trait method defined here
58+
--> $DIR/ambiguous_method_names.rs:11:8
59+
|
60+
LL | fn ambiguous_default(&self) {}
61+
| ^^^^^^^^^^^^^^^^^
62+
63+
error: ambiguous inherent method name
64+
--> $DIR/ambiguous_method_names.rs:42:8
65+
|
66+
LL | fn ambiguous_default(&self) {}
67+
| ^^^^^^^^^^^^^^^^^
68+
|
69+
note: trait method defined here
70+
--> $DIR/ambiguous_method_names.rs:11:8
71+
|
72+
LL | fn ambiguous_default(&self) {}
73+
| ^^^^^^^^^^^^^^^^^
2674

2775
error: ambiguous inherent method name
28-
--> $DIR/ambiguous_method_names.rs:46:8
76+
--> $DIR/ambiguous_method_names.rs:25:8
2977
|
3078
LL | fn another(&self) {}
3179
| ^^^^^^^
3280
|
3381
note: trait method defined here
34-
--> $DIR/ambiguous_method_names.rs:58:8
82+
--> $DIR/ambiguous_method_names.rs:15:8
83+
|
84+
LL | fn another(&self);
85+
| ^^^^^^^
86+
87+
error: ambiguous inherent method name
88+
--> $DIR/ambiguous_method_names.rs:43:8
3589
|
3690
LL | fn another(&self) {}
3791
| ^^^^^^^
92+
|
93+
note: trait method defined here
94+
--> $DIR/ambiguous_method_names.rs:15:8
95+
|
96+
LL | fn another(&self);
97+
| ^^^^^^^
3898

39-
error: aborting due to 3 previous errors
99+
error: aborting due to 8 previous errors
40100

0 commit comments

Comments
 (0)