Skip to content

Commit 194a91c

Browse files
borsbors
committedJan 4, 2019
Auto merge of #3601 - xfix:move-constant-write-lint, r=flip1995
Move constant write checks to temporary_assignment lint They make more sense here cc #3595
·
rust-1.86.0rust-1.33.0
2 parents 756b32e + 815e434 commit 194a91c

6 files changed

+124
-101
lines changed
 

‎clippy_lints/src/no_effect.rs

-14
Original file line numberDiff line numberDiff line change
@@ -98,20 +98,6 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
9898
false
9999
}
100100
},
101-
ExprKind::Assign(ref left, ref right) => {
102-
if has_no_effect(cx, left) {
103-
let mut left = left;
104-
while let ExprKind::Field(f, _) | ExprKind::Index(f, _) = &left.node {
105-
left = f;
106-
}
107-
if let ExprKind::Path(qpath) = &left.node {
108-
if let Def::Const(..) = cx.tables.qpath_def(qpath, left.hir_id) {
109-
return has_no_effect(cx, right);
110-
}
111-
}
112-
}
113-
false
114-
},
115101
_ => false,
116102
}
117103
}

‎clippy_lints/src/temporary_assignment.rs

+17-7
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use crate::utils::is_adjusted;
1111
use crate::utils::span_lint;
12+
use rustc::hir::def::Def;
1213
use rustc::hir::{Expr, ExprKind};
1314
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
1415
use rustc::{declare_tool_lint, lint_array};
@@ -31,9 +32,16 @@ declare_clippy_lint! {
3132
"assignments to temporaries"
3233
}
3334

