From 56d20c2b53db2c2d39ee3e8368c5cd556c35f5b0 Mon Sep 17 00:00:00 2001 From: cocodery Date: Fri, 8 Dec 2023 21:33:28 +0800 Subject: [PATCH] Fix nits and add test for `unnecessary_operation` --- clippy_lints/src/no_effect.rs | 16 ++++++----- tests/ui/unnecessary_operation.fixed | 19 ++++++++++++++ tests/ui/unnecessary_operation.rs | 19 ++++++++++++++ tests/ui/unnecessary_operation.stderr | 38 +++++++++++++-------------- 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index a0e7dbf9ec6b5..5978da8319932 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -92,8 +92,10 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { return false; } let expr = peel_blocks(expr); - // assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation` - if is_operator_overriden(cx, expr) { + + if is_operator_overridden(cx, expr) { + // Return `true`, to prevent `check_unnecessary_operation` from + // linting on this statement as well. return true; } if has_no_effect(cx, expr) { @@ -162,18 +164,18 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { false } -fn is_operator_overriden(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - // It's very hard or impossable to check whether overrided operator have side-effect this lint. - // So, this function assume user-defined operator is overrided with an side-effect. +fn is_operator_overridden(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + // It's very hard or impossable to check whether overridden operator have side-effect this lint. + // So, this function assume user-defined operator is overridden with an side-effect. // The definition of user-defined structure here is ADT-type, // Althrough this will weaken the ability of this lint, less error lint-fix happen. match expr.kind { ExprKind::Binary(..) | ExprKind::Unary(..) => { // No need to check type of `lhs` and `rhs` - // because if the operator is overrided, at least one operand is ADT type + // because if the operator is overridden, at least one operand is ADT type // reference: rust/compiler/rustc_middle/src/ty/typeck_results.rs: `is_method_call`. - // use this function to check whether operator is overrided in `ExprKind::{Binary, Unary}`. + // use this function to check whether operator is overridden in `ExprKind::{Binary, Unary}`. cx.typeck_results().is_method_call(expr) }, _ => false, diff --git a/tests/ui/unnecessary_operation.fixed b/tests/ui/unnecessary_operation.fixed index d0c0298ef4cd4..463412daec08d 100644 --- a/tests/ui/unnecessary_operation.fixed +++ b/tests/ui/unnecessary_operation.fixed @@ -7,6 +7,9 @@ )] #![warn(clippy::unnecessary_operation)] +use std::fmt::Display; +use std::ops::Shl; + struct Tuple(i32); struct Struct { field: i32, @@ -50,6 +53,19 @@ fn get_drop_struct() -> DropStruct { DropStruct { field: 0 } } +struct Cout; + +impl Shl for Cout +where + T: Display, +{ + type Output = Self; + fn shl(self, rhs: T) -> Self::Output { + println!("{}", rhs); + self + } +} + fn main() { get_number(); get_number(); @@ -87,4 +103,7 @@ fn main() { ($($e:expr),*) => {{ $($e;)* }} } use_expr!(isize::MIN / -(one() as isize), i8::MIN / -one()); + + // Issue #11885 + Cout << 16; } diff --git a/tests/ui/unnecessary_operation.rs b/tests/ui/unnecessary_operation.rs index e8e3a2d5657de..f0d28e2890291 100644 --- a/tests/ui/unnecessary_operation.rs +++ b/tests/ui/unnecessary_operation.rs @@ -7,6 +7,9 @@ )] #![warn(clippy::unnecessary_operation)] +use std::fmt::Display; +use std::ops::Shl; + struct Tuple(i32); struct Struct { field: i32, @@ -50,6 +53,19 @@ fn get_drop_struct() -> DropStruct { DropStruct { field: 0 } } +struct Cout; + +impl Shl for Cout +where + T: Display, +{ + type Output = Self; + fn shl(self, rhs: T) -> Self::Output { + println!("{}", rhs); + self + } +} + fn main() { Tuple(get_number()); Struct { field: get_number() }; @@ -91,4 +107,7 @@ fn main() { ($($e:expr),*) => {{ $($e;)* }} } use_expr!(isize::MIN / -(one() as isize), i8::MIN / -one()); + + // Issue #11885 + Cout << 16; } diff --git a/tests/ui/unnecessary_operation.stderr b/tests/ui/unnecessary_operation.stderr index fbe495f518fab..eeee9ad60068b 100644 --- a/tests/ui/unnecessary_operation.stderr +++ b/tests/ui/unnecessary_operation.stderr @@ -1,5 +1,5 @@ error: unnecessary operation - --> $DIR/unnecessary_operation.rs:54:5 + --> $DIR/unnecessary_operation.rs:70:5 | LL | Tuple(get_number()); | ^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` @@ -8,103 +8,103 @@ LL | Tuple(get_number()); = help: to override `-D warnings` add `#[allow(clippy::unnecessary_operation)]` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:55:5 + --> $DIR/unnecessary_operation.rs:71:5 | LL | Struct { field: get_number() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:56:5 + --> $DIR/unnecessary_operation.rs:72:5 | LL | Struct { ..get_struct() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_struct();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:57:5 + --> $DIR/unnecessary_operation.rs:73:5 | LL | Enum::Tuple(get_number()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:58:5 + --> $DIR/unnecessary_operation.rs:74:5 | LL | Enum::Struct { field: get_number() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:59:5 + --> $DIR/unnecessary_operation.rs:75:5 | LL | 5 + get_number(); | ^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:60:5 + --> $DIR/unnecessary_operation.rs:76:5 | LL | *&get_number(); | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:61:5 + --> $DIR/unnecessary_operation.rs:77:5 | LL | &get_number(); | ^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:62:5 + --> $DIR/unnecessary_operation.rs:78:5 | LL | (5, 6, get_number()); | ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;6;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:63:5 + --> $DIR/unnecessary_operation.rs:79:5 | LL | get_number()..; | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:64:5 + --> $DIR/unnecessary_operation.rs:80:5 | LL | ..get_number(); | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:65:5 + --> $DIR/unnecessary_operation.rs:81:5 | LL | 5..get_number(); | ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:66:5 + --> $DIR/unnecessary_operation.rs:82:5 | LL | [42, get_number()]; | ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:67:5 + --> $DIR/unnecessary_operation.rs:83:5 | LL | [42, 55][get_usize()]; | ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42, 55].len() > get_usize());` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:68:5 + --> $DIR/unnecessary_operation.rs:84:5 | LL | (42, get_number()).1; | ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:69:5 + --> $DIR/unnecessary_operation.rs:85:5 | LL | [get_number(); 55]; | ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:70:5 + --> $DIR/unnecessary_operation.rs:86:5 | LL | [42; 55][get_usize()]; | ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42; 55].len() > get_usize());` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:71:5 + --> $DIR/unnecessary_operation.rs:87:5 | LL | / { LL | | get_number() @@ -112,7 +112,7 @@ LL | | }; | |______^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:74:5 + --> $DIR/unnecessary_operation.rs:90:5 | LL | / FooString { LL | | s: String::from("blah"),