about summary refs log tree commit diff
diff options
context:
space:
mode:
authorShotaro Yamada <sinkuu@sinkuu.xyz>2020-04-17 13:09:02 +0900
committerShotaro Yamada <sinkuu@sinkuu.xyz>2020-04-17 13:09:02 +0900
commitf58bb5b234ec5f0ead463d5ce387771e72aaa865 (patch)
tree8b64cbaf883753dc9797dc0a9b1b78facfaa8689
parenteb9c15a6b38d6b1a64f7e6a1360d533014c0e797 (diff)
downloadrust-f58bb5b234ec5f0ead463d5ce387771e72aaa865.tar.gz
rust-f58bb5b234ec5f0ead463d5ce387771e72aaa865.zip
question_mark: don't add `as_ref()` for a call expression
-rw-r--r--clippy_lints/src/question_mark.rs8
-rw-r--r--tests/ui/question_mark.fixed12
-rw-r--r--tests/ui/question_mark.rs14
-rw-r--r--tests/ui/question_mark.stderr10
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