Skip to content

Commit 56d20c2

Browse files
cocoderycocodery
committed
Fix nits and add test for unnecessary_operation 
1 parent ee2354b commit 56d20c2

File tree

4 files changed

+66
-26
lines changed

4 files changed

+66
-26
lines changed

‎clippy_lints/src/no_effect.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,10 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool {
9292
return false;
9393
}
9494
let expr = peel_blocks(expr);
95-
// assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation`
96-
if is_operator_overriden(cx, expr) {
95+
96+
if is_operator_overridden(cx, expr) {
97+
// Return `true`, to prevent `check_unnecessary_operation` from
98+
// linting on this statement as well.
9799
return true;
98100
}
99101
if has_no_effect(cx, expr) {
@@ -162,18 +164,18 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool {
162164
false
163165
}
164166

165-
fn is_operator_overriden(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
166-
// It's very hard or impossable to check whether overrided operator have side-effect this lint.
167-
// So, this function assume user-defined operator is overrided with an side-effect.
167+
fn is_operator_overridden(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
168+
// It's very hard or impossable to check whether overridden operator have side-effect this lint.
169+
// So, this function assume user-defined operator is overridden with an side-effect.
168170
// The definition of user-defined structure here is ADT-type,
169171
// Althrough this will weaken the ability of this lint, less error lint-fix happen.
170172
match expr.kind {
171173
ExprKind::Binary(..) | ExprKind::Unary(..) => {
172174
// No need to check type of `lhs` and `rhs`
173-
// because if the operator is overrided, at least one operand is ADT type
175+
// because if the operator is overridden, at least one operand is ADT type
174176

175177
// reference: rust/compiler/rustc_middle/src/ty/typeck_results.rs: `is_method_call`.
176-
// use this function to check whether operator is overrided in `ExprKind::{Binary, Unary}`.
178+
// use this function to check whether operator is overridden in `ExprKind::{Binary, Unary}`.
177179
cx.typeck_results().is_method_call(expr)
178180
},
179181
_ => false,

‎tests/ui/unnecessary_operation.fixed

+19
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
)]
88
#![warn(clippy::unnecessary_operation)]
99

10+
use std::fmt::Display;
11+
use std::ops::Shl;
12+
1013
struct Tuple(i32);
1114
struct Struct {
1215
field: i32,
@@ -50,6 +53,19 @@ fn get_drop_struct() -> DropStruct {
5053
DropStruct { field: 0 }
5154
}
5255

56+
struct Cout;
57+
58+
impl<T> Shl<T> for Cout
59+
where
60+
T: Display,
61+
{
62+
type Output = Self;
63+
fn shl(self, rhs: T) -> Self::Output {
64+
println!("{}", rhs);
65+
self
66+
}
67+
}
68+
5369
fn main() {
5470
get_number();
5571
get_number();
@@ -87,4 +103,7 @@ fn main() {
87103
($($e:expr),*) => {{ $($e;)* }}
88104
}
89105
use_expr!(isize::MIN / -(one() as isize), i8::MIN / -one());
106+
107+
// Issue #11885
108+
Cout << 16;
90109
}

‎tests/ui/unnecessary_operation.rs

+19
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
)]
88
#![warn(clippy::unnecessary_operation)]
99

10+
use std::fmt::Display;
11+
use std::ops::Shl;
12+
1013
struct Tuple(i32);
1114
struct Struct {
1215
field: i32,
@@ -50,6 +53,19 @@ fn get_drop_struct() -> DropStruct {
5053
DropStruct { field: 0 }
5154
}
5255

56+
struct Cout;
57+
58+
impl<T> Shl<T> for Cout
59+
where
60+
T: Display,
61+
{
62+
type Output = Self;
63+
fn shl(self, rhs: T) -> Self::Output {
64+
println!("{}", rhs);
65+
self
66+
}
67+
}
68+
5369
fn main() {
5470
Tuple(get_number());
5571
Struct { field: get_number() };
@@ -91,4 +107,7 @@ fn main() {
91107
($($e:expr),*) => {{ $($e;)* }}
92108
}
93109
use_expr!(isize::MIN / -(one() as isize), i8::MIN / -one());
110+
111+
// Issue #11885
112+
Cout << 16;
94113
}

‎tests/ui/unnecessary_operation.stderr

+19-19
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: unnecessary operation
2-
--> $DIR/unnecessary_operation.rs:54:5
2+
--> $DIR/unnecessary_operation.rs:70:5
33
|
44
LL | Tuple(get_number());
55
| ^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
@@ -8,111 +8,111 @@ LL | Tuple(get_number());
88
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_operation)]`
99

1010
error: unnecessary operation
11-
--> $DIR/unnecessary_operation.rs:55:5
11+
--> $DIR/unnecessary_operation.rs:71:5
1212
|
1313
LL | Struct { field: get_number() };
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
1515

1616
error: unnecessary operation
17-
--> $DIR/unnecessary_operation.rs:56:5
17+
--> $DIR/unnecessary_operation.rs:72:5
1818
|
1919
LL | Struct { ..get_struct() };
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_struct();`
2121

2222
error: unnecessary operation
23-
--> $DIR/unnecessary_operation.rs:57:5
23+
--> $DIR/unnecessary_operation.rs:73:5
2424
|
2525
LL | Enum::Tuple(get_number());
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
2727

2828
error: unnecessary operation
29-
--> $DIR/unnecessary_operation.rs:58:5
29+
--> $DIR/unnecessary_operation.rs:74:5
3030
|
3131
LL | Enum::Struct { field: get_number() };
3232
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
3333

3434
error: unnecessary operation
35-
--> $DIR/unnecessary_operation.rs:59:5
35+
--> $DIR/unnecessary_operation.rs:75:5
3636
|
3737
LL | 5 + get_number();
3838
| ^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();`
3939

4040
error: unnecessary operation
41-
--> $DIR/unnecessary_operation.rs:60:5
41+
--> $DIR/unnecessary_operation.rs:76:5
4242
|
4343
LL | *&get_number();
4444
| ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
4545

4646
error: unnecessary operation
47-
--> $DIR/unnecessary_operation.rs:61:5
47+
--> $DIR/unnecessary_operation.rs:77:5
4848
|
4949
LL | &get_number();
5050
| ^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
5151

5252
error: unnecessary operation
53-
--> $DIR/unnecessary_operation.rs:62:5
53+
--> $DIR/unnecessary_operation.rs:78:5
5454
|
5555
LL | (5, 6, get_number());
5656
| ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;6;get_number();`
5757

5858
error: unnecessary operation
59-
--> $DIR/unnecessary_operation.rs:63:5
59+
--> $DIR/unnecessary_operation.rs:79:5
6060
|
6161
LL | get_number()..;
6262
| ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
6363

6464
error: unnecessary operation
65-
--> $DIR/unnecessary_operation.rs:64:5
65+
--> $DIR/unnecessary_operation.rs:80:5
6666
|
6767
LL | ..get_number();
6868
| ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
6969

7070
error: unnecessary operation
71-
--> $DIR/unnecessary_operation.rs:65:5
71+
--> $DIR/unnecessary_operation.rs:81:5
7272
|
7373
LL | 5..get_number();
7474
| ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();`
7575

7676
error: unnecessary operation
77-
--> $DIR/unnecessary_operation.rs:66:5
77+
--> $DIR/unnecessary_operation.rs:82:5
7878
|
7979
LL | [42, get_number()];
8080
| ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();`
8181

8282
error: unnecessary operation
83-
--> $DIR/unnecessary_operation.rs:67:5
83+
--> $DIR/unnecessary_operation.rs:83:5
8484
|
8585
LL | [42, 55][get_usize()];
8686
| ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42, 55].len() > get_usize());`
8787

8888
error: unnecessary operation
89-
--> $DIR/unnecessary_operation.rs:68:5
89+
--> $DIR/unnecessary_operation.rs:84:5
9090
|
9191
LL | (42, get_number()).1;
9292
| ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();`
9393

9494
error: unnecessary operation
95-
--> $DIR/unnecessary_operation.rs:69:5
95+
--> $DIR/unnecessary_operation.rs:85:5
9696
|
9797
LL | [get_number(); 55];
9898
| ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
9999

100100
error: unnecessary operation
101-
--> $DIR/unnecessary_operation.rs:70:5
101+
--> $DIR/unnecessary_operation.rs:86:5
102102
|
103103
LL | [42; 55][get_usize()];
104104
| ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42; 55].len() > get_usize());`
105105

106106
error: unnecessary operation
107-
--> $DIR/unnecessary_operation.rs:71:5
107+
--> $DIR/unnecessary_operation.rs:87:5
108108
|
109109
LL | / {
110110
LL | | get_number()
111111
LL | | };
112112
| |______^ help: statement can be reduced to: `get_number();`
113113

114114
error: unnecessary operation
115-
--> $DIR/unnecessary_operation.rs:74:5
115+
--> $DIR/unnecessary_operation.rs:90:5
116116
|
117117
LL | / FooString {
118118
LL | | s: String::from("blah"),

0 commit comments

Comments
 (0)