Skip to content

Commit aaf16b5

Browse files
lengyijunlengyijun
committed
default method in trait
1 parent acd8b0d commit aaf16b5

File tree

3 files changed

+94
-53
lines changed

3 files changed

+94
-53
lines changed

‎clippy_lints/src/same_name_method.rs

Lines changed: 80 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use rustc_data_structures::fx::FxHashMap;
3-
use rustc_hir::def::Res;
4-
use rustc_hir::{Crate, Impl, ItemKind, Path, QPath, TyKind};
3+
use rustc_hir::def::{DefKind, Res};
4+
use rustc_hir::{Crate, Impl, ItemKind, Node, Path, QPath, TraitRef, TyKind};
55
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::ty::AssocKind;
67
use rustc_session::{declare_tool_lint, impl_lint_pass};
78
use rustc_span::symbol::SymbolStr;
89
use rustc_span::Span;
9-
// use rustc_hir::Node;
10-
// use rustc_hir::TraitRef;
11-
// use rustc_hir::def::DefKind;
12-
// use rustc_middle::ty::AssocKind;
13-
use std::collections::BTreeMap;
10+
use std::collections::{BTreeMap, BTreeSet};
1411

1512
declare_clippy_lint! {
1613
/// ### What it does
@@ -65,39 +62,82 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod {
6562
}) = &item.kind
6663
{
6764
if let TyKind::Path(QPath::Resolved(_, Path { res, .. })) = self_ty.kind {
65+
if !mp.contains_key(res) {
66+
mp.insert(
67+
*res,
68+
S {
69+
impl_methods: BTreeMap::new(),
70+
trait_methods: BTreeMap::new(),
71+
},
72+
);
73+
}
74+
let s = mp.get_mut(res).unwrap();
75+
6876
match of_trait {
69-
Some(_trait_ref) => {
77+
Some(trait_ref) => {
78+
let mut methods_in_trait: BTreeSet<SymbolStr> = if_chain! {
79+
if let Some(Node::TraitRef(TraitRef { path, .. })) = cx.tcx.hir().find(trait_ref.hir_ref_id);
80+
if let Res::Def(DefKind::Trait, did) = path.res;
81+
then{
82+
// FIXME: if
83+
// `rustc_middle::ty::assoc::AssocItems::items` is public,
84+
// we can iterate the keys, it's more
85+
// efficient
86+
cx.tcx
87+
.associated_items(did)
88+
.in_definition_order()
89+
.filter(|assoc_item| {
90+
matches!(assoc_item.kind, AssocKind::Fn)
91+
})
92+
.map(|assoc_item| assoc_item.ident.as_str())
93+
.collect()
94+
}else{
95+
BTreeSet::new()
96+
}
97+
};
98+
7099
for impl_item_ref in (*items).iter().filter(|impl_item_ref| {
71100
matches!(impl_item_ref.kind, rustc_hir::AssocItemKind::Fn { .. })
72101
}) {
73102
let method_name = impl_item_ref.ident.as_str();
103+
methods_in_trait.remove(&method_name);
74104
let trait_span = impl_item_ref.span;
75-
if let Some(s) = mp.get_mut(res) {
76-
if let Some(impl_span) = s.impl_methods.get(&method_name) {
77-
span_lint_and_then(
78-
cx,
79-
SAME_NAME_METHOD,
80-
*impl_span,
81-
"binding's name is same to an existing binding",
82-
|diag| {
83-
diag.span_note(trait_span, "existing binding defined here");
84-
},
85-
);
86-
}
87-
if let Some(v) = s.trait_methods.get_mut(&method_name) {
88-
v.push(trait_span);
89-
} else {
90-
s.trait_methods.insert(method_name, vec![trait_span]);
91-
}
105+
if let Some(impl_span) = s.impl_methods.get(&method_name) {
106+
span_lint_and_then(
107+
cx,
108+
SAME_NAME_METHOD,
109+
*impl_span,
110+
"binding's name is same to an existing binding",
111+
|diag| {
112+
diag.span_note(trait_span, "existing binding defined here");
113+
},
114+
);
115+
}
116+
if let Some(v) = s.trait_methods.get_mut(&method_name) {
117+
v.push(trait_span);
92118
} else {
93-
mp.insert(
94-
*res,
95-
S {
96-
impl_methods: BTreeMap::new(),
97-
trait_methods: BTreeMap::from([(method_name, vec![trait_span])]),
119+
s.trait_methods.insert(method_name, vec![trait_span]);
120+
}
121+
}
122+
123+
let trait_span = item.span;
124+
for method_name in methods_in_trait {
125+
if let Some(impl_span) = s.impl_methods.get(&method_name) {
126+
span_lint_and_then(
127+
cx,
128+
SAME_NAME_METHOD,
129+
*impl_span,
130+
"binding's name is same to an existing binding",
131+
|diag| {
132+
diag.span_note(trait_span, "existing binding defined here");
98133
},
99134
);
100135
}
136+
if let Some(v) = s.trait_methods.get_mut(&method_name) {
137+
v.push(trait_span);
138+
} else {
139+
s.trait_methods.insert(method_name, vec![trait_span]);
140+
}
101141
}
102142
},
103143
None => {
@@ -106,29 +146,19 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod {
106146
}) {
107147
let method_name = impl_item_ref.ident.as_str();
108148
let impl_span = impl_item_ref.span;
109-
if let Some(s) = mp.get_mut(res) {
110-
if let Some(trait_spans) = s.trait_methods.get(&method_name) {
111-
span_lint_and_then(
112-
cx,
113-
SAME_NAME_METHOD,
114-
impl_span,
115-
"binding's name is same to an existing binding",
116-
|diag| {
117-
// TODO should we `span_note` on every trait?
118-
diag.span_note(trait_spans[0], "existing binding defined here");
119-
},
120-
);
121-
}
122-
s.impl_methods.insert(method_name, impl_span);
123-
} else {
124-
mp.insert(
125-
*res,
126-
S {
127-
impl_methods: BTreeMap::from([(method_name, impl_span)]),
128-
trait_methods: BTreeMap::new(),
149+
if let Some(trait_spans) = s.trait_methods.get(&method_name) {
150+
span_lint_and_then(
151+
cx,
152+
SAME_NAME_METHOD,
153+
impl_span,
154+
"binding's name is same to an existing binding",
155+
|diag| {
156+
// TODO should we `span_note` on every trait?
157+
diag.span_note(trait_spans[0], "existing binding defined here");
129158
},
130159
);
131160
}
161+
s.impl_methods.insert(method_name, impl_span);
132162
}
133163
},
134164
}

‎tests/ui/same_name_method.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ mod with_generic {
6161
}
6262
}
6363

64-
mod false_negative {
64+
mod default_method {
6565
trait T1 {
6666
fn foo() {}
6767
}
@@ -72,7 +72,6 @@ mod false_negative {
7272
fn foo() {}
7373
}
7474

75-
// `same_name_method` couldn't detect implicit method
7675
impl T1 for S {}
7776
}
7877

‎tests/ui/same_name_method.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ note: existing binding defined here
2323
LL | fn foo() {}
2424
| ^^^^^^^^^^^
2525

26+
error: binding's name is same to an existing binding
27+
--> $DIR/same_name_method.rs:72:9
28+
|
29+
LL | fn foo() {}
30+
| ^^^^^^^^^^^
31+
|
32+
note: existing binding defined here
33+
--> $DIR/same_name_method.rs:75:5
34+
|
35+
LL | impl T1 for S {}
36+
| ^^^^^^^^^^^^^^^^
37+
2638
error: binding's name is same to an existing binding
2739
--> $DIR/same_name_method.rs:24:9
2840
|
@@ -36,5 +48,5 @@ LL | #[derive(Clone)]
3648
| ^^^^^
3749
= note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)
3850

39-
error: aborting due to 3 previous errors
51+
error: aborting due to 4 previous errors
4052

0 commit comments

Comments
 (0)