Skip to content

Commit

Permalink
erasing_op lint ignored when output type is different from the non-co…
Browse files Browse the repository at this point in the history
…nst one
  • Loading branch information
Wigy committed Jan 2, 2022
1 parent b25dbc6 commit e8b6b2a
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 24 deletions.
26 changes: 19 additions & 7 deletions 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
Expand Down Expand Up @@ -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",
);
}
Expand Down
34 changes: 34 additions & 0 deletions tests/ui/erasing_op.rs
@@ -1,3 +1,34 @@
struct Length(u8);
struct Meter;

impl core::ops::Mul<Meter> 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<Vec1> for i32 {
type Output = Vec1;
fn mul(self, mut right: Vec1) -> Vec1 {
right.x *= self;
right
}
}

impl core::ops::Mul<i32> 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() {
Expand All @@ -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;
}
20 changes: 16 additions & 4 deletions tests/ui/erasing_op.stderr
@@ -1,22 +1,34 @@
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;
| ^^^^^
|
= 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

13 changes: 13 additions & 0 deletions tests/ui/identity_op.rs
Expand Up @@ -11,6 +11,17 @@ impl std::ops::Shl<i32> for A {
self
}
}

struct Length(u8);
struct Meter;

impl core::ops::Mul<Meter> for u8 {
type Output = Length;
fn mul(self, _: Meter) -> Length {
Length(self)
}
}

#[allow(
clippy::eq_op,
clippy::no_effect,
Expand Down Expand Up @@ -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
}
26 changes: 13 additions & 13 deletions tests/ui/identity_op.stderr
@@ -1,79 +1,79 @@
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;
| ^^^^^
|
= 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;
| ^^^^^^^
Expand Down

0 comments on commit e8b6b2a

Please sign in to comment.