Detect conversion of error type

rust-lang · Dec 30, 2020 · 33da535 · 33da535
1 parent 5e0eece
commit 33da535
Show file tree

Hide file tree

Showing 4 changed files with 203 additions and 19 deletions.
diff --git a/clippy_lints/src/needless_question_mark.rs b/clippy_lints/src/needless_question_mark.rs
@@ -1,10 +1,10 @@
 use rustc_errors::Applicability;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::ty::DefIdTree;
 use rustc_semver::RustcVersion;
-use rustc_session::{impl_lint_pass, declare_tool_lint};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::sym;
 
 use crate::utils;
@@ -139,7 +139,11 @@ fn emit_lint(cx: &LateContext<'_>, expr: &SomeOkCall<'_>) {
     );
 }
 
-fn is_some_or_ok_call<'a>(nqml: &NeedlessQuestionMark, cx: &'a LateContext<'_>, expr: &'a Expr<'_>) -> Option<SomeOkCall<'a>> {
+fn is_some_or_ok_call<'a>(
+    nqml: &NeedlessQuestionMark,
+    cx: &'a LateContext<'_>,
+    expr: &'a Expr<'_>,
+) -> Option<SomeOkCall<'a>> {
     if_chain! {
         // Check outer expression matches CALL_IDENT(ARGUMENT) format
         if let ExprKind::Call(path, args) = &expr.kind;
@@ -189,6 +193,10 @@ fn is_some_or_ok_call<'a>(nqml: &NeedlessQuestionMark, cx: &'a LateContext<'_>,
     None
 }
 
