diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 6e65dd628a4..5978da83199 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -87,6 +87,17 @@ fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { if let StmtKind::Semi(expr) = stmt.kind { + // move `expr.span.from_expansion()` ahead + if expr.span.from_expansion() { + return false; + } + let expr = peel_blocks(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) { span_lint_hir_and_then( cx, @@ -153,11 +164,26 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { false } -fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - if expr.span.from_expansion() { - return false; +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 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 overridden in `ExprKind::{Binary, Unary}`. + cx.typeck_results().is_method_call(expr) + }, + _ => false, } - match peel_blocks(expr).kind { +} + +fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + match expr.kind { ExprKind::Lit(..) | ExprKind::Closure { .. } => true, ExprKind::Path(..) => !has_drop(cx, cx.typeck_results().expr_ty(expr)), ExprKind::Index(a, b, _) | ExprKind::Binary(_, a, b) => has_no_effect(cx, a) && has_no_effect(cx, b), diff --git a/tests/ui/no_effect.rs b/tests/ui/no_effect.rs index c52f4389192..777b1e52c2d 100644 --- a/tests/ui/no_effect.rs +++ b/tests/ui/no_effect.rs @@ -9,6 +9,30 @@ clippy::useless_vec )] +use std::fmt::Display; +use std::ops::{Neg, Shl}; + +struct Cout; + +impl Shl for Cout +where + T: Display, +{ + type Output = Self; + fn shl(self, rhs: T) -> Self::Output { + println!("{}", rhs); + self + } +} + +impl Neg for Cout { + type Output = Self; + fn neg(self) -> Self::Output { + println!("hello world"); + self + } +} + struct Unit; struct Tuple(i32); struct Struct { @@ -174,4 +198,11 @@ fn main() { GreetStruct1("world"); GreetStruct2()("world"); GreetStruct3 {}("world"); + + fn n() -> i32 { + 42 + } + + Cout << 142; + -Cout; } diff --git a/tests/ui/no_effect.stderr b/tests/ui/no_effect.stderr index feba35697f5..f5ba234b4cb 100644 --- a/tests/ui/no_effect.stderr +++ b/tests/ui/no_effect.stderr @@ -1,5 +1,5 @@ error: statement with no effect - --> $DIR/no_effect.rs:98:5 + --> $DIR/no_effect.rs:122:5 | LL | 0; | ^^ @@ -8,151 +8,151 @@ LL | 0; = help: to override `-D warnings` add `#[allow(clippy::no_effect)]` error: statement with no effect - --> $DIR/no_effect.rs:101:5 + --> $DIR/no_effect.rs:125:5 | LL | s2; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:103:5 + --> $DIR/no_effect.rs:127:5 | LL | Unit; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:105:5 + --> $DIR/no_effect.rs:129:5 | LL | Tuple(0); | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:107:5 + --> $DIR/no_effect.rs:131:5 | LL | Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:109:5 + --> $DIR/no_effect.rs:133:5 | LL | Struct { ..s }; | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:111:5 + --> $DIR/no_effect.rs:135:5 | LL | Union { a: 0 }; | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:113:5 + --> $DIR/no_effect.rs:137:5 | LL | Enum::Tuple(0); | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:115:5 + --> $DIR/no_effect.rs:139:5 | LL | Enum::Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:117:5 + --> $DIR/no_effect.rs:141:5 | LL | 5 + 6; | ^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:119:5 + --> $DIR/no_effect.rs:143:5 | LL | *&42; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:121:5 + --> $DIR/no_effect.rs:145:5 | LL | &6; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:123:5 + --> $DIR/no_effect.rs:147:5 | LL | (5, 6, 7); | ^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:125:5 + --> $DIR/no_effect.rs:149:5 | LL | ..; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:127:5 + --> $DIR/no_effect.rs:151:5 | LL | 5..; | ^^^^ error: statement with no effect - --> $DIR/no_effect.rs:129:5 + --> $DIR/no_effect.rs:153:5 | LL | ..5; | ^^^^ error: statement with no effect - --> $DIR/no_effect.rs:131:5 + --> $DIR/no_effect.rs:155:5 | LL | 5..6; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:133:5 + --> $DIR/no_effect.rs:157:5 | LL | 5..=6; | ^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:135:5 + --> $DIR/no_effect.rs:159:5 | LL | [42, 55]; | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:137:5 + --> $DIR/no_effect.rs:161:5 | LL | [42, 55][1]; | ^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:139:5 + --> $DIR/no_effect.rs:163:5 | LL | (42, 55).1; | ^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:141:5 + --> $DIR/no_effect.rs:165:5 | LL | [42; 55]; | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:143:5 + --> $DIR/no_effect.rs:167:5 | LL | [42; 55][13]; | ^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:146:5 + --> $DIR/no_effect.rs:170:5 | LL | || x += 5; | ^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:149:5 + --> $DIR/no_effect.rs:173:5 | LL | FooString { s: s }; | ^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:151:5 + --> $DIR/no_effect.rs:175:5 | LL | let _unused = 1; | ^^^^^^^^^^^^^^^^ @@ -161,19 +161,19 @@ LL | let _unused = 1; = help: to override `-D warnings` add `#[allow(clippy::no_effect_underscore_binding)]` error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:154:5 + --> $DIR/no_effect.rs:178:5 | LL | let _penguin = || println!("Some helpful closure"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:156:5 + --> $DIR/no_effect.rs:180:5 | LL | let _duck = Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:158:5 + --> $DIR/no_effect.rs:182:5 | LL | let _cat = [2, 4, 6, 8][2]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/unnecessary_operation.fixed b/tests/ui/unnecessary_operation.fixed index d0c0298ef4c..463412daec0 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 e8e3a2d5657..f0d28e28902 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 @@ macro_rules! use_expr { ($($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 fbe495f518f..eeee9ad6006 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"),