Auto merge of #12021 - PartiallyTyped:11982, r=flip1995

FP: `needless_return_with_question_mark` with implicit Error Conversion

Return with a question mark was triggered in situations where the `?` desuraging was performing error conversion via `Into`/`From`.

The desugared `?` produces a match over an expression with type `std::ops::ControlFlow<B,C>` with `B:Result<Infallible, E:Error>` and `C:Result<_, E':Error>`, and the arms perform the conversion. The patch adds another check in the lint that checks that `E == E'`. If `E == E'`, then the `?` is indeed unnecessary.

changelog: False Positive: [`needless_return_with_question_mark`] when implicit Error Conversion occurs.

fixes: #11982

2 files changed, 100 insertions, 0 deletions

diff --git a/tests/ui/needless_return_with_question_mark.fixed b/tests/ui/needless_return_with_question_mark.fixed
index 0147c73a94b..9b7da852663 100644
--- a/tests/ui/needless_return_with_question_mark.fixed
+++ b/tests/ui/needless_return_with_question_mark.fixed
@@ -77,3 +77,53 @@ fn issue11616() -> Result<(), ()> {
     };
     Ok(())
 }
+
+fn issue11982() {
+    mod bar {
+        pub struct Error;
+        pub fn foo(_: bool) -> Result<(), Error> {
+            Ok(())
+        }
+    }
+
+    pub struct Error;
+
+    impl From<bar::Error> for Error {
+        fn from(_: bar::Error) -> Self {
+            Error
+        }
+    }
+
+    fn foo(ok: bool) -> Result<(), Error> {
+        if !ok {
+            return bar::foo(ok).map(|_| Ok::<(), Error>(()))?;
+        };
+        Ok(())
+    }
+}
+
+fn issue11982_no_conversion() {
+    mod bar {
+        pub struct Error;
+        pub fn foo(_: bool) -> Result<(), Error> {
+            Ok(())
+        }
+    }
+
+    fn foo(ok: bool) -> Result<(), bar::Error> {
+        if !ok {
+            return bar::foo(ok).map(|_| Ok::<(), bar::Error>(()))?;
+        };
+        Ok(())
+    }
+}
+
+fn general_return() {
+    fn foo(ok: bool) -> Result<(), ()> {
+        let bar = Result::Ok(Result::<(), ()>::Ok(()));
+        if !ok {
+            return bar?;
+        };
+        Ok(())
+    }
+}
diff --git a/tests/ui/needless_return_with_question_mark.rs b/tests/ui/needless_return_with_question_mark.rs
index 66e1f438f8c..68e76d2b640 100644
--- a/tests/ui/needless_return_with_question_mark.rs
+++ b/tests/ui/needless_return_with_question_mark.rs
@@ -77,3 +77,53 @@ fn issue11616() -> Result<(), ()> {
     };
     Ok(())
 }
+
+fn issue11982() {
+    mod bar {
+        pub struct Error;
+        pub fn foo(_: bool) -> Result<(), Error> {
+            Ok(())
+        }
+    }
+
+    pub struct Error;
+
+    impl From<bar::Error> for Error {
+        fn from(_: bar::Error) -> Self {
+            Error
+        }
+    }
+
+    fn foo(ok: bool) -> Result<(), Error> {
+        if !ok {
+            return bar::foo(ok).map(|_| Ok::<(), Error>(()))?;
+        };
+        Ok(())
+    }
+}
+
+fn issue11982_no_conversion() {
+    mod bar {
+        pub struct Error;
+        pub fn foo(_: bool) -> Result<(), Error> {
+            Ok(())
+        }
+    }
+
+    fn foo(ok: bool) -> Result<(), bar::Error> {
+        if !ok {
+            return bar::foo(ok).map(|_| Ok::<(), bar::Error>(()))?;
+        };
+        Ok(())
+    }
+}
+
+fn general_return() {
+    fn foo(ok: bool) -> Result<(), ()> {
+        let bar = Result::Ok(Result::<(), ()>::Ok(()));
+        if !ok {
+            return bar?;
+        };
+        Ok(())
+    }
+}