Skip to content

Commit

Permalink
Don't shadow unnecessary operation lint if no_effect is allowed
Browse files Browse the repository at this point in the history
  • Loading branch information
F3real committed Oct 14, 2021
1 parent f18a2f1 commit 74b4b83
Show file tree
Hide file tree
Showing 25 changed files with 289 additions and 59 deletions.
4 changes: 3 additions & 1 deletion clippy_lints/src/no_effect.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
use clippy_utils::is_lint_allowed;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::has_drop;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -79,12 +80,13 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect {

fn check_no_effect(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> bool {
if let StmtKind::Semi(expr) = stmt.kind {
if has_no_effect(cx, expr) {
if has_no_effect(cx, expr) && !is_lint_allowed(cx, NO_EFFECT, expr.hir_id) {
span_lint_hir(cx, NO_EFFECT, expr.hir_id, stmt.span, "statement with no effect");
return true;
}
} else if let StmtKind::Local(local) = stmt.kind {
if_chain! {
if !is_lint_allowed(cx, NO_EFFECT_UNDERSCORE_BINDING, local.hir_id);
if let Some(init) = local.init;
if !local.pat.span.from_expansion();
if has_no_effect(cx, init);
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/author/blocks.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(redundant_semicolons, clippy::no_effect)]
#![allow(redundant_semicolons, clippy::no_effect, clippy::unnecessary_operation)]

#[rustfmt::skip]
fn main() {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/cfg_attr_rustfmt.fixed
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// run-rustfix
#![feature(stmt_expr_attributes)]

#![allow(unused, clippy::no_effect)]
#![allow(unused, clippy::no_effect, clippy::unnecessary_operation)]
#![warn(clippy::deprecated_cfg_attr)]

// This doesn't get linted, see known problems
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/cfg_attr_rustfmt.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// run-rustfix
#![feature(stmt_expr_attributes)]

#![allow(unused, clippy::no_effect)]
#![allow(unused, clippy::no_effect, clippy::unnecessary_operation)]
#![warn(clippy::deprecated_cfg_attr)]

// This doesn't get linted, see known problems
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-7340.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(clippy::no_effect)]
#![allow(clippy::no_effect, clippy::unnecessary_operation)]

fn main() {
const CONSTANT: usize = 8;
Expand Down
22 changes: 21 additions & 1 deletion tests/ui/erasing_op.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
error: unnecessary operation
--> $DIR/erasing_op.rs:6:5
|
LL | x * 0;
| ^^^^^^ help: statement can be reduced to: `x;0;`
|
= note: `-D clippy::unnecessary-operation` implied by `-D warnings`

error: this operation will always return zero. This is likely not the intended outcome
--> $DIR/erasing_op.rs:6:5
|
Expand All @@ -6,17 +14,29 @@ LL | x * 0;
|
= note: `-D clippy::erasing-op` implied by `-D warnings`

error: unnecessary operation
--> $DIR/erasing_op.rs:7:5
|
LL | 0 & x;
| ^^^^^^ help: statement can be reduced to: `0;x;`

error: this operation will always return zero. This is likely not the intended outcome
--> $DIR/erasing_op.rs:7:5
|
LL | 0 & x;
| ^^^^^

error: unnecessary operation
--> $DIR/erasing_op.rs:8:5
|
LL | 0 / x;
| ^^^^^^ help: statement can be reduced to: `0;x;`

error: this operation will always return zero. This is likely not the intended outcome
--> $DIR/erasing_op.rs:8:5
|
LL | 0 / x;
| ^^^^^

error: aborting due to 3 previous errors
error: aborting due to 6 previous errors

2 changes: 1 addition & 1 deletion tests/ui/eta.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ macro_rules! closure_mac {

fn main() {
let a = Some(1u8).map(foo);
let c = Some(1u8).map(|a| {1+2; foo}(a));
let c = Some(1u8).map(|a| {1;2; foo}(a));
true.then(|| mac!()); // don't lint function in macro expansion
Some(1).map(closure_mac!()); // don't lint closure in macro expansion
let _: Option<Vec<u8>> = true.then(std::vec::Vec::new); // special case vec!
Expand Down
10 changes: 9 additions & 1 deletion tests/ui/eta.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ LL | let a = Some(1u8).map(|a| foo(a));
|
= note: `-D clippy::redundant-closure` implied by `-D warnings`

error: unnecessary operation
--> $DIR/eta.rs:32:32
|
LL | let c = Some(1u8).map(|a| {1+2; foo}(a));
| ^^^^ help: statement can be reduced to: `1;2;`
|
= note: `-D clippy::unnecessary-operation` implied by `-D warnings`

error: redundant closure
--> $DIR/eta.rs:35:40
|
Expand Down Expand Up @@ -130,5 +138,5 @@ error: redundant closure
LL | map_str_to_path(|s| s.as_ref());
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::convert::AsRef::as_ref`

error: aborting due to 21 previous errors
error: aborting due to 22 previous errors

100 changes: 99 additions & 1 deletion tests/ui/if_same_then_else.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,92 @@ LL | | foo();
LL | | }
| |_____^

error: unnecessary operation
--> $DIR/if_same_then_else.rs:22:9
|
LL | Foo { bar: 42 };
| ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;`
|
= note: `-D clippy::unnecessary-operation` implied by `-D warnings`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:23:9
|
LL | 0..10;
| ^^^^^^ help: statement can be reduced to: `0;10;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:24:9
|
LL | ..;
| ^^^ help: statement can be reduced to

error: unnecessary operation
--> $DIR/if_same_then_else.rs:25:9
|
LL | 0..;
| ^^^^ help: statement can be reduced to: `0;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:26:9
|
LL | ..10;
| ^^^^^ help: statement can be reduced to: `10;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:31:9
|
LL | Foo { bar: 42 };
| ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:32:9
|
LL | 0..10;
| ^^^^^^ help: statement can be reduced to: `0;10;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:33:9
|
LL | ..;
| ^^^ help: statement can be reduced to

error: unnecessary operation
--> $DIR/if_same_then_else.rs:34:9
|
LL | 0..;
| ^^^^ help: statement can be reduced to: `0;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:35:9
|
LL | ..10;
| ^^^^^ help: statement can be reduced to: `10;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:41:9
|
LL | Foo { bar: 42 };
| ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:43:9
|
LL | Foo { bar: 43 };
| ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `43;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:47:9
|
LL | ();
| ^^^ help: statement can be reduced to

error: unnecessary operation
--> $DIR/if_same_then_else.rs:53:9
|
LL | 0..10;
| ^^^^^^ help: statement can be reduced to: `0;10;`

error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:65:21
|
Expand Down Expand Up @@ -108,5 +194,17 @@ LL | | bar + 1;
LL | | }
| |_____^

error: aborting due to 5 previous errors
error: unnecessary operation
--> $DIR/if_same_then_else.rs:101:9
|
LL | bar + 1;
| ^^^^^^^^ help: statement can be reduced to: `bar;1;`

error: unnecessary operation
--> $DIR/if_same_then_else.rs:109:9
|
LL | bar + 1;
| ^^^^^^^^ help: statement can be reduced to: `bar;1;`

error: aborting due to 21 previous errors

2 changes: 1 addition & 1 deletion tests/ui/let_unit.fixed
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// run-rustfix

#![warn(clippy::let_unit_value)]
#![allow(clippy::no_effect)]
#![allow(clippy::no_effect, clippy::unnecessary_operation)]
#![allow(unused_variables)]

macro_rules! let_and_return {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/let_unit.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// run-rustfix

#![warn(clippy::let_unit_value)]
#![allow(clippy::no_effect)]
#![allow(clippy::no_effect, clippy::unnecessary_operation)]
#![allow(unused_variables)]

macro_rules! let_and_return {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/needless_bool/fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
unused,
dead_code,
clippy::no_effect,
clippy::unnecessary_operation,
clippy::if_same_then_else,
clippy::equatable_if_let,
clippy::needless_return,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/needless_bool/fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
unused,
dead_code,
clippy::no_effect,
clippy::unnecessary_operation,
clippy::if_same_then_else,
clippy::equatable_if_let,
clippy::needless_return,
Expand Down
24 changes: 12 additions & 12 deletions tests/ui/needless_bool/fixable.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:41:5
--> $DIR/fixable.rs:42:5
|
LL | / if x {
LL | | true
Expand All @@ -11,7 +11,7 @@ LL | | };
= note: `-D clippy::needless-bool` implied by `-D warnings`

error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:46:5
--> $DIR/fixable.rs:47:5
|
LL | / if x {
LL | | false
Expand All @@ -21,7 +21,7 @@ LL | | };
| |_____^ help: you can reduce it to: `!x`

error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:51:5
--> $DIR/fixable.rs:52:5
|
LL | / if x && y {
LL | | false
Expand All @@ -31,7 +31,7 @@ LL | | };
| |_____^ help: you can reduce it to: `!(x && y)`

error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:71:5
--> $DIR/fixable.rs:72:5
|
LL | / if x {
LL | | return true;
Expand All @@ -41,7 +41,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return x`

error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:79:5
--> $DIR/fixable.rs:80:5
|
LL | / if x {
LL | | return false;
Expand All @@ -51,7 +51,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return !x`

error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:87:5
--> $DIR/fixable.rs:88:5
|
LL | / if x && y {
LL | | return true;
Expand All @@ -61,7 +61,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return x && y`

error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:95:5
--> $DIR/fixable.rs:96:5
|
LL | / if x && y {
LL | | return false;
Expand All @@ -71,33 +71,33 @@ LL | | };
| |_____^ help: you can reduce it to: `return !(x && y)`

error: equality checks against true are unnecessary
--> $DIR/fixable.rs:103:8
--> $DIR/fixable.rs:104:8
|
LL | if x == true {};
| ^^^^^^^^^ help: try simplifying it as shown: `x`
|
= note: `-D clippy::bool-comparison` implied by `-D warnings`

error: equality checks against false can be replaced by a negation
--> $DIR/fixable.rs:107:8
--> $DIR/fixable.rs:108:8
|
LL | if x == false {};
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`

error: equality checks against true are unnecessary
--> $DIR/fixable.rs:117:8
--> $DIR/fixable.rs:118:8
|
LL | if x == true {};
| ^^^^^^^^^ help: try simplifying it as shown: `x`

error: equality checks against false can be replaced by a negation
--> $DIR/fixable.rs:118:8
--> $DIR/fixable.rs:119:8
|
LL | if x == false {};
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`

error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:127:12
--> $DIR/fixable.rs:128:12
|
LL | } else if returns_bool() {
| ____________^
Expand Down
Loading

0 comments on commit 74b4b83

Please sign in to comment.