34-
fn is_temporary(expr: &Expr) -> bool {
35-
match expr.node {
35+
fn is_temporary(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
36+
match &expr.node {
3637
ExprKind::Struct(..) | ExprKind::Tup(..) => true,
38+
ExprKind::Path(qpath) => {
39+
if let Def::Const(..) = cx.tables.qpath_def(qpath, expr.hir_id) {
40+
true
41+
} else {
42+
false
43+
}
44+
},
3745
_ => false,
3846
}
3947
}
@@ -49,11 +57,13 @@ impl LintPass for Pass {
4957

5058
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
5159
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
52-
if let ExprKind::Assign(ref target, _) = expr.node {
53-
if let ExprKind::Field(ref base, _) = target.node {
54-
if is_temporary(base) && !is_adjusted(cx, base) {
55-
span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, "assignment to temporary");
56-
}
60+
if let ExprKind::Assign(target, _) = &expr.node {
61+
let mut base = target;
62+
while let ExprKind::Field(f, _) | ExprKind::Index(f, _) = &base.node {
63+
base = f;
64+
}
65+
if is_temporary(cx, base) && !is_adjusted(cx, base) {
66+
span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, "assignment to temporary");
5767
}
5868
}
5969
}

‎tests/ui/no_effect.rs

-27
Original file line numberDiff line numberDiff line change
@@ -67,21 +67,6 @@ unsafe fn unsafe_fn() -> i32 {
6767
0
6868
}
6969

70-
struct A(i32);
71-
struct B {
72-
field: i32,
73-
}
74-
struct C {
75-
b: B,
76-
}
77-
struct D {
78-
arr: [i32; 1],
79-
}
80-
const A_CONST: A = A(1);
81-
const B: B = B { field: 1 };
82-
const C: C = C { b: B { field: 1 } };
83-
const D: D = D { arr: [1] };
84-
8570
fn main() {
8671
let s = get_struct();
8772
let s2 = get_struct();
@@ -114,10 +99,6 @@ fn main() {
11499
|| x += 5;
115100
let s: String = "foo".into();
116101
FooString { s: s };
117-
A_CONST.0 = 2;
118-
B.field = 2;
119-
C.b.field = 2;
120-
D.arr[0] = 2;
121102

122103
// Do not warn
123104
get_number();
@@ -127,12 +108,4 @@ fn main() {
127108
DropTuple(0);
128109
DropEnum::Tuple(0);
129110
DropEnum::Struct { field: 0 };
130-
let mut a_mut = A(1);
131-
a_mut.0 = 2;
132-
let mut b_mut = B { field: 1 };
133-
b_mut.field = 2;
134-
let mut c_mut = C { b: B { field: 1 } };
135-
c_mut.b.field = 2;
136-
let mut d_mut = D { arr: [1] };
137-
d_mut.arr[0] = 2;
138111
}

‎tests/ui/no_effect.stderr

+26-50
Original file line numberDiff line numberDiff line change
@@ -1,178 +1,154 @@
11
error: statement with no effect
2-
--> $DIR/no_effect.rs:89:5
2+
--> $DIR/no_effect.rs:74:5
33
|
44
LL | 0;
55
| ^^
66
|
77
= note: `-D clippy::no-effect` implied by `-D warnings`
88

99
error: statement with no effect
10-
--> $DIR/no_effect.rs:90:5
10+
--> $DIR/no_effect.rs:75:5
1111
|
1212
LL | s2;
1313
| ^^^
1414

1515
error: statement with no effect
16-
--> $DIR/no_effect.rs:91:5
16+
--> $DIR/no_effect.rs:76:5
1717
|
1818
LL | Unit;
1919
| ^^^^^
2020

2121
error: statement with no effect
22-
--> $DIR/no_effect.rs:92:5
22+
--> $DIR/no_effect.rs:77:5
2323
|
2424
LL | Tuple(0);
2525
| ^^^^^^^^^
2626

2727
error: statement with no effect
28-
--> $DIR/no_effect.rs:93:5
28+
--> $DIR/no_effect.rs:78:5
2929
|
3030
LL | Struct { field: 0 };
3131
| ^^^^^^^^^^^^^^^^^^^^
3232

3333
error: statement with no effect
34-
--> $DIR/no_effect.rs:94:5
34+
--> $DIR/no_effect.rs:79:5
3535
|
3636
LL | Struct { ..s };
3737
| ^^^^^^^^^^^^^^^
3838

3939
error: statement with no effect
40-
--> $DIR/no_effect.rs:95:5
40+
--> $DIR/no_effect.rs:80:5
4141
|
4242
LL | Union { a: 0 };
4343
| ^^^^^^^^^^^^^^^
4444

4545
error: statement with no effect
46-
--> $DIR/no_effect.rs:96:5
46+
--> $DIR/no_effect.rs:81:5
4747
|
4848
LL | Enum::Tuple(0);
4949
| ^^^^^^^^^^^^^^^
5050

5151
error: statement with no effect
52-
--> $DIR/no_effect.rs:97:5
52+
--> $DIR/no_effect.rs:82:5
5353
|
5454
LL | Enum::Struct { field: 0 };
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
5656

5757
error: statement with no effect
58-
--> $DIR/no_effect.rs:98:5
58+
--> $DIR/no_effect.rs:83:5
5959
|
6060
LL | 5 + 6;
6161
| ^^^^^^
6262

6363
error: statement with no effect
64-
--> $DIR/no_effect.rs:99:5
64+
--> $DIR/no_effect.rs:84:5
6565
|
6666
LL | *&42;
6767
| ^^^^^
6868

6969
error: statement with no effect
70-
--> $DIR/no_effect.rs:100:5
70+
--> $DIR/no_effect.rs:85:5
7171
|
7272
LL | &6;
7373
| ^^^
7474

7575
error: statement with no effect
76-
--> $DIR/no_effect.rs:101:5
76+
--> $DIR/no_effect.rs:86:5
7777
|
7878
LL | (5, 6, 7);
7979
| ^^^^^^^^^^
8080

8181
error: statement with no effect
82-
--> $DIR/no_effect.rs:102:5
82+
--> $DIR/no_effect.rs:87:5
8383
|
8484
LL | box 42;
8585
| ^^^^^^^
8686

8787
error: statement with no effect
88-
--> $DIR/no_effect.rs:103:5
88+
--> $DIR/no_effect.rs:88:5
8989
|
9090
LL | ..;
9191
| ^^^
9292

9393
error: statement with no effect
94-
--> $DIR/no_effect.rs:104:5
94+
--> $DIR/no_effect.rs:89:5
9595
|
9696
LL | 5..;
9797
| ^^^^
9898

9999
error: statement with no effect
100-
--> $DIR/no_effect.rs:105:5
100+
--> $DIR/no_effect.rs:90:5
101101
|
102102
LL | ..5;
103103
| ^^^^
104104

105105
error: statement with no effect
106-
--> $DIR/no_effect.rs:106:5
106+
--> $DIR/no_effect.rs:91:5
107107
|
108108
LL | 5..6;
109109
| ^^^^^
110110

111111
error: statement with no effect
112-
--> $DIR/no_effect.rs:108:5
112+
--> $DIR/no_effect.rs:93:5
113113
|
114114
LL | [42, 55];
115115
| ^^^^^^^^^
116116

117117
error: statement with no effect
118-
--> $DIR/no_effect.rs:109:5
118+
--> $DIR/no_effect.rs:94:5
119119
|
120120
LL | [42, 55][1];
121121
| ^^^^^^^^^^^^
122122

123123
error: statement with no effect
124-
--> $DIR/no_effect.rs:110:5
124+
--> $DIR/no_effect.rs:95:5
125125
|
126126
LL | (42, 55).1;
127127
| ^^^^^^^^^^^
128128

129129
error: statement with no effect
130-
--> $DIR/no_effect.rs:111:5
130+
--> $DIR/no_effect.rs:96:5
131131
|
132132
LL | [42; 55];
133133
| ^^^^^^^^^
134134

135135
error: statement with no effect
136-
--> $DIR/no_effect.rs:112:5
136+
--> $DIR/no_effect.rs:97:5
137137
|
138138
LL | [42; 55][13];
139139
| ^^^^^^^^^^^^^
140140

141141
error: statement with no effect
142-
--> $DIR/no_effect.rs:114:5
142+
--> $DIR/no_effect.rs:99:5
143143
|
144144
LL | || x += 5;
145145
| ^^^^^^^^^^
146146

147147
error: statement with no effect
148-
--> $DIR/no_effect.rs:116:5
148+
--> $DIR/no_effect.rs:101:5
149149
|
150150
LL | FooString { s: s };
151151
| ^^^^^^^^^^^^^^^^^^^
152152

153-
error: statement with no effect
154-
--> $DIR/no_effect.rs:117:5
155-
|
156-
LL | A_CONST.0 = 2;
157-
| ^^^^^^^^^^^^^^
158-
159-
error: statement with no effect
160-
--> $DIR/no_effect.rs:118:5
161-
|
162-
LL | B.field = 2;
163-
| ^^^^^^^^^^^^
164-
165-
error: statement with no effect
166-
--> $DIR/no_effect.rs:119:5
167-
|
168-
LL | C.b.field = 2;
169-
| ^^^^^^^^^^^^^^
170-
171-
error: statement with no effect
172-
--> $DIR/no_effect.rs:120:5
173-
|
174-
LL | D.arr[0] = 2;
175-
| ^^^^^^^^^^^^^
176-
177-
error: aborting due to 29 previous errors
153+
error: aborting due to 25 previous errors
178154

‎tests/ui/temporary_assignment.rs

+38
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,16 @@
1111

1212
use std::ops::{Deref, DerefMut};
1313

14+
struct TupleStruct(i32);
15+
1416
struct Struct {
1517
field: i32,
1618
}
1719

20+
struct MultiStruct {
21+
structure: Struct,
22+
}
23+
1824
struct Wrapper<'a> {
1925
inner: &'a mut Struct,
2026
}
@@ -32,15 +38,47 @@ impl<'a> DerefMut for Wrapper<'a> {
3238
}
3339
}
3440

41+
struct ArrayStruct {
42+
array: [i32; 1],
43+
}
44+
45+
const A: TupleStruct = TupleStruct(1);
46+
const B: Struct = Struct { field: 1 };
47+
const C: MultiStruct = MultiStruct {
48+
structure: Struct { field: 1 },
49+
};
50+
const D: ArrayStruct = ArrayStruct { array: [1] };
51+
3552
fn main() {
3653
let mut s = Struct { field: 0 };
3754
let mut t = (0, 0);
3855

3956
Struct { field: 0 }.field = 1;
57+
MultiStruct {
58+
structure: Struct { field: 0 },
59+
}
60+
.structure
61+
.field = 1;
62+
ArrayStruct { array: [0] }.array[0] = 1;
4063
(0, 0).0 = 1;
4164

65+
A.0 = 2;
66+
B.field = 2;
67+
C.structure.field = 2;
68+
D.array[0] = 2;
69+
4270
// no error
4371
s.field = 1;
4472
t.0 = 1;
4573
Wrapper { inner: &mut s }.field = 1;
74+
let mut a_mut = TupleStruct(1);
75+
a_mut.0 = 2;
76+
let mut b_mut = Struct { field: 1 };
77+
b_mut.field = 2;
78+
let mut c_mut = MultiStruct {
79+
structure: Struct { field: 1 },
80+
};
81+
c_mut.structure.field = 2;
82+
let mut d_mut = ArrayStruct { array: [1] };
83+
d_mut.array[0] = 2;
4684
}

‎tests/ui/temporary_assignment.stderr

+43-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,56 @@
11
error: assignment to temporary
2-
--> $DIR/temporary_assignment.rs:39:5
2+
--> $DIR/temporary_assignment.rs:56:5
33
|
44
LL | Struct { field: 0 }.field = 1;
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::temporary-assignment` implied by `-D warnings`
88

99
error: assignment to temporary
10-
--> $DIR/temporary_assignment.rs:40:5
10+
--> $DIR/temporary_assignment.rs:57:5
11+
|
12+
LL | / MultiStruct {
13+
LL | | structure: Struct { field: 0 },
14+
LL | | }
15+
LL | | .structure
16+
LL | | .field = 1;
17+
| |______________^
18+
19+
error: assignment to temporary
20+
--> $DIR/temporary_assignment.rs:62:5
21+
|
22+
LL | ArrayStruct { array: [0] }.array[0] = 1;
23+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
24+
25+
error: assignment to temporary
26+
--> $DIR/temporary_assignment.rs:63:5
1127
|
1228
LL | (0, 0).0 = 1;
1329
| ^^^^^^^^^^^^
1430

15-
error: aborting due to 2 previous errors
31+
error: assignment to temporary
32+
--> $DIR/temporary_assignment.rs:65:5
33+
|
34+
LL | A.0 = 2;
35+
| ^^^^^^^
36+
37+
error: assignment to temporary
38+
--> $DIR/temporary_assignment.rs:66:5
39+
|
40+
LL | B.field = 2;
41+
| ^^^^^^^^^^^
42+
43+
error: assignment to temporary
44+
--> $DIR/temporary_assignment.rs:67:5
45+
|
46+
LL | C.structure.field = 2;
47+
| ^^^^^^^^^^^^^^^^^^^^^
48+
49+
error: assignment to temporary
50+
--> $DIR/temporary_assignment.rs:68:5
51+
|
52+
LL | D.array[0] = 2;
53+
| ^^^^^^^^^^^^^^
54+
55+
error: aborting due to 8 previous errors
1656

0 commit comments

Comments
 (0)
Please sign in to comment.