From e8b6b2ac0c115e7208d725b4f6340e68194d0cf3 Mon Sep 17 00:00:00 2001 From: Wigy Date: Fri, 31 Dec 2021 09:37:39 +0100 Subject: [PATCH] erasing_op lint ignored when output type is different from the non-const one --- clippy_lints/src/erasing_op.rs | 26 +++++++++++++++++++------- tests/ui/erasing_op.rs | 34 ++++++++++++++++++++++++++++++++++ tests/ui/erasing_op.stderr | 20 ++++++++++++++++---- tests/ui/identity_op.rs | 13 +++++++++++++ tests/ui/identity_op.stderr | 26 +++++++++++++------------- 5 files changed, 95 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/erasing_op.rs b/clippy_lints/src/erasing_op.rs index d49cec26be5..bb6acd8c5dd 100644 --- a/clippy_lints/src/erasing_op.rs +++ b/clippy_lints/src/erasing_op.rs @@ -1,9 +1,11 @@ use clippy_utils::consts::{constant_simple, Constant}; use clippy_utils::diagnostics::span_lint; +use clippy_utils::ty::same_type_and_consts; + use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TypeckResults; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::Span; declare_clippy_lint! { /// ### What it does @@ -35,24 +37,34 @@ impl<'tcx> LateLintPass<'tcx> for ErasingOp { return; } if let ExprKind::Binary(ref cmp, left, right) = e.kind { + let tck = cx.typeck_results(); match cmp.node { BinOpKind::Mul | BinOpKind::BitAnd => { - check(cx, left, e.span); - check(cx, right, e.span); + check(cx, tck, left, right, e); + check(cx, tck, right, left, e); }, - BinOpKind::Div => check(cx, left, e.span), + BinOpKind::Div => check(cx, tck, left, right, e), _ => (), } } } } -fn check(cx: &LateContext<'_>, e: &Expr<'_>, span: Span) { - if constant_simple(cx, cx.typeck_results(), e) == Some(Constant::Int(0)) { +fn different_types(tck: &TypeckResults<'tcx>, input: &'tcx Expr<'_>, output: &'tcx Expr<'_>) -> bool { + let input_ty = tck.expr_ty(input).peel_refs(); + let output_ty = tck.expr_ty(output).peel_refs(); + !same_type_and_consts(input_ty, output_ty) +} + +fn check(cx: &LateContext<'cx>, tck: &TypeckResults<'cx>, op: &Expr<'cx>, other: &Expr<'cx>, parent: &Expr<'cx>) { + if constant_simple(cx, tck, op) == Some(Constant::Int(0)) { + if different_types(tck, other, parent) { + return; + } span_lint( cx, ERASING_OP, - span, + parent.span, "this operation will always return zero. This is likely not the intended outcome", ); } diff --git a/tests/ui/erasing_op.rs b/tests/ui/erasing_op.rs index 1540062a4bc..ae2fad0086d 100644 --- a/tests/ui/erasing_op.rs +++ b/tests/ui/erasing_op.rs @@ -1,3 +1,34 @@ +struct Length(u8); +struct Meter; + +impl core::ops::Mul for u8 { + type Output = Length; + fn mul(self, _: Meter) -> Length { + Length(self) + } +} + +#[derive(Clone, Default, PartialEq, Eq, Hash)] +struct Vec1 { + x: i32, +} + +impl core::ops::Mul for i32 { + type Output = Vec1; + fn mul(self, mut right: Vec1) -> Vec1 { + right.x *= self; + right + } +} + +impl core::ops::Mul for Vec1 { + type Output = Vec1; + fn mul(mut self, right: i32) -> Vec1 { + self.x *= right; + self + } +} + #[allow(clippy::no_effect)] #[warn(clippy::erasing_op)] fn main() { @@ -6,4 +37,7 @@ fn main() { x * 0; 0 & x; 0 / x; + 0 * Meter; // no error: Output type is different from the non-zero argument + 0 * Vec1 { x: 5 }; + Vec1 { x: 5 } * 0; } diff --git a/tests/ui/erasing_op.stderr b/tests/ui/erasing_op.stderr index e54ce85f98e..165ed9bfe58 100644 --- a/tests/ui/erasing_op.stderr +++ b/tests/ui/erasing_op.stderr @@ -1,5 +1,5 @@ error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:6:5 + --> $DIR/erasing_op.rs:37:5 | LL | x * 0; | ^^^^^ @@ -7,16 +7,28 @@ LL | x * 0; = note: `-D clippy::erasing-op` implied by `-D warnings` error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:7:5 + --> $DIR/erasing_op.rs:38:5 | LL | 0 & x; | ^^^^^ error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:8:5 + --> $DIR/erasing_op.rs:39:5 | LL | 0 / x; | ^^^^^ -error: aborting due to 3 previous errors +error: this operation will always return zero. This is likely not the intended outcome + --> $DIR/erasing_op.rs:41:5 + | +LL | 0 * Vec1 { x: 5 }; + | ^^^^^^^^^^^^^^^^^ + +error: this operation will always return zero. This is likely not the intended outcome + --> $DIR/erasing_op.rs:42:5 + | +LL | Vec1 { x: 5 } * 0; + | ^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs index 2ed4b5db574..12bbda71f43 100644 --- a/tests/ui/identity_op.rs +++ b/tests/ui/identity_op.rs @@ -11,6 +11,17 @@ impl std::ops::Shl for A { self } } + +struct Length(u8); +struct Meter; + +impl core::ops::Mul for u8 { + type Output = Length; + fn mul(self, _: Meter) -> Length { + Length(self) + } +} + #[allow( clippy::eq_op, clippy::no_effect, @@ -53,4 +64,6 @@ fn main() { let mut a = A("".into()); let b = a << 0; // no error: non-integer + + 1 * Meter; // no error: non-integer } diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr index ff34b38db01..0103cf5457e 100644 --- a/tests/ui/identity_op.stderr +++ b/tests/ui/identity_op.stderr @@ -1,5 +1,5 @@ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:26:5 + --> $DIR/identity_op.rs:37:5 | LL | x + 0; | ^^^^^ @@ -7,73 +7,73 @@ LL | x + 0; = note: `-D clippy::identity-op` implied by `-D warnings` error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:27:5 + --> $DIR/identity_op.rs:38:5 | LL | x + (1 - 1); | ^^^^^^^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:29:5 + --> $DIR/identity_op.rs:40:5 | LL | 0 + x; | ^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:32:5 + --> $DIR/identity_op.rs:43:5 | LL | x | (0); | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:35:5 + --> $DIR/identity_op.rs:46:5 | LL | x * 1; | ^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:36:5 + --> $DIR/identity_op.rs:47:5 | LL | 1 * x; | ^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:42:5 + --> $DIR/identity_op.rs:53:5 | LL | -1 & x; | ^^^^^^ error: the operation is ineffective. Consider reducing it to `u` - --> $DIR/identity_op.rs:45:5 + --> $DIR/identity_op.rs:56:5 | LL | u & 255; | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `42` - --> $DIR/identity_op.rs:48:5 + --> $DIR/identity_op.rs:59:5 | LL | 42 << 0; | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `1` - --> $DIR/identity_op.rs:49:5 + --> $DIR/identity_op.rs:60:5 | LL | 1 >> 0; | ^^^^^^ error: the operation is ineffective. Consider reducing it to `42` - --> $DIR/identity_op.rs:50:5 + --> $DIR/identity_op.rs:61:5 | LL | 42 >> 0; | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `&x` - --> $DIR/identity_op.rs:51:5 + --> $DIR/identity_op.rs:62:5 | LL | &x >> 0; | ^^^^^^^ error: the operation is ineffective. Consider reducing it to `x` - --> $DIR/identity_op.rs:52:5 + --> $DIR/identity_op.rs:63:5 | LL | x >> &0; | ^^^^^^^