From: Michael Wright <mikerite@lavabit.com>
Date: Tue, 29 Jan 2019 05:22:08 +0000 (+0200)
Subject: Fix `unit_arg` false positive
X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=df04238d3aea4b8b105c286b49d54dd77c2b5f83;p=rust.git

Fix `unit_arg` false positive

Ignore arguments with the question mark operator.

Closes #2945
---

diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index 4683ffb4c85..94c83ed5720 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -609,36 +609,43 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
         if in_macro(expr.span) {
             return;
         }
+
+        // apparently stuff in the desugaring of `?` can trigger this
+        // so check for that here
+        // only the calls to `Try::from_error` is marked as desugared,
+        // so we need to check both the current Expr and its parent.
+        if is_questionmark_desugar_marked_call(expr) {
+            return;
+        }
+        if_chain! {
+            let map = &cx.tcx.hir();
+            let opt_parent_node = map.find(map.get_parent_node(expr.id));
+            if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
+            if is_questionmark_desugar_marked_call(parent_expr);
+            then {
+                return;
+            }
+        }
+
         match expr.node {
             ExprKind::Call(_, ref args) | ExprKind::MethodCall(_, _, ref args) => {
                 for arg in args {
                     if is_unit(cx.tables.expr_ty(arg)) && !is_unit_literal(arg) {
-                        let map = &cx.tcx.hir();
-                        // apparently stuff in the desugaring of `?` can trigger this
-                        // so check for that here
-                        // only the calls to `Try::from_error` is marked as desugared,
-                        // so we need to check both the current Expr and its parent.
-                        if !is_questionmark_desugar_marked_call(expr) {
-                            if_chain! {
-                                let opt_parent_node = map.find(map.get_parent_node(expr.id));
-                                if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
-                                if is_questionmark_desugar_marked_call(parent_expr);
-                                then {}
-                                else {
-                                    // `expr` and `parent_expr` where _both_ not from
-                                    // desugaring `?`, so lint
-                                    span_lint_and_sugg(
-                                        cx,
-                                        UNIT_ARG,
-                                        arg.span,
-                                        "passing a unit value to a function",
-                                        "if you intended to pass a unit value, use a unit literal instead",
-                                        "()".to_string(),
-                                        Applicability::MachineApplicable,
-                                    );
-                                }
+                        if let ExprKind::Match(.., match_source) = &arg.node {
+                            if *match_source == MatchSource::TryDesugar {
+                                continue;
                             }
                         }
+
+                        span_lint_and_sugg(
+                            cx,
+                            UNIT_ARG,
+                            arg.span,
+                            "passing a unit value to a function",
+                            "if you intended to pass a unit value, use a unit literal instead",
+                            "()".to_string(),
+                            Applicability::MachineApplicable,
+                        );
                     }
                 }
             },
diff --git a/tests/ui/unit_arg.fixed b/tests/ui/unit_arg.fixed
index d8f3e854ca9..cf146c91f6d 100644
--- a/tests/ui/unit_arg.fixed
+++ b/tests/ui/unit_arg.fixed
@@ -47,6 +47,17 @@ fn question_mark() -> Result<(), ()> {
     Ok(())
 }
 
+#[allow(dead_code)]
+mod issue_2945 {
+    fn unit_fn() -> Result<(), i32> {
+        Ok(())
+    }
+
+    fn fallible() -> Result<(), i32> {
+        Ok(unit_fn()?)
+    }
+}
+
 fn main() {
     bad();
     ok();
diff --git a/tests/ui/unit_arg.rs b/tests/ui/unit_arg.rs
index 1403870eacf..c15b0a50045 100644
--- a/tests/ui/unit_arg.rs
+++ b/tests/ui/unit_arg.rs
@@ -54,6 +54,17 @@ fn question_mark() -> Result<(), ()> {
     Ok(())
 }
 
+#[allow(dead_code)]
+mod issue_2945 {
+    fn unit_fn() -> Result<(), i32> {
+        Ok(())
+    }
+
+    fn fallible() -> Result<(), i32> {
+        Ok(unit_fn()?)
+    }
+}
+
 fn main() {
     bad();
     ok();