From ede977cf3131caeaac71e36c32390ca3c5b4deda Mon Sep 17 00:00:00 2001 From: F3real Date: Mon, 9 Aug 2021 09:49:55 +0200 Subject: [PATCH] Update suggestion in case of index for unnecessary_operation lint --- clippy_lints/src/no_effect.rs | 69 +++++++++++++++++------ tests/ui/unnecessary_operation.fixed | 4 +- tests/ui/unnecessary_operation.stderr | 80 +++++++++++++-------------- 3 files changed, 94 insertions(+), 59 deletions(-) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index e07518b25868..c50980e1cf6d 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -94,28 +94,63 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect { if has_no_effect(cx, expr) { span_lint_hir(cx, NO_EFFECT, expr.hir_id, stmt.span, "statement with no effect"); } else if let Some(reduced) = reduce_expression(cx, expr) { - let mut snippet = String::new(); - for e in reduced { + for e in &reduced { if e.span.from_expansion() { return; } - if let Some(snip) = snippet_opt(cx, e.span) { - snippet.push_str(&snip); - snippet.push(';'); - } else { - return; + } + if let ExprKind::Index(..) = &expr.kind { + let snippet; + if_chain! { + if let Some(arr) = snippet_opt(cx, reduced[0].span); + if let Some(func) = snippet_opt(cx, reduced[1].span); + then { + snippet = format!("assert!({}.len() > {});", &arr, &func); + } else { + return; + } } + span_lint_hir_and_then( + cx, + UNNECESSARY_OPERATION, + expr.hir_id, + stmt.span, + "unnecessary operation", + |diag| { + diag.span_suggestion( + stmt.span, + "statement can be written as", + snippet, + Applicability::MaybeIncorrect, + ); + }, + ); + } else { + let mut snippet = String::new(); + for e in reduced { + if let Some(snip) = snippet_opt(cx, e.span) { + snippet.push_str(&snip); + snippet.push(';'); + } else { + return; + } + } + span_lint_hir_and_then( + cx, + UNNECESSARY_OPERATION, + expr.hir_id, + stmt.span, + "unnecessary operation", + |diag| { + diag.span_suggestion( + stmt.span, + "statement can be reduced to", + snippet, + Applicability::MachineApplicable, + ); + }, + ); } - span_lint_hir_and_then( - cx, - UNNECESSARY_OPERATION, - expr.hir_id, - stmt.span, - "statement can be reduced", - |diag| { - diag.span_suggestion(stmt.span, "replace it with", snippet, Applicability::MachineApplicable); - }, - ); } } } diff --git a/tests/ui/unnecessary_operation.fixed b/tests/ui/unnecessary_operation.fixed index 2fca96c4cd55..bf0ec8deb345 100644 --- a/tests/ui/unnecessary_operation.fixed +++ b/tests/ui/unnecessary_operation.fixed @@ -62,10 +62,10 @@ fn main() { get_number(); 5;get_number(); 42;get_number(); - [42, 55];get_usize(); + assert!([42, 55].len() > get_usize()); 42;get_number(); get_number(); - [42; 55];get_usize(); + assert!([42; 55].len() > get_usize()); get_number(); String::from("blah"); diff --git a/tests/ui/unnecessary_operation.stderr b/tests/ui/unnecessary_operation.stderr index f88c9f9908be..f66d08ecb828 100644 --- a/tests/ui/unnecessary_operation.stderr +++ b/tests/ui/unnecessary_operation.stderr @@ -1,128 +1,128 @@ -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:51:5 | LL | Tuple(get_number()); - | ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` | = note: `-D clippy::unnecessary-operation` implied by `-D warnings` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:52:5 | LL | Struct { field: get_number() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:53:5 | LL | Struct { ..get_struct() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_struct();` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_struct();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:54:5 | LL | Enum::Tuple(get_number()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:55:5 | LL | Enum::Struct { field: get_number() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:56:5 | LL | 5 + get_number(); - | ^^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();` + | ^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:57:5 | LL | *&get_number(); - | ^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:58:5 | LL | &get_number(); - | ^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:59:5 | LL | (5, 6, get_number()); - | ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `5;6;get_number();` + | ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;6;get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:60:5 | LL | box get_number(); - | ^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:61:5 | LL | get_number()..; - | ^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:62:5 | LL | ..get_number(); - | ^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:63:5 | LL | 5..get_number(); - | ^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();` + | ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:64:5 | LL | [42, get_number()]; - | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();` + | ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:65:5 | LL | [42, 55][get_usize()]; - | ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42, 55];get_usize();` + | ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42, 55].len() > get_usize());` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:66:5 | LL | (42, get_number()).1; - | ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();` + | ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:67:5 | LL | [get_number(); 55]; - | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` + | ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:68:5 | LL | [42; 55][get_usize()]; - | ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42; 55];get_usize();` + | ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42; 55].len() > get_usize());` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:69:5 | LL | / { LL | | get_number() LL | | }; - | |______^ help: replace it with: `get_number();` + | |______^ help: statement can be reduced to: `get_number();` -error: statement can be reduced +error: unnecessary operation --> $DIR/unnecessary_operation.rs:72:5 | LL | / FooString { LL | | s: String::from("blah"), LL | | }; - | |______^ help: replace it with: `String::from("blah");` + | |______^ help: statement can be reduced to: `String::from("blah");` error: aborting due to 20 previous errors