diff --git a/clippy_lints/src/needless_question_mark.rs b/clippy_lints/src/needless_question_mark.rs index 694da60389ab..e79bd655158b 100644 --- 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> { +fn is_some_or_ok_call<'a>( + nqml: &NeedlessQuestionMark, + cx: &'a LateContext<'_>, + expr: &'a Expr<'_>, +) -> Option> { 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 index 3924ec2ffe66..d8efc9cb7cf3 100644 --- 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, } @@ -79,4 +81,83 @@ fn also_bad(tr: Result) -> Result { Err(false) } +fn false_positive_test(x: Result<(), U>) -> Result<(), T> +where + T: From, +{ + Ok(x?) +} + fn main() {} + +mod question_mark_none { + #![clippy::msrv = "1.12.0"] + fn needless_question_mark_option() -> Option { + struct TO { + magic: Option, + }; + let to = TO { magic: None }; + Some(to.magic?) // should not be triggered + } + + fn needless_question_mark_result() -> Result { + struct TO { + magic: Result, + }; + 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 { + struct TO { + magic: Option, + }; + let to = TO { magic: None }; + Some(to.magic?) // should not be triggered + } + + fn needless_question_mark_result() -> Result { + struct TO { + magic: Result, + }; + 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 { + struct TO { + magic: Option, + }; + let to = TO { magic: None }; + to.magic // should be triggered + } + + fn needless_question_mark_result() -> Result { + struct TO { + magic: Result, + }; + 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 index 93ccec83954a..a8f8efcce1c8 100644 --- 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, } @@ -79,8 +81,83 @@ fn also_bad(tr: Result) -> Result { Err(false) } -fn false_positive_test(x: Result<(), U>) -> Result<(), T> where T: From { +fn false_positive_test(x: Result<(), U>) -> Result<(), T> +where + T: From, +{ Ok(x?) -} +} fn main() {} + +mod question_mark_none { + #![clippy::msrv = "1.12.0"] + fn needless_question_mark_option() -> Option { + struct TO { + magic: Option, + }; + let to = TO { magic: None }; + Some(to.magic?) // should not be triggered + } + + fn needless_question_mark_result() -> Result { + struct TO { + magic: Result, + }; + 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 { + struct TO { + magic: Option, + }; + let to = TO { magic: None }; + Some(to.magic?) // should not be triggered + } + + fn needless_question_mark_result() -> Result { + struct TO { + magic: Result, + }; + 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 { + struct TO { + magic: Option, + }; + let to = TO { magic: None }; + Some(to.magic?) // should be triggered + } + + fn needless_question_mark_result() -> Result { + struct TO { + magic: Result, + }; + 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 index f3be1fd4a3d1..b4eb21882ece 100644 --- a/tests/ui/needless_question_mark.stderr +++ b/tests/ui/needless_question_mark.stderr @@ -1,5 +1,5 @@ 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` @@ -7,64 +7,82 @@ LL | return Some(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