From ee8fae3f5d4aac2bcbea8936e39d3027aade8e32 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Wed, 4 May 2022 18:13:06 +0100 Subject: [PATCH] identity_op: add parenthesis to suggestions where required --- clippy_lints/src/identity_op.rs | 127 +++++++------ tests/ui/identity_op.fixed | 119 +++++++++++++ tests/ui/identity_op.rs | 59 ++++--- tests/ui/identity_op.stderr | 304 ++++++++++++++++++-------------- tests/ui/modulo_one.rs | 2 +- tests/ui/modulo_one.stderr | 10 +- 6 files changed, 398 insertions(+), 223 deletions(-) create mode 100644 tests/ui/identity_op.fixed diff --git a/clippy_lints/src/identity_op.rs b/clippy_lints/src/identity_op.rs index 40cc5cd4bcf..419ea5a6811 100644 --- a/clippy_lints/src/identity_op.rs +++ b/clippy_lints/src/identity_op.rs @@ -1,15 +1,14 @@ -use clippy_utils::get_parent_expr; -use clippy_utils::source::snippet; -use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind}; +use clippy_utils::consts::{constant_full_int, constant_simple, Constant, FullInt}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::{clip, unsext}; +use rustc_errors::Applicability; +use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; -use clippy_utils::consts::{constant_full_int, constant_simple, Constant, FullInt}; -use clippy_utils::diagnostics::span_lint; -use clippy_utils::{clip, unsext}; - declare_clippy_lint! { /// ### What it does /// Checks for identity operations, e.g., `x + 0`. @@ -23,11 +22,6 @@ declare_clippy_lint! { /// # let x = 1; /// x / 1 + 0 * 1 - 0 | 0; /// ``` - /// - /// ### Known problems - /// False negatives: `f(0 + if b { 1 } else { 2 } + 3);` is reducible to - /// `f(if b { 1 } else { 2 } + 3);`. But the lint doesn't trigger for the code. - /// See [#8724](https://github.com/rust-lang/rust-clippy/issues/8724) #[clippy::version = "pre 1.29.0"] pub IDENTITY_OP, complexity, @@ -45,31 +39,22 @@ impl<'tcx> LateLintPass<'tcx> for IdentityOp { if !is_allowed(cx, *cmp, left, right) { match cmp.node { BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => { - if reducible_to_right(cx, expr, right) { - check(cx, left, 0, expr.span, right.span); - } - check(cx, right, 0, expr.span, left.span); + check(cx, left, 0, expr.span, right.span, needs_parenthesis(cx, expr, right)); + check(cx, right, 0, expr.span, left.span, Parens::Unneeded); }, BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => { - check(cx, right, 0, expr.span, left.span); + check(cx, right, 0, expr.span, left.span, Parens::Unneeded); }, BinOpKind::Mul => { - if reducible_to_right(cx, expr, right) { - check(cx, left, 1, expr.span, right.span); - } - check(cx, right, 1, expr.span, left.span); + check(cx, left, 1, expr.span, right.span, needs_parenthesis(cx, expr, right)); + check(cx, right, 1, expr.span, left.span, Parens::Unneeded); }, - BinOpKind::Div => check(cx, right, 1, expr.span, left.span), + BinOpKind::Div => check(cx, right, 1, expr.span, left.span, Parens::Unneeded), BinOpKind::BitAnd => { - if reducible_to_right(cx, expr, right) { - check(cx, left, -1, expr.span, right.span); - } - check(cx, right, -1, expr.span, left.span); - }, - BinOpKind::Rem => { - // Don't call reducible_to_right because N % N is always reducible to 1 - check_remainder(cx, left, right, expr.span, left.span); + check(cx, left, -1, expr.span, right.span, needs_parenthesis(cx, expr, right)); + check(cx, right, -1, expr.span, left.span, Parens::Unneeded); }, + BinOpKind::Rem => check_remainder(cx, left, right, expr.span, left.span), _ => (), } } @@ -77,24 +62,50 @@ impl<'tcx> LateLintPass<'tcx> for IdentityOp { } } -/// Checks if `left op ..right` can be actually reduced to `right` -/// e.g. `0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }` -/// cannot be reduced to `if b { 1 } else { 2 } + if b { 3 } else { 4 }` -/// See #8724 -fn reducible_to_right(cx: &LateContext<'_>, binary: &Expr<'_>, right: &Expr<'_>) -> bool { - if let ExprKind::If(..) | ExprKind::Match(..) | ExprKind::Block(..) | ExprKind::Loop(..) = right.kind { - is_toplevel_binary(cx, binary) - } else { - true - } +#[derive(Copy, Clone)] +enum Parens { + Needed, + Unneeded, } -fn is_toplevel_binary(cx: &LateContext<'_>, must_be_binary: &Expr<'_>) -> bool { - if let Some(parent) = get_parent_expr(cx, must_be_binary) && let ExprKind::Binary(..) = &parent.kind { - false - } else { - true +/// Checks if `left op right` needs parenthesis when reduced to `right` +/// e.g. `0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }` cannot be reduced +/// to `if b { 1 } else { 2 } + if b { 3 } else { 4 }` where the `if` could be +/// interpreted as a statement +/// +/// See #8724 +fn needs_parenthesis(cx: &LateContext<'_>, binary: &Expr<'_>, right: &Expr<'_>) -> Parens { + match right.kind { + ExprKind::Binary(_, lhs, _) | ExprKind::Cast(lhs, _) => { + // ensure we're checking against the leftmost expression of `right` + // + // ~~~ `lhs` + // 0 + {4} * 2 + // ~~~~~~~ `right` + return needs_parenthesis(cx, binary, lhs); + }, + ExprKind::If(..) | ExprKind::Match(..) | ExprKind::Block(..) | ExprKind::Loop(..) => {}, + _ => return Parens::Unneeded, } + + let mut prev_id = binary.hir_id; + for (_, node) in cx.tcx.hir().parent_iter(binary.hir_id) { + if let Node::Expr(expr) = node + && let ExprKind::Binary(_, lhs, _) | ExprKind::Cast(lhs, _) = expr.kind + && lhs.hir_id == prev_id + { + // keep going until we find a node that encompasses left of `binary` + prev_id = expr.hir_id; + continue; + } + + match node { + Node::Block(_) | Node::Stmt(_) => break, + _ => return Parens::Unneeded, + }; + } + + Parens::Needed } fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool { @@ -115,11 +126,11 @@ fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span (Some(FullInt::U(lv)), Some(FullInt::U(rv))) => lv < rv, _ => return, } { - span_ineffective_operation(cx, span, arg); + span_ineffective_operation(cx, span, arg, Parens::Unneeded); } } -fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) { +fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens) { if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) { let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() { ty::Int(ity) => unsext(cx.tcx, -1_i128, ity), @@ -132,19 +143,27 @@ fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) { 1 => v == 1, _ => unreachable!(), } { - span_ineffective_operation(cx, span, arg); + span_ineffective_operation(cx, span, arg, parens); } } } -fn span_ineffective_operation(cx: &LateContext<'_>, span: Span, arg: Span) { - span_lint( +fn span_ineffective_operation(cx: &LateContext<'_>, span: Span, arg: Span, parens: Parens) { + let mut applicability = Applicability::MachineApplicable; + let expr_snippet = snippet_with_applicability(cx, arg, "..", &mut applicability); + + let suggestion = match parens { + Parens::Needed => format!("({expr_snippet})"), + Parens::Unneeded => expr_snippet.into_owned(), + }; + + span_lint_and_sugg( cx, IDENTITY_OP, span, - &format!( - "the operation is ineffective. Consider reducing it to `{}`", - snippet(cx, arg, "..") - ), + "this operation has no effect", + "consider reducing it to", + suggestion, + applicability, ); } diff --git a/tests/ui/identity_op.fixed b/tests/ui/identity_op.fixed new file mode 100644 index 00000000000..5f9cebe212a --- /dev/null +++ b/tests/ui/identity_op.fixed @@ -0,0 +1,119 @@ +// run-rustfix + +#![warn(clippy::identity_op)] +#![allow( + clippy::eq_op, + clippy::no_effect, + clippy::unnecessary_operation, + clippy::op_ref, + clippy::double_parens, + unused +)] + +use std::fmt::Write as _; + +const ONE: i64 = 1; +const NEG_ONE: i64 = -1; +const ZERO: i64 = 0; + +struct A(String); + +impl std::ops::Shl for A { + type Output = A; + fn shl(mut self, other: i32) -> Self { + let _ = write!(self.0, "{}", other); + self + } +} + +struct Length(u8); +struct Meter; + +impl core::ops::Mul for u8 { + type Output = Length; + fn mul(self, _: Meter) -> Length { + Length(self) + } +} + +#[rustfmt::skip] +fn main() { + let x = 0; + + x; + x; + x + 1; + x; + 1 + x; + x - ZERO; //no error, as we skip lookups (for now) + x; + ((ZERO)) | x; //no error, as we skip lookups (for now) + + x; + x; + x / ONE; //no error, as we skip lookups (for now) + + x / 2; //no false positive + + x & NEG_ONE; //no error, as we skip lookups (for now) + x; + + let u: u8 = 0; + u; + + 1 << 0; // no error, this case is allowed, see issue 3430 + 42; + 1; + 42; + &x; + x; + + let mut a = A("".into()); + let b = a << 0; // no error: non-integer + + 1 * Meter; // no error: non-integer + + 2; + -2; + 2 + x; + -2 + x; + x + 1; + (x + 1) % 3; // no error + 4 % 3; // no error + 4 % -3; // no error + + // See #8724 + let a = 0; + let b = true; + (if b { 1 } else { 2 }); + (if b { 1 } else { 2 }) + if b { 3 } else { 4 }; + (match a { 0 => 10, _ => 20 }); + (match a { 0 => 10, _ => 20 }) + match a { 0 => 30, _ => 40 }; + (if b { 1 } else { 2 }) + match a { 0 => 30, _ => 40 }; + (match a { 0 => 10, _ => 20 }) + if b { 3 } else { 4 }; + (if b { 1 } else { 2 }); + + ({ a }) + 3; + ({ a } * 2); + (loop { let mut c = 0; if c == 10 { break c; } c += 1; }) + { a * 2 }; + + fn f(_: i32) { + todo!(); + } + f(a + { 8 * 5 }); + f(if b { 1 } else { 2 } + 3); + const _: i32 = { 2 * 4 } + 3; + const _: i32 = { 1 + 2 * 3 } + 3; + + a as usize; + let _ = a as usize; + ({ a } as usize); + + 2 * { a }; + (({ a } + 4)); + 1; +} + +pub fn decide(a: bool, b: bool) -> u32 { + (if a { 1 } else { 2 }) + if b { 3 } else { 5 } +} diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs index fec54d00ccb..ca799c9cfac 100644 --- a/tests/ui/identity_op.rs +++ b/tests/ui/identity_op.rs @@ -1,3 +1,15 @@ +// run-rustfix + +#![warn(clippy::identity_op)] +#![allow( + clippy::eq_op, + clippy::no_effect, + clippy::unnecessary_operation, + clippy::op_ref, + clippy::double_parens, + unused +)] + use std::fmt::Write as _; const ONE: i64 = 1; @@ -24,14 +36,6 @@ impl core::ops::Mul for u8 { } } -#[allow( - clippy::eq_op, - clippy::no_effect, - clippy::unnecessary_operation, - clippy::op_ref, - clippy::double_parens -)] -#[warn(clippy::identity_op)] #[rustfmt::skip] fn main() { let x = 0; @@ -82,29 +86,34 @@ fn main() { let a = 0; let b = true; 0 + if b { 1 } else { 2 }; - 0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }; // no error + 0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }; 0 + match a { 0 => 10, _ => 20 }; - 0 + match a { 0 => 10, _ => 20 } + match a { 0 => 30, _ => 40 }; // no error - 0 + if b { 1 } else { 2 } + match a { 0 => 30, _ => 40 }; // no error - 0 + match a { 0 => 10, _ => 20 } + if b { 3 } else { 4 }; // no error - - 0 + if b { 0 + 1 } else { 2 }; - 0 + match a { 0 => 0 + 10, _ => 20 }; - 0 + if b { 0 + 1 } else { 2 } + match a { 0 => 0 + 30, _ => 40 }; + 0 + match a { 0 => 10, _ => 20 } + match a { 0 => 30, _ => 40 }; + 0 + if b { 1 } else { 2 } + match a { 0 => 30, _ => 40 }; + 0 + match a { 0 => 10, _ => 20 } + if b { 3 } else { 4 }; + (if b { 1 } else { 2 }) + 0; - let _ = 0 + if 0 + 1 > 0 { 1 } else { 2 } + if 0 + 1 > 0 { 3 } else { 4 }; - let _ = 0 + match 0 + 1 { 0 => 10, _ => 20 } + match 0 + 1 { 0 => 30, _ => 40 }; + 0 + { a } + 3; + 0 + { a } * 2; + 0 + loop { let mut c = 0; if c == 10 { break c; } c += 1; } + { a * 2 }; - 0 + if b { 1 } else { 2 } + if b { 3 } else { 4 } + 0; - - 0 + { a } + 3; // no error - 0 + loop { let mut c = 0; if c == 10 { break c; } c += 1; } + { a * 2 }; // no error - fn f(_: i32) { todo!(); } f(1 * a + { 8 * 5 }); - f(0 + if b { 1 } else { 2 } + 3); // no error + f(0 + if b { 1 } else { 2 } + 3); const _: i32 = { 2 * 4 } + 0 + 3; - const _: i32 = 0 + { 1 + 2 * 3 } + 3; // no error + const _: i32 = 0 + { 1 + 2 * 3 } + 3; + + 0 + a as usize; + let _ = 0 + a as usize; + 0 + { a } as usize; + + 2 * (0 + { a }); + 1 * ({ a } + 4); + 1 * 1; +} + +pub fn decide(a: bool, b: bool) -> u32 { + 0 + if a { 1 } else { 2 } + if b { 3 } else { 5 } } diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr index d8cb65839cb..1a104a20b84 100644 --- a/tests/ui/identity_op.stderr +++ b/tests/ui/identity_op.stderr @@ -1,202 +1,238 @@ -error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:39:5 +error: this operation has no effect + --> $DIR/identity_op.rs:43:5 | LL | x + 0; - | ^^^^^ + | ^^^^^ help: consider reducing it to: `x` | = note: `-D clippy::identity-op` implied by `-D warnings` -error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:40:5 +error: this operation has no effect + --> $DIR/identity_op.rs:44:5 | LL | x + (1 - 1); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ help: consider reducing it to: `x` -error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:42:5 +error: this operation has no effect + --> $DIR/identity_op.rs:46:5 | LL | 0 + x; - | ^^^^^ + | ^^^^^ help: consider reducing it to: `x` -error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:45:5 - | -LL | x | (0); - | ^^^^^^^ - -error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:48:5 - | -LL | x * 1; - | ^^^^^ - -error: the operation is ineffective. Consider reducing it to `x` +error: this operation has no effect --> $DIR/identity_op.rs:49:5 | -LL | 1 * x; - | ^^^^^ +LL | x | (0); + | ^^^^^^^ help: consider reducing it to: `x` -error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:55:5 +error: this operation has no effect + --> $DIR/identity_op.rs:52:5 + | +LL | x * 1; + | ^^^^^ help: consider reducing it to: `x` + +error: this operation has no effect + --> $DIR/identity_op.rs:53:5 + | +LL | 1 * x; + | ^^^^^ help: consider reducing it to: `x` + +error: this operation has no effect + --> $DIR/identity_op.rs:59:5 | LL | -1 & x; - | ^^^^^^ + | ^^^^^^ help: consider reducing it to: `x` -error: the operation is ineffective. Consider reducing it to `u` - --> $DIR/identity_op.rs:58:5 - | -LL | u & 255; - | ^^^^^^^ - -error: the operation is ineffective. Consider reducing it to `42` - --> $DIR/identity_op.rs:61:5 - | -LL | 42 << 0; - | ^^^^^^^ - -error: the operation is ineffective. Consider reducing it to `1` +error: this operation has no effect --> $DIR/identity_op.rs:62:5 | -LL | 1 >> 0; - | ^^^^^^ +LL | u & 255; + | ^^^^^^^ help: consider reducing it to: `u` -error: the operation is ineffective. Consider reducing it to `42` - --> $DIR/identity_op.rs:63:5 - | -LL | 42 >> 0; - | ^^^^^^^ - -error: the operation is ineffective. Consider reducing it to `&x` - --> $DIR/identity_op.rs:64:5 - | -LL | &x >> 0; - | ^^^^^^^ - -error: the operation is ineffective. Consider reducing it to `x` +error: this operation has no effect --> $DIR/identity_op.rs:65:5 | -LL | x >> &0; - | ^^^^^^^ +LL | 42 << 0; + | ^^^^^^^ help: consider reducing it to: `42` -error: the operation is ineffective. Consider reducing it to `2` - --> $DIR/identity_op.rs:72:5 +error: this operation has no effect + --> $DIR/identity_op.rs:66:5 + | +LL | 1 >> 0; + | ^^^^^^ help: consider reducing it to: `1` + +error: this operation has no effect + --> $DIR/identity_op.rs:67:5 + | +LL | 42 >> 0; + | ^^^^^^^ help: consider reducing it to: `42` + +error: this operation has no effect + --> $DIR/identity_op.rs:68:5 + | +LL | &x >> 0; + | ^^^^^^^ help: consider reducing it to: `&x` + +error: this operation has no effect + --> $DIR/identity_op.rs:69:5 + | +LL | x >> &0; + | ^^^^^^^ help: consider reducing it to: `x` + +error: this operation has no effect + --> $DIR/identity_op.rs:76:5 | LL | 2 % 3; - | ^^^^^ + | ^^^^^ help: consider reducing it to: `2` -error: the operation is ineffective. Consider reducing it to `-2` - --> $DIR/identity_op.rs:73:5 +error: this operation has no effect + --> $DIR/identity_op.rs:77:5 | LL | -2 % 3; - | ^^^^^^ + | ^^^^^^ help: consider reducing it to: `-2` -error: the operation is ineffective. Consider reducing it to `2` - --> $DIR/identity_op.rs:74:5 +error: this operation has no effect + --> $DIR/identity_op.rs:78:5 | LL | 2 % -3 + x; - | ^^^^^^ + | ^^^^^^ help: consider reducing it to: `2` -error: the operation is ineffective. Consider reducing it to `-2` - --> $DIR/identity_op.rs:75:5 +error: this operation has no effect + --> $DIR/identity_op.rs:79:5 | LL | -2 % -3 + x; - | ^^^^^^^ + | ^^^^^^^ help: consider reducing it to: `-2` -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:76:9 +error: this operation has no effect + --> $DIR/identity_op.rs:80:9 | LL | x + 1 % 3; - | ^^^^^ + | ^^^^^ help: consider reducing it to: `1` -error: the operation is ineffective. Consider reducing it to `if b { 1 } else { 2 }` - --> $DIR/identity_op.rs:84:5 +error: this operation has no effect + --> $DIR/identity_op.rs:88:5 | LL | 0 + if b { 1 } else { 2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(if b { 1 } else { 2 })` -error: the operation is ineffective. Consider reducing it to `match a { 0 => 10, _ => 20 }` - --> $DIR/identity_op.rs:86:5 +error: this operation has no effect + --> $DIR/identity_op.rs:89:5 + | +LL | 0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(if b { 1 } else { 2 })` + +error: this operation has no effect + --> $DIR/identity_op.rs:90:5 | LL | 0 + match a { 0 => 10, _ => 20 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(match a { 0 => 10, _ => 20 })` -error: the operation is ineffective. Consider reducing it to `if b { 0 + 1 } else { 2 }` +error: this operation has no effect --> $DIR/identity_op.rs:91:5 | -LL | 0 + if b { 0 + 1 } else { 2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | 0 + match a { 0 => 10, _ => 20 } + match a { 0 => 30, _ => 40 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(match a { 0 => 10, _ => 20 })` -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:91:16 - | -LL | 0 + if b { 0 + 1 } else { 2 }; - | ^^^^^ - -error: the operation is ineffective. Consider reducing it to `match a { 0 => 0 + 10, _ => 20 }` +error: this operation has no effect --> $DIR/identity_op.rs:92:5 | -LL | 0 + match a { 0 => 0 + 10, _ => 20 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | 0 + if b { 1 } else { 2 } + match a { 0 => 30, _ => 40 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(if b { 1 } else { 2 })` -error: the operation is ineffective. Consider reducing it to `10` - --> $DIR/identity_op.rs:92:25 +error: this operation has no effect + --> $DIR/identity_op.rs:93:5 | -LL | 0 + match a { 0 => 0 + 10, _ => 20 }; - | ^^^^^^ +LL | 0 + match a { 0 => 10, _ => 20 } + if b { 3 } else { 4 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(match a { 0 => 10, _ => 20 })` -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:93:16 +error: this operation has no effect + --> $DIR/identity_op.rs:94:5 | -LL | 0 + if b { 0 + 1 } else { 2 } + match a { 0 => 0 + 30, _ => 40 }; - | ^^^^^ +LL | (if b { 1 } else { 2 }) + 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(if b { 1 } else { 2 })` -error: the operation is ineffective. Consider reducing it to `30` - --> $DIR/identity_op.rs:93:52 +error: this operation has no effect + --> $DIR/identity_op.rs:96:5 | -LL | 0 + if b { 0 + 1 } else { 2 } + match a { 0 => 0 + 30, _ => 40 }; - | ^^^^^^ +LL | 0 + { a } + 3; + | ^^^^^^^^^ help: consider reducing it to: `({ a })` -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:95:20 +error: this operation has no effect + --> $DIR/identity_op.rs:97:5 | -LL | let _ = 0 + if 0 + 1 > 0 { 1 } else { 2 } + if 0 + 1 > 0 { 3 } else { 4 }; - | ^^^^^ +LL | 0 + { a } * 2; + | ^^^^^^^^^^^^^ help: consider reducing it to: `({ a } * 2)` -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:95:52 - | -LL | let _ = 0 + if 0 + 1 > 0 { 1 } else { 2 } + if 0 + 1 > 0 { 3 } else { 4 }; - | ^^^^^ - -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:96:23 - | -LL | let _ = 0 + match 0 + 1 { 0 => 10, _ => 20 } + match 0 + 1 { 0 => 30, _ => 40 }; - | ^^^^^ - -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:96:58 - | -LL | let _ = 0 + match 0 + 1 { 0 => 10, _ => 20 } + match 0 + 1 { 0 => 30, _ => 40 }; - | ^^^^^ - -error: the operation is ineffective. Consider reducing it to `0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }` +error: this operation has no effect --> $DIR/identity_op.rs:98:5 | -LL | 0 + if b { 1 } else { 2 } + if b { 3 } else { 4 } + 0; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | 0 + loop { let mut c = 0; if c == 10 { break c; } c += 1; } + { a * 2 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(loop { let mut c = 0; if c == 10 { break c; } c += 1; })` -error: the operation is ineffective. Consider reducing it to `a` - --> $DIR/identity_op.rs:106:7 +error: this operation has no effect + --> $DIR/identity_op.rs:103:7 | LL | f(1 * a + { 8 * 5 }); - | ^^^^^ + | ^^^^^ help: consider reducing it to: `a` -error: the operation is ineffective. Consider reducing it to `{ 2 * 4 }` - --> $DIR/identity_op.rs:108:20 +error: this operation has no effect + --> $DIR/identity_op.rs:104:7 + | +LL | f(0 + if b { 1 } else { 2 } + 3); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `if b { 1 } else { 2 }` + +error: this operation has no effect + --> $DIR/identity_op.rs:105:20 | LL | const _: i32 = { 2 * 4 } + 0 + 3; - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ help: consider reducing it to: `{ 2 * 4 }` -error: aborting due to 33 previous errors +error: this operation has no effect + --> $DIR/identity_op.rs:106:20 + | +LL | const _: i32 = 0 + { 1 + 2 * 3 } + 3; + | ^^^^^^^^^^^^^^^^^ help: consider reducing it to: `{ 1 + 2 * 3 }` + +error: this operation has no effect + --> $DIR/identity_op.rs:108:5 + | +LL | 0 + a as usize; + | ^^^^^^^^^^^^^^ help: consider reducing it to: `a as usize` + +error: this operation has no effect + --> $DIR/identity_op.rs:109:13 + | +LL | let _ = 0 + a as usize; + | ^^^^^^^^^^^^^^ help: consider reducing it to: `a as usize` + +error: this operation has no effect + --> $DIR/identity_op.rs:110:5 + | +LL | 0 + { a } as usize; + | ^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `({ a } as usize)` + +error: this operation has no effect + --> $DIR/identity_op.rs:112:9 + | +LL | 2 * (0 + { a }); + | ^^^^^^^^^^^ help: consider reducing it to: `{ a }` + +error: this operation has no effect + --> $DIR/identity_op.rs:113:5 + | +LL | 1 * ({ a } + 4); + | ^^^^^^^^^^^^^^^ help: consider reducing it to: `(({ a } + 4))` + +error: this operation has no effect + --> $DIR/identity_op.rs:114:5 + | +LL | 1 * 1; + | ^^^^^ help: consider reducing it to: `1` + +error: this operation has no effect + --> $DIR/identity_op.rs:118:5 + | +LL | 0 + if a { 1 } else { 2 } + if b { 3 } else { 5 } + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(if a { 1 } else { 2 })` + +error: aborting due to 39 previous errors diff --git a/tests/ui/modulo_one.rs b/tests/ui/modulo_one.rs index 678a312f66e..adff08e5d1e 100644 --- a/tests/ui/modulo_one.rs +++ b/tests/ui/modulo_one.rs @@ -1,5 +1,5 @@ #![warn(clippy::modulo_one)] -#![allow(clippy::no_effect, clippy::unnecessary_operation)] +#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::identity_op)] static STATIC_ONE: usize = 2 - 1; static STATIC_NEG_ONE: i64 = 1 - 2; diff --git a/tests/ui/modulo_one.stderr b/tests/ui/modulo_one.stderr index 03f460897fc..04ecdef5e99 100644 --- a/tests/ui/modulo_one.stderr +++ b/tests/ui/modulo_one.stderr @@ -38,14 +38,6 @@ error: any number modulo -1 will panic/overflow or result in 0 LL | i32::MIN % (-1); // also caught by rustc | ^^^^^^^^^^^^^^^ -error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/modulo_one.rs:13:22 - | -LL | const ONE: u32 = 1 * 1; - | ^^^^^ - | - = note: `-D clippy::identity-op` implied by `-D warnings` - error: any number modulo 1 will be 0 --> $DIR/modulo_one.rs:17:5 | @@ -64,5 +56,5 @@ error: any number modulo -1 will panic/overflow or result in 0 LL | INT_MIN % NEG_ONE; // also caught by rustc | ^^^^^^^^^^^^^^^^^ -error: aborting due to 10 previous errors +error: aborting due to 9 previous errors