Skip to content

Commit e3ef2e6

Browse files
tnielenstnielens
committed
should_impl_trait - pr comments round 3
1 parent d8f898b commit e3ef2e6

10 files changed

+470
-385
lines changed

‎clippy_lints/src/methods/mod.rs

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3393,7 +3393,7 @@ const FN_HEADER: hir::FnHeader = hir::FnHeader {
33933393
abi: rustc_target::spec::abi::Abi::Rust,
33943394
};
33953395

3396-
struct ShouldImplTrait {
3396+
struct ShouldImplTraitCase {
33973397
trait_name: &'static str,
33983398
method_name: &'static str,
33993399
param_count: usize,
@@ -3405,7 +3405,7 @@ struct ShouldImplTrait {
34053405
// certain methods with explicit lifetimes can't implement the equivalent trait method
34063406
lint_explicit_lifetime: bool,
34073407
}
3408-
impl ShouldImplTrait {
3408+
impl ShouldImplTraitCase {
34093409
const fn new(
34103410
trait_name: &'static str,
34113411
method_name: &'static str,
@@ -3414,8 +3414,8 @@ impl ShouldImplTrait {
34143414
self_kind: SelfKind,
34153415
output_type: OutType,
34163416
lint_explicit_lifetime: bool,
3417-
) -> ShouldImplTrait {
3418-
ShouldImplTrait {
3417+
) -> ShouldImplTraitCase {
3418+
ShouldImplTraitCase {
34193419
trait_name,
34203420
method_name,
34213421
param_count,
@@ -3440,38 +3440,38 @@ impl ShouldImplTrait {
34403440
}
34413441

34423442
#[rustfmt::skip]
3443-
const TRAIT_METHODS: [ShouldImplTrait; 30] = [
3444-
ShouldImplTrait::new("std::ops::Add", "add", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3445-
ShouldImplTrait::new("std::convert::AsMut", "as_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3446-
ShouldImplTrait::new("std::convert::AsRef", "as_ref", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3447-
ShouldImplTrait::new("std::ops::BitAnd", "bitand", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3448-
ShouldImplTrait::new("std::ops::BitOr", "bitor", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3449-
ShouldImplTrait::new("std::ops::BitXor", "bitxor", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3450-
ShouldImplTrait::new("std::borrow::Borrow", "borrow", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3451-
ShouldImplTrait::new("std::borrow::BorrowMut", "borrow_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3452-
ShouldImplTrait::new("std::clone::Clone", "clone", 1, &FN_HEADER, SelfKind::Ref, OutType::Any, true),
3453-
ShouldImplTrait::new("std::cmp::Ord", "cmp", 2, &FN_HEADER, SelfKind::Ref, OutType::Any, true),
3443+
const TRAIT_METHODS: [ShouldImplTraitCase; 30] = [
3444+
ShouldImplTraitCase::new("std::ops::Add", "add", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3445+
ShouldImplTraitCase::new("std::convert::AsMut", "as_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3446+
ShouldImplTraitCase::new("std::convert::AsRef", "as_ref", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3447+
ShouldImplTraitCase::new("std::ops::BitAnd", "bitand", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3448+
ShouldImplTraitCase::new("std::ops::BitOr", "bitor", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3449+
ShouldImplTraitCase::new("std::ops::BitXor", "bitxor", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3450+
ShouldImplTraitCase::new("std::borrow::Borrow", "borrow", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3451+
ShouldImplTraitCase::new("std::borrow::BorrowMut", "borrow_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3452+
ShouldImplTraitCase::new("std::clone::Clone", "clone", 1, &FN_HEADER, SelfKind::Ref, OutType::Any, true),
3453+
ShouldImplTraitCase::new("std::cmp::Ord", "cmp", 2, &FN_HEADER, SelfKind::Ref, OutType::Any, true),
34543454
// FIXME: default doesn't work
3455-
ShouldImplTrait::new("std::default::Default", "default", 0, &FN_HEADER, SelfKind::No, OutType::Any, true),
3456-
ShouldImplTrait::new("std::ops::Deref", "deref", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3457-
ShouldImplTrait::new("std::ops::DerefMut", "deref_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3458-
ShouldImplTrait::new("std::ops::Div", "div", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3459-
ShouldImplTrait::new("std::ops::Drop", "drop", 1, &FN_HEADER, SelfKind::RefMut, OutType::Unit, true),
3460-
ShouldImplTrait::new("std::cmp::PartialEq", "eq", 2, &FN_HEADER, SelfKind::Ref, OutType::Bool, true),
3461-
ShouldImplTrait::new("std::iter::FromIterator", "from_iter", 1, &FN_HEADER, SelfKind::No, OutType::Any, true),
3462-
ShouldImplTrait::new("std::str::FromStr", "from_str", 1, &FN_HEADER, SelfKind::No, OutType::Any, true),
3463-
ShouldImplTrait::new("std::hash::Hash", "hash", 2, &FN_HEADER, SelfKind::Ref, OutType::Unit, true),
3464-
ShouldImplTrait::new("std::ops::Index", "index", 2, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3465-
ShouldImplTrait::new("std::ops::IndexMut", "index_mut", 2, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3466-
ShouldImplTrait::new("std::iter::IntoIterator", "into_iter", 1, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3467-
ShouldImplTrait::new("std::ops::Mul", "mul", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3468-
ShouldImplTrait::new("std::ops::Neg", "neg", 1, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3469-
ShouldImplTrait::new("std::iter::Iterator", "next", 1, &FN_HEADER, SelfKind::RefMut, OutType::Any, false),
3470-
ShouldImplTrait::new("std::ops::Not", "not", 1, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3471-
ShouldImplTrait::new("std::ops::Rem", "rem", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3472-
ShouldImplTrait::new("std::ops::Shl", "shl", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3473-
ShouldImplTrait::new("std::ops::Shr", "shr", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3474-
ShouldImplTrait::new("std::ops::Sub", "sub", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3455+
ShouldImplTraitCase::new("std::default::Default", "default", 0, &FN_HEADER, SelfKind::No, OutType::Any, true),
3456+
ShouldImplTraitCase::new("std::ops::Deref", "deref", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3457+
ShouldImplTraitCase::new("std::ops::DerefMut", "deref_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3458+
ShouldImplTraitCase::new("std::ops::Div", "div", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3459+
ShouldImplTraitCase::new("std::ops::Drop", "drop", 1, &FN_HEADER, SelfKind::RefMut, OutType::Unit, true),
3460+
ShouldImplTraitCase::new("std::cmp::PartialEq", "eq", 2, &FN_HEADER, SelfKind::Ref, OutType::Bool, true),
3461+
ShouldImplTraitCase::new("std::iter::FromIterator", "from_iter", 1, &FN_HEADER, SelfKind::No, OutType::Any, true),
3462+
ShouldImplTraitCase::new("std::str::FromStr", "from_str", 1, &FN_HEADER, SelfKind::No, OutType::Any, true),
3463+
ShouldImplTraitCase::new("std::hash::Hash", "hash", 2, &FN_HEADER, SelfKind::Ref, OutType::Unit, true),
3464+
ShouldImplTraitCase::new("std::ops::Index", "index", 2, &FN_HEADER, SelfKind::Ref, OutType::Ref, true),
3465+
ShouldImplTraitCase::new("std::ops::IndexMut", "index_mut", 2, &FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
3466+
ShouldImplTraitCase::new("std::iter::IntoIterator", "into_iter", 1, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3467+
ShouldImplTraitCase::new("std::ops::Mul", "mul", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3468+
ShouldImplTraitCase::new("std::ops::Neg", "neg", 1, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3469+
ShouldImplTraitCase::new("std::iter::Iterator", "next", 1, &FN_HEADER, SelfKind::RefMut, OutType::Any, false),
3470+
ShouldImplTraitCase::new("std::ops::Not", "not", 1, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3471+
ShouldImplTraitCase::new("std::ops::Rem", "rem", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3472+
ShouldImplTraitCase::new("std::ops::Shl", "shl", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3473+
ShouldImplTraitCase::new("std::ops::Shr", "shr", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
3474+
ShouldImplTraitCase::new("std::ops::Sub", "sub", 2, &FN_HEADER, SelfKind::Value, OutType::Any, true),
34753475
];
34763476

34773477
#[rustfmt::skip]

‎tests/ui/methods.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ impl BadNew {
107107
}
108108
}
109109

110+
struct T;
111+
110112
impl Mul<T> for T {
111113
type Output = T;
112114
// No error, obviously.

‎tests/ui/methods.stderr

Lines changed: 99 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,99 @@
1-
error[E0412]: cannot find type `T` in this scope
2-
--> $DIR/methods.rs:110:10
3-
|
4-
LL | struct U;
5-
| --------- similarly named struct `U` defined here
6-
...
7-
LL | impl Mul<T> for T {
8-
| ^ help: a struct with a similar name exists: `U`
9-
10-
error[E0412]: cannot find type `T` in this scope
11-
--> $DIR/methods.rs:110:17
12-
|
13-
LL | struct U;
14-
| --------- similarly named struct `U` defined here
15-
...
16-
LL | impl Mul<T> for T {
17-
| ^ help: a struct with a similar name exists: `U`
18-
19-
error[E0412]: cannot find type `T` in this scope
20-
--> $DIR/methods.rs:111:19
21-
|
22-
LL | struct U;
23-
| --------- similarly named struct `U` defined here
24-
...
25-
LL | type Output = T;
26-
| ^ help: a struct with a similar name exists: `U`
27-
28-
error[E0412]: cannot find type `T` in this scope
29-
--> $DIR/methods.rs:113:25
30-
|
31-
LL | struct U;
32-
| --------- similarly named struct `U` defined here
33-
...
34-
LL | fn mul(self, other: T) -> T {
35-
| ^ help: a struct with a similar name exists: `U`
36-
37-
error[E0412]: cannot find type `T` in this scope
38-
--> $DIR/methods.rs:113:31
39-
|
40-
LL | struct U;
41-
| --------- similarly named struct `U` defined here
42-
...
43-
LL | fn mul(self, other: T) -> T {
44-
| ^ help: a struct with a similar name exists: `U`
45-
46-
error: aborting due to 5 previous errors
47-
48-
For more information about this error, try `rustc --explain E0412`.
1+
error: methods called `new` usually return `Self`
2+
--> $DIR/methods.rs:105:5
3+
|
4+
LL | / fn new() -> i32 {
5+
LL | | 0
6+
LL | | }
7+
| |_____^
8+
|
9+
= note: `-D clippy::new-ret-no-self` implied by `-D warnings`
10+
11+
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
12+
--> $DIR/methods.rs:126:13
13+
|
14+
LL | let _ = v.iter().filter(|&x| *x < 0).next();
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
|
17+
= note: `-D clippy::filter-next` implied by `-D warnings`
18+
= note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)`
19+
20+
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
21+
--> $DIR/methods.rs:129:13
22+
|
23+
LL | let _ = v.iter().filter(|&x| {
24+
| _____________^
25+
LL | | *x < 0
26+
LL | | }
27+
LL | | ).next();
28+
| |___________________________^
29+
30+
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
31+
--> $DIR/methods.rs:146:22
32+
|
33+
LL | let _ = v.iter().find(|&x| *x < 0).is_some();
34+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)`
35+
|
36+
= note: `-D clippy::search-is-some` implied by `-D warnings`
37+
38+
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
39+
--> $DIR/methods.rs:147:20
40+
|
41+
LL | let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less
42+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)`
43+
44+
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
45+
--> $DIR/methods.rs:148:20
46+
|
47+
LL | let _ = (0..1).find(|x| *x == 0).is_some();
48+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)`
49+
50+
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
51+
--> $DIR/methods.rs:149:22
52+
|
53+
LL | let _ = v.iter().find(|x| **x == 0).is_some();
54+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x == 0)`
55+
56+
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
57+
--> $DIR/methods.rs:152:13
58+
|
59+
LL | let _ = v.iter().find(|&x| {
60+
| _____________^
61+
LL | | *x < 0
62+
LL | | }
63+
LL | | ).is_some();
64+
| |______________________________^
65+
66+
error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`.
67+
--> $DIR/methods.rs:158:22
68+
|
69+
LL | let _ = v.iter().position(|&x| x < 0).is_some();
70+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)`
71+
72+
error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`.
73+
--> $DIR/methods.rs:161:13
74+
|
75+
LL | let _ = v.iter().position(|&x| {
76+
| _____________^
77+
LL | | x < 0
78+
LL | | }
79+
LL | | ).is_some();
80+
| |______________________________^
81+
82+
error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`.
83+
--> $DIR/methods.rs:167:22
84+
|
85+
LL | let _ = v.iter().rposition(|&x| x < 0).is_some();
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)`
87+
88+
error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`.
89+
--> $DIR/methods.rs:170:13
90+
|
91+
LL | let _ = v.iter().rposition(|&x| {
92+
| _____________^
93+
LL | | x < 0
94+
LL | | }
95+
LL | | ).is_some();
96+
| |______________________________^
97+
98+
error: aborting due to 12 previous errors
99+

0 commit comments

Comments
 (0)