Skip to content

Commit 8977423

Browse files
cocoderycocodery
committed
Rewrite logic of has_nontrivial_oprand.
Check whether operator is overrided with a `struct` operand. The struct here refers to `struct`, `enum`, `union`. Add and fix test for `no_effect` lint.
1 parent 6d40b10 commit 8977423

File tree

3 files changed

+86
-85
lines changed

3 files changed

+86
-85
lines changed

‎clippy_lints/src/no_effect.rs

Lines changed: 34 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use rustc_hir::{
1010
use rustc_infer::infer::TyCtxtInferExt as _;
1111
use rustc_lint::{LateContext, LateLintPass, LintContext};
1212
use rustc_middle::lint::in_external_macro;
13+
use rustc_middle::ty;
1314
use rustc_session::declare_lint_pass;
1415
use std::ops::Deref;
1516

@@ -87,8 +88,13 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect {
8788

8889
fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool {
8990
if let StmtKind::Semi(expr) = stmt.kind {
91+
// move `expr.span.from_expansion()` ahead
92+
if expr.span.from_expansion() {
93+
return false;
94+
}
95+
let expr = peel_blocks(expr);
9096
// assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation`
91-
if has_nontrivial_oprand(expr) {
97+
if has_nontrivial_oprand(cx, expr) {
9298
return true;
9399
}
94100
if has_no_effect(cx, expr) {
@@ -157,66 +163,38 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool {
157163
false
158164
}
159165

160-
fn has_nontrivial_oprand(expr: &Expr<'_>) -> bool {
161-
if expr.span.from_expansion() {
162-
return false;
163-
}
164-
return match peel_blocks(expr).kind {
165-
ExprKind::Binary(_, lhs, rhs) => !check_nontrivial_operand(lhs, rhs),
166-
_ => false,
167-
};
168-
}
169-
170-
fn check_nontrivial_operand(lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool {
171-
// It's seem that impossable to check whether operator is overrided through context of this lint,
172-
// so, this function assume user-defined binary operator is overrided with an side-effect.
173-
// The definition of user-defined structure here is `tuple`, `array`, `struct`,
174-
// it looks like a little bit simple, but useful.
175-
// Althrough this will weaken the ability of this lint,
176-
// less miss lint-fix happen.
177-
178-
// a closure to check whether expr belongs to user-defined structure
179-
let closure = |expr: &Expr<'_>| -> bool {
180-
match &expr.kind {
181-
// check whether expr is a user-defined sturcture
182-
ExprKind::Tup(..) | ExprKind::Array(..) | ExprKind::Struct(..) => true,
183-
// resolve expr's path
184-
ExprKind::Path(rustc_hir::QPath::Resolved(
185-
_,
186-
rustc_hir::Path {
187-
span: _,
188-
res,
189-
segments: _,
190-
},
191-
)) => {
192-
match res {
193-
Res::Def(defkind, _) => match defkind {
194-
// user-defined
195-
DefKind::Struct | DefKind::Ctor(_, _) => true,
196-
_ => false,
197-
},
198-
_ => false,
199-
};
200-
false
201-
},
202-
_ => false,
203-
}
204-
};
166+
fn has_nontrivial_oprand(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
167+
// It's very hard or impossable to check whether overrided operator have side-effect this lint.
168+
// So, this function assume user-defined binary operator is overrided with an side-effect.
169+
// The definition of user-defined structure here is `struct`, `enum`, `uniom`,
170+
// Althrough this will weaken the ability of this lint, less error lint-fix happen.
171+
match expr.kind {
172+
ExprKind::Binary(_, lhs, rhs) => {
173+
// get type of lhs and rhs
174+
let tyck_result = cx.typeck_results();
175+
let ty_lhs = tyck_result.expr_ty(lhs).kind();
176+
let ty_rhs = tyck_result.expr_ty(rhs).kind();
177+
// check whether lhs is a user-defined structure
178+
// only need to check lhs in fact
179+
let ud_lhs = match ty_lhs {
180+
ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(),
181+
_ => false,
182+
};
183+
let ud_rhs = match ty_rhs {
184+
ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(),
185+
_ => false,
186+
};
205187

206-
let lhs_ud = closure(lhs);
207-
let rhs_ud = closure(rhs);
208-
// one of lhs or rhs is user-defined structure
209-
if lhs_ud || rhs_ud {
210-
return false;
188+
// reference: rust/compiler/rustc_middle/src/ty/typeck_results.rs: `is_method_call`.
189+
// use this function to check whether operator is overrided in `ExprKind::Binary`.
190+
(ud_lhs || ud_rhs) && tyck_result.is_method_call(expr)
191+
},
192+
_ => false,
211193
}
212-
true
213194
}
214195

215196
fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
216-
if expr.span.from_expansion() {
217-
return false;
218-
}
219-
match peel_blocks(expr).kind {
197+
match expr.kind {
220198
ExprKind::Lit(..) | ExprKind::Closure { .. } => true,
221199
ExprKind::Path(..) => !has_drop(cx, cx.typeck_results().expr_ty(expr)),
222200
ExprKind::Index(a, b, _) | ExprKind::Binary(_, a, b) => has_no_effect(cx, a) && has_no_effect(cx, b),

‎tests/ui/no_effect.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,22 @@
99
clippy::useless_vec
1010
)]
1111

12+
use std::fmt::Display;
13+
use std::ops::Shl;
14+
15+
struct Cout;
16+
17+
impl<T> Shl<T> for Cout
18+
where
19+
T: Display,
20+
{
21+
type Output = Self;
22+
fn shl(self, rhs: T) -> Self::Output {
23+
println!("{}", rhs);
24+
self
25+
}
26+
}
27+
1228
struct Unit;
1329
struct Tuple(i32);
1430
struct Struct {
@@ -174,4 +190,11 @@ fn main() {
174190
GreetStruct1("world");
175191
GreetStruct2()("world");
176192
GreetStruct3 {}("world");
193+
194+
fn n() -> i32 {
195+
42
196+
}
197+
198+
Cout << 142;
199+
Cout << n();
177200
}

‎tests/ui/no_effect.stderr

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: statement with no effect
2-
--> $DIR/no_effect.rs:98:5
2+
--> $DIR/no_effect.rs:114:5
33
|
44
LL | 0;
55
| ^^
@@ -8,151 +8,151 @@ LL | 0;
88
= help: to override `-D warnings` add `#[allow(clippy::no_effect)]`
99

1010
error: statement with no effect
11-
--> $DIR/no_effect.rs:101:5
11+
--> $DIR/no_effect.rs:117:5
1212
|
1313
LL | s2;
1414
| ^^^
1515

1616
error: statement with no effect
17-
--> $DIR/no_effect.rs:103:5
17+
--> $DIR/no_effect.rs:119:5
1818
|
1919
LL | Unit;
2020
| ^^^^^
2121

2222
error: statement with no effect
23-
--> $DIR/no_effect.rs:105:5
23+
--> $DIR/no_effect.rs:121:5
2424
|
2525
LL | Tuple(0);
2626
| ^^^^^^^^^
2727

2828
error: statement with no effect
29-
--> $DIR/no_effect.rs:107:5
29+
--> $DIR/no_effect.rs:123:5
3030
|
3131
LL | Struct { field: 0 };
3232
| ^^^^^^^^^^^^^^^^^^^^
3333

3434
error: statement with no effect
35-
--> $DIR/no_effect.rs:109:5
35+
--> $DIR/no_effect.rs:125:5
3636
|
3737
LL | Struct { ..s };
3838
| ^^^^^^^^^^^^^^^
3939

4040
error: statement with no effect
41-
--> $DIR/no_effect.rs:111:5
41+
--> $DIR/no_effect.rs:127:5
4242
|
4343
LL | Union { a: 0 };
4444
| ^^^^^^^^^^^^^^^
4545

4646
error: statement with no effect
47-
--> $DIR/no_effect.rs:113:5
47+
--> $DIR/no_effect.rs:129:5
4848
|
4949
LL | Enum::Tuple(0);
5050
| ^^^^^^^^^^^^^^^
5151

5252
error: statement with no effect
53-
--> $DIR/no_effect.rs:115:5
53+
--> $DIR/no_effect.rs:131:5
5454
|
5555
LL | Enum::Struct { field: 0 };
5656
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
5757

5858
error: statement with no effect
59-
--> $DIR/no_effect.rs:117:5
59+
--> $DIR/no_effect.rs:133:5
6060
|
6161
LL | 5 + 6;
6262
| ^^^^^^
6363

6464
error: statement with no effect
65-
--> $DIR/no_effect.rs:119:5
65+
--> $DIR/no_effect.rs:135:5
6666
|
6767
LL | *&42;
6868
| ^^^^^
6969

7070
error: statement with no effect
71-
--> $DIR/no_effect.rs:121:5
71+
--> $DIR/no_effect.rs:137:5
7272
|
7373
LL | &6;
7474
| ^^^
7575

7676
error: statement with no effect
77-
--> $DIR/no_effect.rs:123:5
77+
--> $DIR/no_effect.rs:139:5
7878
|
7979
LL | (5, 6, 7);
8080
| ^^^^^^^^^^
8181

8282
error: statement with no effect
83-
--> $DIR/no_effect.rs:125:5
83+
--> $DIR/no_effect.rs:141:5
8484
|
8585
LL | ..;
8686
| ^^^
8787

8888
error: statement with no effect
89-
--> $DIR/no_effect.rs:127:5
89+
--> $DIR/no_effect.rs:143:5
9090
|
9191
LL | 5..;
9292
| ^^^^
9393

9494
error: statement with no effect
95-
--> $DIR/no_effect.rs:129:5
95+
--> $DIR/no_effect.rs:145:5
9696
|
9797
LL | ..5;
9898
| ^^^^
9999

100100
error: statement with no effect
101-
--> $DIR/no_effect.rs:131:5
101+
--> $DIR/no_effect.rs:147:5
102102
|
103103
LL | 5..6;
104104
| ^^^^^
105105

106106
error: statement with no effect
107-
--> $DIR/no_effect.rs:133:5
107+
--> $DIR/no_effect.rs:149:5
108108
|
109109
LL | 5..=6;
110110
| ^^^^^^
111111

112112
error: statement with no effect
113-
--> $DIR/no_effect.rs:135:5
113+
--> $DIR/no_effect.rs:151:5
114114
|
115115
LL | [42, 55];
116116
| ^^^^^^^^^
117117

118118
error: statement with no effect
119-
--> $DIR/no_effect.rs:137:5
119+
--> $DIR/no_effect.rs:153:5
120120
|
121121
LL | [42, 55][1];
122122
| ^^^^^^^^^^^^
123123

124124
error: statement with no effect
125-
--> $DIR/no_effect.rs:139:5
125+
--> $DIR/no_effect.rs:155:5
126126
|
127127
LL | (42, 55).1;
128128
| ^^^^^^^^^^^
129129

130130
error: statement with no effect
131-
--> $DIR/no_effect.rs:141:5
131+
--> $DIR/no_effect.rs:157:5
132132
|
133133
LL | [42; 55];
134134
| ^^^^^^^^^
135135

136136
error: statement with no effect
137-
--> $DIR/no_effect.rs:143:5
137+
--> $DIR/no_effect.rs:159:5
138138
|
139139
LL | [42; 55][13];
140140
| ^^^^^^^^^^^^^
141141

142142
error: statement with no effect
143-
--> $DIR/no_effect.rs:146:5
143+
--> $DIR/no_effect.rs:162:5
144144
|
145145
LL | || x += 5;
146146
| ^^^^^^^^^^
147147

148148
error: statement with no effect
149-
--> $DIR/no_effect.rs:149:5
149+
--> $DIR/no_effect.rs:165:5
150150
|
151151
LL | FooString { s: s };
152152
| ^^^^^^^^^^^^^^^^^^^
153153

154154
error: binding to `_` prefixed variable with no side-effect
155-
--> $DIR/no_effect.rs:151:5
155+
--> $DIR/no_effect.rs:167:5
156156
|
157157
LL | let _unused = 1;
158158
| ^^^^^^^^^^^^^^^^
@@ -161,19 +161,19 @@ LL | let _unused = 1;
161161
= help: to override `-D warnings` add `#[allow(clippy::no_effect_underscore_binding)]`
162162

163163
error: binding to `_` prefixed variable with no side-effect
164-
--> $DIR/no_effect.rs:154:5
164+
--> $DIR/no_effect.rs:170:5
165165
|
166166
LL | let _penguin = || println!("Some helpful closure");
167167
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
168168

169169
error: binding to `_` prefixed variable with no side-effect
170-
--> $DIR/no_effect.rs:156:5
170+
--> $DIR/no_effect.rs:172:5
171171
|
172172
LL | let _duck = Struct { field: 0 };
173173
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
174174

175175
error: binding to `_` prefixed variable with no side-effect
176-
--> $DIR/no_effect.rs:158:5
176+
--> $DIR/no_effect.rs:174:5
177177
|
178178
LL | let _cat = [2, 4, 6, 8][2];
179179
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)