Auto merge of #11907 - cocodery:issue11885, r=y21,xFrednet

Add a function to check whether binary oprands are nontrivial

fixes [#issue11885](https://github.com/rust-lang/rust-clippy/issues/11885)

It's hard to check whether operator is overrided through context of lint.
So, assume non-trivial structure like tuple, array or sturt, using a overrided binary operator in this lint, which might cause a side effict.
This is not detected before.
Althrough this might weaken the ability of this lint, it may more useful than before. Maybe this lint will cause an error, but now, it not. And assuming side effect of non-trivial structure with operator  is not a bad thing, right?

changelog: Fix: [`no_effect`] check if binary operands are nontrivial

6 files changed, 147 insertions, 52 deletions

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 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect {
 
 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<T> Shl<T> 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<T> Shl<T> 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<T> Shl<T> 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 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"),