Skip to content

Commit d8391e4

Browse files
lengyijunlengyijun
committed
capture
1 parent aaf16b5 commit d8391e4

File tree

3 files changed

+76
-59
lines changed

3 files changed

+76
-59
lines changed

‎clippy_lints/src/same_name_method.rs

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,18 @@ use rustc_hir::def::{DefKind, Res};
44
use rustc_hir::{Crate, Impl, ItemKind, Node, Path, QPath, TraitRef, TyKind};
55
use rustc_lint::{LateContext, LateLintPass};
66
use rustc_middle::ty::AssocKind;
7-
use rustc_session::{declare_tool_lint, impl_lint_pass};
7+
use rustc_session::{declare_tool_lint, declare_lint_pass};
88
use rustc_span::symbol::SymbolStr;
99
use rustc_span::Span;
1010
use std::collections::{BTreeMap, BTreeSet};
1111

1212
declare_clippy_lint! {
1313
/// ### What it does
14-
/// it lints if a struct has two method with same time:
15-
/// one from trait, another not from trait.
16-
///
14+
/// It lints if a struct has two method with same time:
15+
/// one from a trait, another not from trait.
1716
///
1817
/// ### Why is this bad?
19-
/// confusing.
18+
/// Confusing.
2019
///
2120
/// ### Example
2221
/// ```rust
@@ -33,16 +32,13 @@ declare_clippy_lint! {
3332
/// impl S {
3433
/// fn foo(&self) {}
3534
/// }
36-
///
3735
/// ```
3836
pub SAME_NAME_METHOD,
3937
style,
4038
"two method with same name"
4139
}
4240

43-
impl_lint_pass!(SameNameMethod => [SAME_NAME_METHOD]);
44-
45-
pub struct SameNameMethod;
41+
declare_lint_pass!(SameNameMethod => [SAME_NAME_METHOD]);
4642

4743
struct S {
4844
impl_methods: BTreeMap<SymbolStr, Span>,
@@ -76,13 +72,13 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod {
7672
match of_trait {
7773
Some(trait_ref) => {
7874
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);
75+
if let Some(Node::TraitRef(TraitRef { path, .. })) =
76+
cx.tcx.hir().find(trait_ref.hir_ref_id);
8077
if let Res::Def(DefKind::Trait, did) = path.res;
8178
then{
8279
// FIXME: if
8380
// `rustc_middle::ty::assoc::AssocItems::items` is public,
84-
// we can iterate the keys, it's more
85-
// efficient
81+
// we can iterate its keys, which's more efficient
8682
cx.tcx
8783
.associated_items(did)
8884
.in_definition_order()
@@ -96,48 +92,36 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod {
9692
}
9793
};
9894

99-
for impl_item_ref in (*items).iter().filter(|impl_item_ref| {
100-
matches!(impl_item_ref.kind, rustc_hir::AssocItemKind::Fn { .. })
101-
}) {
102-
let method_name = impl_item_ref.ident.as_str();
103-
methods_in_trait.remove(&method_name);
104-
let trait_span = impl_item_ref.span;
95+
let mut check_trait_method = |method_name: SymbolStr, trait_method_span| {
10596
if let Some(impl_span) = s.impl_methods.get(&method_name) {
10697
span_lint_and_then(
10798
cx,
10899
SAME_NAME_METHOD,
109100
*impl_span,
110101
"binding's name is same to an existing binding",
111102
|diag| {
112-
diag.span_note(trait_span, "existing binding defined here");
103+
diag.span_note(trait_method_span, "existing binding defined here");
113104
},
114105
);
115106
}
116107
if let Some(v) = s.trait_methods.get_mut(&method_name) {
117-
v.push(trait_span);
108+
v.push(trait_method_span);
118109
} else {
119-
s.trait_methods.insert(method_name, vec![trait_span]);
110+
s.trait_methods.insert(method_name, vec![trait_method_span]);
120111
}
112+
};
113+
114+
for impl_item_ref in (*items).iter().filter(|impl_item_ref| {
115+
matches!(impl_item_ref.kind, rustc_hir::AssocItemKind::Fn { .. })
116+
}) {
117+
let method_name = impl_item_ref.ident.as_str();
118+
methods_in_trait.remove(&method_name);
119+
check_trait_method(method_name, impl_item_ref.span);
121120
}
122121

123122
let trait_span = item.span;
124123
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");
133-
},
134-
);
135-
}
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-
}
124+
check_trait_method(method_name, trait_span);
141125
}
142126
},
143127
None => {
@@ -154,6 +138,7 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod {
154138
"binding's name is same to an existing binding",
155139
|diag| {
156140
// TODO should we `span_note` on every trait?
141+
// iterate on trait_spans?
157142
diag.span_note(trait_spans[0], "existing binding defined here");
158143
},
159144
);

‎tests/ui/same_name_method.rs

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![allow(dead_code)]
22

3-
mod test_ok {
3+
mod test_simple_case {
44
trait T1 {
55
fn foo() {}
66
}
@@ -25,54 +25,74 @@ mod test_derive {
2525
}
2626
}
2727

28-
mod not_check_two_trait_method {
28+
mod with_generic {
2929
trait T1 {
3030
fn foo();
3131
}
3232

33-
trait T2 {
34-
fn foo();
35-
}
36-
37-
struct S;
33+
struct S<U>(U);
3834

39-
impl T1 for S {
35+
impl<U> S<U> {
4036
fn foo() {}
4137
}
4238

43-
impl T2 for S {
39+
impl<U: Copy> T1 for S<U> {
4440
fn foo() {}
4541
}
4642
}
4743

48-
mod with_generic {
44+
mod default_method {
4945
trait T1 {
50-
fn foo();
46+
fn foo() {}
5147
}
5248

53-
struct S<U>(U);
49+
struct S;
5450

55-
impl<U> S<U> {
51+
impl S {
5652
fn foo() {}
5753
}
5854

59-
impl<U: Copy> T1 for S<U> {
60-
fn foo() {}
61-
}
55+
impl T1 for S {}
6256
}
6357

64-
mod default_method {
58+
mod mulitply_conflicit_trait {
6559
trait T1 {
6660
fn foo() {}
6761
}
6862

63+
trait T2 {
64+
fn foo() {}
65+
}
66+
6967
struct S;
7068

7169
impl S {
7270
fn foo() {}
7371
}
7472

7573
impl T1 for S {}
74+
75+
impl T2 for S {}
76+
}
77+
78+
mod not_lint_two_trait_method {
79+
trait T1 {
80+
fn foo();
81+
}
82+
83+
trait T2 {
84+
fn foo();
85+
}
86+
87+
struct S;
88+
89+
impl T1 for S {
90+
fn foo() {}
91+
}
92+
93+
impl T2 for S {
94+
fn foo() {}
95+
}
7696
}
7797

7898
fn main() {}

‎tests/ui/same_name_method.stderr

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,37 @@ LL | fn foo() {}
1212
| ^^^^^^^^^^^
1313

1414
error: binding's name is same to an existing binding
15-
--> $DIR/same_name_method.rs:56:9
15+
--> $DIR/same_name_method.rs:36:9
1616
|
1717
LL | fn foo() {}
1818
| ^^^^^^^^^^^
1919
|
2020
note: existing binding defined here
21-
--> $DIR/same_name_method.rs:60:9
21+
--> $DIR/same_name_method.rs:40:9
2222
|
2323
LL | fn foo() {}
2424
| ^^^^^^^^^^^
2525

2626
error: binding's name is same to an existing binding
27-
--> $DIR/same_name_method.rs:72:9
27+
--> $DIR/same_name_method.rs:52:9
2828
|
2929
LL | fn foo() {}
3030
| ^^^^^^^^^^^
3131
|
3232
note: existing binding defined here
33-
--> $DIR/same_name_method.rs:75:5
33+
--> $DIR/same_name_method.rs:55:5
34+
|
35+
LL | impl T1 for S {}
36+
| ^^^^^^^^^^^^^^^^
37+
38+
error: binding's name is same to an existing binding
39+
--> $DIR/same_name_method.rs:70:9
40+
|
41+
LL | fn foo() {}
42+
| ^^^^^^^^^^^
43+
|
44+
note: existing binding defined here
45+
--> $DIR/same_name_method.rs:73:5
3446
|
3547
LL | impl T1 for S {}
3648
| ^^^^^^^^^^^^^^^^
@@ -48,5 +60,5 @@ LL | #[derive(Clone)]
4860
| ^^^^^
4961
= note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)
5062

51-
error: aborting due to 4 previous errors
63+
error: aborting due to 5 previous errors
5264

0 commit comments

Comments
 (0)