mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #3601 - xfix:move-constant-write-lint, r=flip1995

Browse Source 
Move constant write checks to temporary_assignment lint

They make more sense here

cc #3595
This commit is contained in:
bors 2019-01-04 14:59:11 +00:00
commit 194a91c45d
6 changed files with 124 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
clippy_lints/src/no_effect.rs
View File

@ -98,20 +98,6 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
false
}
},
ExprKind::Assign(ref left, ref right) => {
if has_no_effect(cx, left) {
let mut left = left;
while let ExprKind::Field(f, _) | ExprKind::Index(f, _) = &left.node {
left = f;
}
if let ExprKind::Path(qpath) = &left.node {
if let Def::Const(..) = cx.tables.qpath_def(qpath, left.hir_id) {
return has_no_effect(cx, right);
}
}
}
false
},
_ => false,
}
}

24
clippy_lints/src/temporary_assignment.rs
View File

@ -9,6 +9,7 @@
use crate::utils::is_adjusted;
use crate::utils::span_lint;
use rustc::hir::def::Def;
use rustc::hir::{Expr, ExprKind};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_tool_lint, lint_array};
@ -31,9 +32,16 @@
"assignments to temporaries"
}
fn is_temporary(expr: &Expr) -> bool {
match expr.node {
fn is_temporary(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
match &expr.node {
ExprKind::Struct(..) | ExprKind::Tup(..) => true,
ExprKind::Path(qpath) => {
if let Def::Const(..) = cx.tables.qpath_def(qpath, expr.hir_id) {
true
} else {
false
}
},
_ => false,
}
}
@ -49,11 +57,13 @@ fn get_lints(&self) -> LintArray {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Assign(ref target, _) = expr.node {
if let ExprKind::Field(ref base, _) = target.node {
if is_temporary(base) && !is_adjusted(cx, base) {
span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, "assignment to temporary");
}
if let ExprKind::Assign(target, _) = &expr.node {
let mut base = target;
while let ExprKind::Field(f, _) | ExprKind::Index(f, _) = &base.node {
base = f;
}
if is_temporary(cx, base) && !is_adjusted(cx, base) {
span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, "assignment to temporary");
}
}
}

27
tests/ui/no_effect.rs
View File

@ -67,21 +67,6 @@ unsafe fn unsafe_fn() -> i32 {
0
}
struct A(i32);
struct B {
field: i32,
}
struct C {
b: B,
}
struct D {
arr: [i32; 1],
}
const A_CONST: A = A(1);
const B: B = B { field: 1 };
const C: C = C { b: B { field: 1 } };
const D: D = D { arr: [1] };
fn main() {
let s = get_struct();
let s2 = get_struct();
@ -114,10 +99,6 @@ fn main() {
|| x += 5;
let s: String = "foo".into();
FooString { s: s };
A_CONST.0 = 2;
B.field = 2;
C.b.field = 2;
D.arr[0] = 2;
// Do not warn
get_number();
@ -127,12 +108,4 @@ fn main() {
DropTuple(0);
DropEnum::Tuple(0);
DropEnum::Struct { field: 0 };
let mut a_mut = A(1);
a_mut.0 = 2;
let mut b_mut = B { field: 1 };
b_mut.field = 2;
let mut c_mut = C { b: B { field: 1 } };
c_mut.b.field = 2;
let mut d_mut = D { arr: [1] };
d_mut.arr[0] = 2;
}

76
tests/ui/no_effect.stderr
View File

@ -1,5 +1,5 @@
error: statement with no effect
--> $DIR/no_effect.rs:89:5
--> $DIR/no_effect.rs:74:5
|
LL | 0;
| ^^
@ -7,172 +7,148 @@ LL | 0;
= note: `-D clippy::no-effect` implied by `-D warnings`
error: statement with no effect
--> $DIR/no_effect.rs:90:5
--> $DIR/no_effect.rs:75:5
|
LL | s2;
| ^^^
error: statement with no effect
--> $DIR/no_effect.rs:91:5
--> $DIR/no_effect.rs:76:5
|
LL | Unit;
| ^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:92:5
--> $DIR/no_effect.rs:77:5
|
LL | Tuple(0);
| ^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:93:5
--> $DIR/no_effect.rs:78:5
|
LL | Struct { field: 0 };
| ^^^^^^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:94:5
--> $DIR/no_effect.rs:79:5
|
LL | Struct { ..s };
| ^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:95:5
--> $DIR/no_effect.rs:80:5
|
LL | Union { a: 0 };
| ^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:96:5
--> $DIR/no_effect.rs:81:5
|
LL | Enum::Tuple(0);
| ^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:97:5
--> $DIR/no_effect.rs:82:5
|
LL | Enum::Struct { field: 0 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:98:5
--> $DIR/no_effect.rs:83:5
|
LL | 5 + 6;
| ^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:99:5
--> $DIR/no_effect.rs:84:5
|
LL | *&42;
| ^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:100:5
--> $DIR/no_effect.rs:85:5
|
LL | &6;
| ^^^
error: statement with no effect
--> $DIR/no_effect.rs:101:5
--> $DIR/no_effect.rs:86:5
|
LL | (5, 6, 7);
| ^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:102:5
--> $DIR/no_effect.rs:87:5
|
LL | box 42;
| ^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:103:5
--> $DIR/no_effect.rs:88:5
|
LL | ..;
| ^^^
error: statement with no effect
--> $DIR/no_effect.rs:104:5
--> $DIR/no_effect.rs:89:5
|
LL | 5..;
| ^^^^
error: statement with no effect
--> $DIR/no_effect.rs:105:5
--> $DIR/no_effect.rs:90:5
|
LL | ..5;
| ^^^^
error: statement with no effect
--> $DIR/no_effect.rs:106:5
--> $DIR/no_effect.rs:91:5
|
LL | 5..6;
| ^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:108:5
--> $DIR/no_effect.rs:93:5
|
LL | [42, 55];
| ^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:109:5
--> $DIR/no_effect.rs:94:5
|
LL | [42, 55][1];
| ^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:110:5
--> $DIR/no_effect.rs:95:5
|
LL | (42, 55).1;
| ^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:111:5
--> $DIR/no_effect.rs:96:5
|
LL | [42; 55];
| ^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:112:5
--> $DIR/no_effect.rs:97:5
|
LL | [42; 55][13];
| ^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:114:5
--> $DIR/no_effect.rs:99:5
|
LL | || x += 5;
| ^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:116:5
--> $DIR/no_effect.rs:101:5
|
LL | FooString { s: s };
| ^^^^^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:117:5
|
LL | A_CONST.0 = 2;
| ^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:118:5
|
LL | B.field = 2;
| ^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:119:5
|
LL | C.b.field = 2;
| ^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:120:5
|
LL | D.arr[0] = 2;
| ^^^^^^^^^^^^^
error: aborting due to 29 previous errors
error: aborting due to 25 previous errors

38
tests/ui/temporary_assignment.rs
View File

@ -11,10 +11,16 @@
use std::ops::{Deref, DerefMut};
struct TupleStruct(i32);
struct Struct {
field: i32,
}
struct MultiStruct {
structure: Struct,
}
struct Wrapper<'a> {
inner: &'a mut Struct,
}
@ -32,15 +38,47 @@ fn deref_mut(&mut self) -> &mut Struct {
}
}
struct ArrayStruct {
array: [i32; 1],
}
const A: TupleStruct = TupleStruct(1);
const B: Struct = Struct { field: 1 };
const C: MultiStruct = MultiStruct {
structure: Struct { field: 1 },
};
const D: ArrayStruct = ArrayStruct { array: [1] };
fn main() {
let mut s = Struct { field: 0 };
let mut t = (0, 0);
Struct { field: 0 }.field = 1;
MultiStruct {
structure: Struct { field: 0 },
}
.structure
.field = 1;
ArrayStruct { array: [0] }.array[0] = 1;
(0, 0).0 = 1;
A.0 = 2;
B.field = 2;
C.structure.field = 2;
D.array[0] = 2;
// no error
s.field = 1;
t.0 = 1;
Wrapper { inner: &mut s }.field = 1;
let mut a_mut = TupleStruct(1);
a_mut.0 = 2;
let mut b_mut = Struct { field: 1 };
b_mut.field = 2;
let mut c_mut = MultiStruct {
structure: Struct { field: 1 },
};
c_mut.structure.field = 2;
let mut d_mut = ArrayStruct { array: [1] };
d_mut.array[0] = 2;
}

46
tests/ui/temporary_assignment.stderr
View File

@ -1,5 +1,5 @@
error: assignment to temporary
--> $DIR/temporary_assignment.rs:39:5
--> $DIR/temporary_assignment.rs:56:5
|
LL | Struct { field: 0 }.field = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -7,10 +7,50 @@ LL | Struct { field: 0 }.field = 1;
= note: `-D clippy::temporary-assignment` implied by `-D warnings`
error: assignment to temporary
--> $DIR/temporary_assignment.rs:40:5
--> $DIR/temporary_assignment.rs:57:5
|
LL | / MultiStruct {
LL | | structure: Struct { field: 0 },
LL | | }
LL | | .structure
LL | | .field = 1;
| |______________^
error: assignment to temporary
--> $DIR/temporary_assignment.rs:62:5
|
LL | ArrayStruct { array: [0] }.array[0] = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: assignment to temporary
--> $DIR/temporary_assignment.rs:63:5
|
LL | (0, 0).0 = 1;
| ^^^^^^^^^^^^
error: aborting due to 2 previous errors
error: assignment to temporary
--> $DIR/temporary_assignment.rs:65:5
|
LL | A.0 = 2;
| ^^^^^^^
error: assignment to temporary
--> $DIR/temporary_assignment.rs:66:5
|
LL | B.field = 2;
| ^^^^^^^^^^^
error: assignment to temporary
--> $DIR/temporary_assignment.rs:67:5
|
LL | C.structure.field = 2;
| ^^^^^^^^^^^^^^^^^^^^^
error: assignment to temporary
--> $DIR/temporary_assignment.rs:68:5
|
LL | D.array[0] = 2;
| ^^^^^^^^^^^^^^
error: aborting due to 8 previous errors