Auto merge of #5481 - sinkuu:no_as_ref, r=phansch

question_mark: don't add `as_ref()` for a call expression

If a call returns a `!Copy` value, it does so regardless of whether `as_ref()` is added. For example, `foo.into_option().as_ref()?` can be simplified to `foo.into_option()?`.

---

changelog: Improved `question_mark` lint suggestion so that it doesn't add redundant `as_ref()`

4 files changed, 40 insertions, 4 deletions

diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
index 28c1d975309..ea654467b86 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 2c3e4989d53..11dff94a288 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 24df7634435..1d0ee82b4f7 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 97741069b50..502615fb175 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