+fn has_implicit_error_from(cx: &LateContext<'_>, entire_expr: &Expr<'_>, inner_result_expr: &Expr<'_>) -> bool {
+    return cx.typeck_results().expr_ty(entire_expr) != cx.typeck_results().expr_ty(inner_result_expr);
+}
+
 fn is_ok_ctor(cx: &LateContext<'_>, res: Res) -> bool {
     if let Some(ok_id) = cx.tcx.lang_items().result_ok_variant() {
         if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {

diff --git a/tests/ui/needless_question_mark.fixed b/tests/ui/needless_question_mark.fixed
@@ -1,7 +1,9 @@
 // run-rustfix
 
 #![warn(clippy::needless_question_mark)]
-#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code)]
+#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code, unused_must_use)]
+#![feature(custom_inner_attributes)]
+
 struct TO {
     magic: Option<usize>,
 }
@@ -79,4 +81,83 @@ fn also_bad(tr: Result<TR, bool>) -> Result<usize, bool> {
     Err(false)
 }
 
+fn false_positive_test<U, T>(x: Result<(), U>) -> Result<(), T>
+where
+    T: From<U>,
+{
+    Ok(x?)
+}
+
 fn main() {}
+
+mod question_mark_none {
+    #![clippy::msrv = "1.12.0"]
+    fn needless_question_mark_option() -> Option<usize> {
+        struct TO {
+            magic: Option<usize>,
+        };
+        let to = TO { magic: None };
+        Some(to.magic?) // should not be triggered
+    }
+
+    fn needless_question_mark_result() -> Result<usize, bool> {
+        struct TO {
+            magic: Result<usize, bool>,
+        };
+        let to = TO { magic: Ok(1_usize) };
+        Ok(to.magic?) // should not be triggered
+    }
+
+    fn main() {
+        needless_question_mark_option();
+        needless_question_mark_result();
+    }
+}
+
+mod question_mark_result {
+    #![clippy::msrv = "1.21.0"]
+    fn needless_question_mark_option() -> Option<usize> {
+        struct TO {
+            magic: Option<usize>,
+        };
+        let to = TO { magic: None };
+        Some(to.magic?) // should not be triggered
+    }
+
+    fn needless_question_mark_result() -> Result<usize, bool> {
+        struct TO {
+            magic: Result<usize, bool>,
+        };
+        let to = TO { magic: Ok(1_usize) };
+        to.magic // should be triggered
+    }
+
+    fn main() {
+        needless_question_mark_option();
+        needless_question_mark_result();
+    }
+}
+
+mod question_mark_both {
+    #![clippy::msrv = "1.22.0"]
+    fn needless_question_mark_option() -> Option<usize> {
+        struct TO {
+            magic: Option<usize>,
+        };
+        let to = TO { magic: None };
+        to.magic // should be triggered
+    }
+
+    fn needless_question_mark_result() -> Result<usize, bool> {
+        struct TO {
+            magic: Result<usize, bool>,
+        };
+        let to = TO { magic: Ok(1_usize) };
+        to.magic // should be triggered
+    }
+
+    fn main() {
+        needless_question_mark_option();
+        needless_question_mark_result();
+    }
+}
diff --git a/tests/ui/needless_question_mark.rs b/tests/ui/needless_question_mark.rs
@@ -1,7 +1,9 @@
 // run-rustfix
 
 #![warn(clippy::needless_question_mark)]
-#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code)]
+#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code, unused_must_use)]
+#![feature(custom_inner_attributes)]
+
 struct TO {
     magic: Option<usize>,
 }
@@ -79,8 +81,83 @@ fn also_bad(tr: Result<TR, bool>) -> Result<usize, bool> {
     Err(false)
 }
 
-fn false_positive_test<U, T>(x: Result<(), U>) -> Result<(), T> where T: From<U> {
+fn false_positive_test<U, T>(x: Result<(), U>) -> Result<(), T>
+where
+    T: From<U>,
+{
     Ok(x?)
-} 
+}
 
 fn main() {}
+
+mod question_mark_none {
+    #![clippy::msrv = "1.12.0"]
+    fn needless_question_mark_option() -> Option<usize> {
+        struct TO {
+            magic: Option<usize>,
+        };
+        let to = TO { magic: None };
+        Some(to.magic?) // should not be triggered
+    }
+
+    fn needless_question_mark_result() -> Result<usize, bool> {
+        struct TO {
+            magic: Result<usize, bool>,
+        };
+        let to = TO { magic: Ok(1_usize) };
+        Ok(to.magic?) // should not be triggered
+    }
+
+    fn main() {
+        needless_question_mark_option();
+        needless_question_mark_result();
+    }
+}
+
+mod question_mark_result {
+    #![clippy::msrv = "1.21.0"]
+    fn needless_question_mark_option() -> Option<usize> {
+        struct TO {
+            magic: Option<usize>,
+        };
+        let to = TO { magic: None };
+        Some(to.magic?) // should not be triggered
+    }
+
+    fn needless_question_mark_result() -> Result<usize, bool> {
+        struct TO {
+            magic: Result<usize, bool>,
+        };
+        let to = TO { magic: Ok(1_usize) };
+        Ok(to.magic?) // should be triggered
+    }
+
+    fn main() {
+        needless_question_mark_option();
+        needless_question_mark_result();
+    }
+}
+
+mod question_mark_both {
+    #![clippy::msrv = "1.22.0"]
+    fn needless_question_mark_option() -> Option<usize> {
+        struct TO {
+            magic: Option<usize>,
+        };
+        let to = TO { magic: None };
+        Some(to.magic?) // should be triggered
+    }
+
+    fn needless_question_mark_result() -> Result<usize, bool> {
+        struct TO {
+            magic: Result<usize, bool>,
+        };
+        let to = TO { magic: Ok(1_usize) };
+        Ok(to.magic?) // should be triggered
+    }
+
+    fn main() {
+        needless_question_mark_option();
+        needless_question_mark_result();
+    }
+}
diff --git a/tests/ui/needless_question_mark.stderr b/tests/ui/needless_question_mark.stderr
@@ -1,70 +1,88 @@
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:15:12
+  --> $DIR/needless_question_mark.rs:17:12
    |
 LL |     return Some(to.magic?);
    |            ^^^^^^^^^^^^^^^ help: try: `to.magic`
    |
    = note: `-D clippy::needless-question-mark` implied by `-D warnings`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:23:12
+  --> $DIR/needless_question_mark.rs:25:12
    |
 LL |     return Some(to.magic?)
    |            ^^^^^^^^^^^^^^^ help: try: `to.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:28:5
+  --> $DIR/needless_question_mark.rs:30:5
    |
 LL |     Some(to.magic?)
    |     ^^^^^^^^^^^^^^^ help: try: `to.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:33:21
+  --> $DIR/needless_question_mark.rs:35:21
    |
 LL |     to.and_then(|t| Some(t.magic?))
    |                     ^^^^^^^^^^^^^^ help: try: `t.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:42:9
+  --> $DIR/needless_question_mark.rs:44:9
    |
 LL |         Some(t.magic?)
    |         ^^^^^^^^^^^^^^ help: try: `t.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:47:12
+  --> $DIR/needless_question_mark.rs:49:12
    |
 LL |     return Ok(tr.magic?);
    |            ^^^^^^^^^^^^^ help: try: `tr.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:54:12
+  --> $DIR/needless_question_mark.rs:56:12
    |
 LL |     return Ok(tr.magic?)
    |            ^^^^^^^^^^^^^ help: try: `tr.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:58:5
+  --> $DIR/needless_question_mark.rs:60:5
    |
 LL |     Ok(tr.magic?)
    |     ^^^^^^^^^^^^^ help: try: `tr.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:62:21
+  --> $DIR/needless_question_mark.rs:64:21
    |
 LL |     tr.and_then(|t| Ok(t.magic?))
    |                     ^^^^^^^^^^^^ help: try: `t.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:70:9
+  --> $DIR/needless_question_mark.rs:72:9
    |
 LL |         Ok(t.magic?)
    |         ^^^^^^^^^^^^ help: try: `t.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:77:16
+  --> $DIR/needless_question_mark.rs:79:16
    |
 LL |         return Ok(t.magic?);
    |                ^^^^^^^^^^^^ help: try: `t.magic`
 
-error: aborting due to 11 previous errors
+error: Question mark operator is useless here
+  --> $DIR/needless_question_mark.rs:132:9
+   |
+LL |         Ok(to.magic?) // should be triggered
+   |         ^^^^^^^^^^^^^ help: try: `to.magic`
+
+error: Question mark operator is useless here
+  --> $DIR/needless_question_mark.rs:148:9
+   |
+LL |         Some(to.magic?) // should be triggered
+   |         ^^^^^^^^^^^^^^^ help: try: `to.magic`
+
+error: Question mark operator is useless here
+  --> $DIR/needless_question_mark.rs:156:9
+   |
+LL |         Ok(to.magic?) // should be triggered
+   |         ^^^^^^^^^^^^^ help: try: `to.magic`
+
+error: aborting due to 14 previous errors