From f58bb5b234ec5f0ead463d5ce387771e72aaa865 Mon Sep 17 00:00:00 2001
From: Shotaro Yamada <sinkuu@sinkuu.xyz>
Date: Fri, 17 Apr 2020 13:09:02 +0900
Subject: [PATCH] question_mark: don't add `as_ref()` for a call expression

---
 clippy_lints/src/question_mark.rs |  8 +++++---
 tests/ui/question_mark.fixed      | 12 ++++++++++++
 tests/ui/question_mark.rs         | 14 ++++++++++++++
 tests/ui/question_mark.stderr     | 10 +++++++++-
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
index 28c1d9753093..ea654467b866 100644
--- a/clippy_lints/src/question_mark.rs
+++ b/clippy_lints/src/question_mark.rs
@@ -70,10 +70,12 @@ impl QuestionMark {
                             replacement = Some(format!("Some({}?)", receiver_str));
                         }
                     }
-                } else if Self::moves_by_default(cx, subject) {
-                        replacement = Some(format!("{}.as_ref()?;", receiver_str));
+                } else if Self::moves_by_default(cx, subject)
+                    && !matches!(subject.kind, ExprKind::Call(..) | ExprKind::MethodCall(..))
+                {
+                    replacement = Some(format!("{}.as_ref()?;", receiver_str));
                 } else {
-                        replacement = Some(format!("{}?;", receiver_str));
+                    replacement = Some(format!("{}?;", receiver_str));
                 }
 
                 if let Some(replacement_str) = replacement {
diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed
index 2c3e4989d534..11dff94a2886 100644
--- a/tests/ui/question_mark.fixed
+++ b/tests/ui/question_mark.fixed
@@ -93,6 +93,16 @@ impl MoveStruct {
     }
 }
 
+fn func() -> Option<i32> {
+    fn f() -> Option<String> {
+        Some(String::new())
+    }
+
+    f()?;
+
+    Some(0)
+}
+
 fn main() {
     some_func(Some(42));
     some_func(None);
@@ -110,4 +120,6 @@ fn main() {
 
     let so = SeemsOption::Some(45);
     returns_something_similar_to_option(so);
+
+    func();
 }
diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs
index 24df76344356..1d0ee82b4f77 100644
--- a/tests/ui/question_mark.rs
+++ b/tests/ui/question_mark.rs
@@ -121,6 +121,18 @@ impl MoveStruct {
     }
 }
 
+fn func() -> Option<i32> {
+    fn f() -> Option<String> {
+        Some(String::new())
+    }
+
+    if f().is_none() {
+        return None;
+    }
+
+    Some(0)
+}
+
 fn main() {
     some_func(Some(42));
     some_func(None);
@@ -138,4 +150,6 @@ fn main() {
 
     let so = SeemsOption::Some(45);
     returns_something_similar_to_option(so);
+
+    func();
 }
diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr
index 97741069b50a..502615fb175a 100644
--- a/tests/ui/question_mark.stderr
+++ b/tests/ui/question_mark.stderr
@@ -92,5 +92,13 @@ LL | |             return None;
 LL | |         };
    | |_________^ help: replace it with: `self.opt?`
 
-error: aborting due to 10 previous errors
+error: this block may be rewritten with the `?` operator
+  --> $DIR/question_mark.rs:129:5
+   |
+LL | /     if f().is_none() {
+LL | |         return None;
+LL | |     }
+   | |_____^ help: replace it with: `f()?;`
+
+error: aborting due to 11 previous errors