Rewrite logic of `has_nontrivial_oprand`.

Check whether operator is overrided with a `struct` operand.
The struct here refers to `struct`, `enum`, `union`.
Add and fix test for `no_effect` lint.

3 files changed, 86 insertions, 85 deletions

diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs
index 66c013b4f35..e3930b0568d 100644
--- a/clippy_lints/src/no_effect.rs
+++ b/clippy_lints/src/no_effect.rs
@@ -10,6 +10,7 @@ use rustc_hir::{
 use rustc_infer::infer::TyCtxtInferExt as _;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty;
 use rustc_session::declare_lint_pass;
 use std::ops::Deref;
 
@@ -87,8 +88,13 @@ 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);
         // assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation`
-        if has_nontrivial_oprand(expr) {
+        if has_nontrivial_oprand(cx, expr) {
             return true;
         }
         if has_no_effect(cx, expr) {
@@ -157,66 +163,38 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool {
     false
 }
 
-fn has_nontrivial_oprand(expr: &Expr<'_>) -> bool {
-    if expr.span.from_expansion() {
-        return false;
-    }
-    return match peel_blocks(expr).kind {
-        ExprKind::Binary(_, lhs, rhs) => !check_nontrivial_operand(lhs, rhs),
-        _ => false,
-    };
-}
-
-fn check_nontrivial_operand(lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool {
-    // It's seem that impossable to check whether operator is overrided through context of this lint,
-    // so, this function assume user-defined binary operator is overrided with an side-effect.
-    // The definition of user-defined structure here is `tuple`, `array`, `struct`,
-    // it looks like a little bit simple, but useful.
-    // Althrough this will weaken the ability of this lint,
-    // less miss lint-fix happen.
-
-    // a closure to check whether expr belongs to user-defined structure
-    let closure = |expr: &Expr<'_>| -> bool {
-        match &expr.kind {
-            // check whether expr is a user-defined sturcture
-            ExprKind::Tup(..) | ExprKind::Array(..) | ExprKind::Struct(..) => true,
-            // resolve expr's path
-            ExprKind::Path(rustc_hir::QPath::Resolved(
-                _,
-                rustc_hir::Path {
-                    span: _,
-                    res,
-                    segments: _,
-                },
-            )) => {
-                match res {
-                    Res::Def(defkind, _) => match defkind {
-                        // user-defined
-                        DefKind::Struct | DefKind::Ctor(_, _) => true,
-                        _ => false,
-                    },
-                    _ => false,
-                };
-                false
-            },
-            _ => false,
-        }
-    };
+fn has_nontrivial_oprand(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 binary operator is overrided with an side-effect.
+    // The definition of user-defined structure here is `struct`, `enum`, `uniom`,
+    // Althrough this will weaken the ability of this lint, less error lint-fix happen.
+    match expr.kind {
+        ExprKind::Binary(_, lhs, rhs) => {
+            // get type of lhs and rhs
+            let tyck_result = cx.typeck_results();
+            let ty_lhs = tyck_result.expr_ty(lhs).kind();
+            let ty_rhs = tyck_result.expr_ty(rhs).kind();
+            // check whether lhs is a user-defined structure
+            // only need to check lhs in fact
+            let ud_lhs = match ty_lhs {
+                ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(),
+                _ => false,
+            };
+            let ud_rhs = match ty_rhs {
+                ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(),
+                _ => false,
+            };
 
-    let lhs_ud = closure(lhs);
-    let rhs_ud = closure(rhs);
-    // one of lhs or rhs is user-defined structure
-    if lhs_ud || rhs_ud {
-        return false;
+            // 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`.
+            (ud_lhs || ud_rhs) && tyck_result.is_method_call(expr)
+        },
+        _ => false,
     }
-    true
 }
 
 fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    if expr.span.from_expansion() {
-        return false;
-    }
-    match peel_blocks(expr).kind {
+    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..7ffdeef6582 100644
--- a/tests/ui/no_effect.rs
+++ b/tests/ui/no_effect.rs
@@ -9,6 +9,22 @@
     clippy::useless_vec
 )]
 
+use std::fmt::Display;
+use std::ops::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
+    }
+}
+
 struct Unit;
 struct Tuple(i32);
 struct Struct {
@@ -174,4 +190,11 @@ fn main() {
     GreetStruct1("world");
     GreetStruct2()("world");
     GreetStruct3 {}("world");
+
+    fn n() -> i32 {
+        42
+    }
+
+    Cout << 142;
+    Cout << n();
 }
diff --git a/tests/ui/no_effect.stderr b/tests/ui/no_effect.stderr
index feba35697f5..437e556a7a8 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:114: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:117:5
    |
 LL |     s2;
    |     ^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:103:5
+  --> $DIR/no_effect.rs:119:5
    |
 LL |     Unit;
    |     ^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:105:5
+  --> $DIR/no_effect.rs:121:5
    |
 LL |     Tuple(0);
    |     ^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:107:5
+  --> $DIR/no_effect.rs:123:5
    |
 LL |     Struct { field: 0 };
    |     ^^^^^^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:109:5
+  --> $DIR/no_effect.rs:125:5
    |
 LL |     Struct { ..s };
    |     ^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:111:5
+  --> $DIR/no_effect.rs:127:5
    |
 LL |     Union { a: 0 };
    |     ^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:113:5
+  --> $DIR/no_effect.rs:129:5
    |
 LL |     Enum::Tuple(0);
    |     ^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:115:5
+  --> $DIR/no_effect.rs:131:5
    |
 LL |     Enum::Struct { field: 0 };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:117:5
+  --> $DIR/no_effect.rs:133:5
    |
 LL |     5 + 6;
    |     ^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:119:5
+  --> $DIR/no_effect.rs:135:5
    |
 LL |     *&42;
    |     ^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:121:5
+  --> $DIR/no_effect.rs:137:5
    |
 LL |     &6;
    |     ^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:123:5
+  --> $DIR/no_effect.rs:139:5
    |
 LL |     (5, 6, 7);
    |     ^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:125:5
+  --> $DIR/no_effect.rs:141:5
    |
 LL |     ..;
    |     ^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:127:5
+  --> $DIR/no_effect.rs:143:5
    |
 LL |     5..;
    |     ^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:129:5
+  --> $DIR/no_effect.rs:145:5
    |
 LL |     ..5;
    |     ^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:131:5
+  --> $DIR/no_effect.rs:147:5
    |
 LL |     5..6;
    |     ^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:133:5
+  --> $DIR/no_effect.rs:149:5
    |
 LL |     5..=6;
    |     ^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:135:5
+  --> $DIR/no_effect.rs:151:5
    |
 LL |     [42, 55];
    |     ^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:137:5
+  --> $DIR/no_effect.rs:153:5
    |
 LL |     [42, 55][1];
    |     ^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:139:5
+  --> $DIR/no_effect.rs:155:5
    |
 LL |     (42, 55).1;
    |     ^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:141:5
+  --> $DIR/no_effect.rs:157:5
    |
 LL |     [42; 55];
    |     ^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:143:5
+  --> $DIR/no_effect.rs:159:5
    |
 LL |     [42; 55][13];
    |     ^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:146:5
+  --> $DIR/no_effect.rs:162:5
    |
 LL |     || x += 5;
    |     ^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:149:5
+  --> $DIR/no_effect.rs:165:5
    |
 LL |     FooString { s: s };
    |     ^^^^^^^^^^^^^^^^^^^
 
 error: binding to `_` prefixed variable with no side-effect
-  --> $DIR/no_effect.rs:151:5
+  --> $DIR/no_effect.rs:167: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:170: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:172: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:174:5
    |
 LL |     let _cat = [2, 4, 6, 8][2];
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^