Auto merge of #6507 - bengsparks:lint/issue6410, r=flip1995
Needless Question Mark Lint Fixes #6410, i.e the needless question mark lint changelog: [`needless_question_mark`] New lint
This commit is contained in:
@@ -2101,6 +2101,7 @@ Released 2018-09-13
|
|||||||
[`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
|
[`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
|
||||||
[`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
|
[`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
|
||||||
[`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
|
[`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
|
||||||
|
[`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
|
||||||
[`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
|
[`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
|
||||||
[`needless_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
|
[`needless_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
|
||||||
[`needless_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_update
|
[`needless_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_update
|
||||||
|
|||||||
@@ -271,6 +271,7 @@ mod needless_borrow;
|
|||||||
mod needless_borrowed_ref;
|
mod needless_borrowed_ref;
|
||||||
mod needless_continue;
|
mod needless_continue;
|
||||||
mod needless_pass_by_value;
|
mod needless_pass_by_value;
|
||||||
|
mod needless_question_mark;
|
||||||
mod needless_update;
|
mod needless_update;
|
||||||
mod neg_cmp_op_on_partial_ord;
|
mod neg_cmp_op_on_partial_ord;
|
||||||
mod neg_multiply;
|
mod neg_multiply;
|
||||||
@@ -800,6 +801,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
|
&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
|
||||||
&needless_continue::NEEDLESS_CONTINUE,
|
&needless_continue::NEEDLESS_CONTINUE,
|
||||||
&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
|
&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
|
||||||
|
&needless_question_mark::NEEDLESS_QUESTION_MARK,
|
||||||
&needless_update::NEEDLESS_UPDATE,
|
&needless_update::NEEDLESS_UPDATE,
|
||||||
&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD,
|
&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD,
|
||||||
&neg_multiply::NEG_MULTIPLY,
|
&neg_multiply::NEG_MULTIPLY,
|
||||||
@@ -1020,6 +1022,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
store.register_late_pass(move || box from_over_into::FromOverInto::new(msrv));
|
store.register_late_pass(move || box from_over_into::FromOverInto::new(msrv));
|
||||||
store.register_late_pass(move || box use_self::UseSelf::new(msrv));
|
store.register_late_pass(move || box use_self::UseSelf::new(msrv));
|
||||||
store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv));
|
store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv));
|
||||||
|
store.register_late_pass(move || box needless_question_mark::NeedlessQuestionMark::new(msrv));
|
||||||
|
|
||||||
store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount);
|
store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount);
|
||||||
store.register_late_pass(|| box map_clone::MapClone);
|
store.register_late_pass(|| box map_clone::MapClone);
|
||||||
@@ -1547,6 +1550,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
LintId::of(&needless_bool::BOOL_COMPARISON),
|
LintId::of(&needless_bool::BOOL_COMPARISON),
|
||||||
LintId::of(&needless_bool::NEEDLESS_BOOL),
|
LintId::of(&needless_bool::NEEDLESS_BOOL),
|
||||||
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
|
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
|
||||||
|
LintId::of(&needless_question_mark::NEEDLESS_QUESTION_MARK),
|
||||||
LintId::of(&needless_update::NEEDLESS_UPDATE),
|
LintId::of(&needless_update::NEEDLESS_UPDATE),
|
||||||
LintId::of(&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
|
LintId::of(&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
|
||||||
LintId::of(&neg_multiply::NEG_MULTIPLY),
|
LintId::of(&neg_multiply::NEG_MULTIPLY),
|
||||||
@@ -1806,6 +1810,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
LintId::of(&needless_bool::BOOL_COMPARISON),
|
LintId::of(&needless_bool::BOOL_COMPARISON),
|
||||||
LintId::of(&needless_bool::NEEDLESS_BOOL),
|
LintId::of(&needless_bool::NEEDLESS_BOOL),
|
||||||
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
|
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
|
||||||
|
LintId::of(&needless_question_mark::NEEDLESS_QUESTION_MARK),
|
||||||
LintId::of(&needless_update::NEEDLESS_UPDATE),
|
LintId::of(&needless_update::NEEDLESS_UPDATE),
|
||||||
LintId::of(&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
|
LintId::of(&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
|
||||||
LintId::of(&no_effect::NO_EFFECT),
|
LintId::of(&no_effect::NO_EFFECT),
|
||||||
|
|||||||
232
clippy_lints/src/needless_question_mark.rs
Normal file
232
clippy_lints/src/needless_question_mark.rs
Normal file
@@ -0,0 +1,232 @@
|
|||||||
|
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, LintContext};
|
||||||
|
use rustc_middle::ty::DefIdTree;
|
||||||
|
use rustc_semver::RustcVersion;
|
||||||
|
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||||
|
use rustc_span::sym;
|
||||||
|
|
||||||
|
use crate::utils;
|
||||||
|
use if_chain::if_chain;
|
||||||
|
|
||||||
|
declare_clippy_lint! {
|
||||||
|
/// **What it does:**
|
||||||
|
/// Suggests alternatives for useless applications of `?` in terminating expressions
|
||||||
|
///
|
||||||
|
/// **Why is this bad?** There's no reason to use ? to short-circuit when execution of the body will end there anyway.
|
||||||
|
///
|
||||||
|
/// **Known problems:** None.
|
||||||
|
///
|
||||||
|
/// **Example:**
|
||||||
|
///
|
||||||
|
/// ```rust
|
||||||
|
/// struct TO {
|
||||||
|
/// magic: Option<usize>,
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// fn f(to: TO) -> Option<usize> {
|
||||||
|
/// Some(to.magic?)
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// struct TR {
|
||||||
|
/// magic: Result<usize, bool>,
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// fn g(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
/// tr.and_then(|t| Ok(t.magic?))
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// ```
|
||||||
|
/// Use instead:
|
||||||
|
/// ```rust
|
||||||
|
/// struct TO {
|
||||||
|
/// magic: Option<usize>,
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// fn f(to: TO) -> Option<usize> {
|
||||||
|
/// to.magic
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// struct TR {
|
||||||
|
/// magic: Result<usize, bool>,
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// fn g(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
/// tr.and_then(|t| t.magic)
|
||||||
|
/// }
|
||||||
|
/// ```
|
||||||
|
pub NEEDLESS_QUESTION_MARK,
|
||||||
|
complexity,
|
||||||
|
"Suggest value.inner_option instead of Some(value.inner_option?). The same goes for Result<T, E>."
|
||||||
|
}
|
||||||
|
|
||||||
|
const NEEDLESS_QUESTION_MARK_RESULT_MSRV: RustcVersion = RustcVersion::new(1, 13, 0);
|
||||||
|
const NEEDLESS_QUESTION_MARK_OPTION_MSRV: RustcVersion = RustcVersion::new(1, 22, 0);
|
||||||
|
|
||||||
|
pub struct NeedlessQuestionMark {
|
||||||
|
msrv: Option<RustcVersion>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl NeedlessQuestionMark {
|
||||||
|
#[must_use]
|
||||||
|
pub fn new(msrv: Option<RustcVersion>) -> Self {
|
||||||
|
Self { msrv }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl_lint_pass!(NeedlessQuestionMark => [NEEDLESS_QUESTION_MARK]);
|
||||||
|
|
||||||
|
#[derive(Debug)]
|
||||||
|
enum SomeOkCall<'a> {
|
||||||
|
SomeCall(&'a Expr<'a>, &'a Expr<'a>),
|
||||||
|
OkCall(&'a Expr<'a>, &'a Expr<'a>),
|
||||||
|
}
|
||||||
|
|
||||||
|
impl LateLintPass<'_> for NeedlessQuestionMark {
|
||||||
|
/*
|
||||||
|
* The question mark operator is compatible with both Result<T, E> and Option<T>,
|
||||||
|
* from Rust 1.13 and 1.22 respectively.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/*
|
||||||
|
* What do we match:
|
||||||
|
* Expressions that look like this:
|
||||||
|
* Some(option?), Ok(result?)
|
||||||
|
*
|
||||||
|
* Where do we match:
|
||||||
|
* Last expression of a body
|
||||||
|
* Return statement
|
||||||
|
* A body's value (single line closure)
|
||||||
|
*
|
||||||
|
* What do we not match:
|
||||||
|
* Implicit calls to `from(..)` on the error value
|
||||||
|
*/
|
||||||
|
|
||||||
|
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
|
||||||
|
let e = match &expr.kind {
|
||||||
|
ExprKind::Ret(Some(e)) => e,
|
||||||
|
_ => return,
|
||||||
|
};
|
||||||
|
|
||||||
|
if let Some(ok_some_call) = is_some_or_ok_call(self, cx, e) {
|
||||||
|
emit_lint(cx, &ok_some_call);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
|
||||||
|
// Function / Closure block
|
||||||
|
let expr_opt = if let ExprKind::Block(block, _) = &body.value.kind {
|
||||||
|
block.expr
|
||||||
|
} else {
|
||||||
|
// Single line closure
|
||||||
|
Some(&body.value)
|
||||||
|
};
|
||||||
|
|
||||||
|
if_chain! {
|
||||||
|
if let Some(expr) = expr_opt;
|
||||||
|
if let Some(ok_some_call) = is_some_or_ok_call(self, cx, expr);
|
||||||
|
then {
|
||||||
|
emit_lint(cx, &ok_some_call);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
extract_msrv_attr!(LateContext);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn emit_lint(cx: &LateContext<'_>, expr: &SomeOkCall<'_>) {
|
||||||
|
let (entire_expr, inner_expr) = match expr {
|
||||||
|
SomeOkCall::OkCall(outer, inner) | SomeOkCall::SomeCall(outer, inner) => (outer, inner),
|
||||||
|
};
|
||||||
|
|
||||||
|
utils::span_lint_and_sugg(
|
||||||
|
cx,
|
||||||
|
NEEDLESS_QUESTION_MARK,
|
||||||
|
entire_expr.span,
|
||||||
|
"Question mark operator is useless here",
|
||||||
|
"try",
|
||||||
|
format!("{}", utils::snippet(cx, inner_expr.span, r#""...""#)),
|
||||||
|
Applicability::MachineApplicable,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
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;
|
||||||
|
if let ExprKind::Path(QPath::Resolved(None, path)) = &path.kind;
|
||||||
|
if is_some_ctor(cx, path.res) || is_ok_ctor(cx, path.res);
|
||||||
|
|
||||||
|
// Extract inner expression from ARGUMENT
|
||||||
|
if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &args[0].kind;
|
||||||
|
if let ExprKind::Call(called, args) = &inner_expr_with_q.kind;
|
||||||
|
if args.len() == 1;
|
||||||
|
|
||||||
|
if let ExprKind::Path(QPath::LangItem(LangItem::TryIntoResult, _)) = &called.kind;
|
||||||
|
then {
|
||||||
|
// Extract inner expr type from match argument generated by
|
||||||
|
// question mark operator
|
||||||
|
let inner_expr = &args[0];
|
||||||
|
|
||||||
|
let inner_ty = cx.typeck_results().expr_ty(inner_expr);
|
||||||
|
let outer_ty = cx.typeck_results().expr_ty(expr);
|
||||||
|
|
||||||
|
// Check if outer and inner type are Option
|
||||||
|
let outer_is_some = utils::is_type_diagnostic_item(cx, outer_ty, sym::option_type);
|
||||||
|
let inner_is_some = utils::is_type_diagnostic_item(cx, inner_ty, sym::option_type);
|
||||||
|
|
||||||
|
// Check for Option MSRV
|
||||||
|
let meets_option_msrv = utils::meets_msrv(nqml.msrv.as_ref(), &NEEDLESS_QUESTION_MARK_OPTION_MSRV);
|
||||||
|
if outer_is_some && inner_is_some && meets_option_msrv {
|
||||||
|
return Some(SomeOkCall::SomeCall(expr, inner_expr));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if outer and inner type are Result
|
||||||
|
let outer_is_result = utils::is_type_diagnostic_item(cx, outer_ty, sym::result_type);
|
||||||
|
let inner_is_result = utils::is_type_diagnostic_item(cx, inner_ty, sym::result_type);
|
||||||
|
|
||||||
|
// Additional check: if the error type of the Result can be converted
|
||||||
|
// via the From trait, then don't match
|
||||||
|
let does_not_call_from = !has_implicit_error_from(cx, expr, inner_expr);
|
||||||
|
|
||||||
|
// Must meet Result MSRV
|
||||||
|
let meets_result_msrv = utils::meets_msrv(nqml.msrv.as_ref(), &NEEDLESS_QUESTION_MARK_RESULT_MSRV);
|
||||||
|
if outer_is_result && inner_is_result && does_not_call_from && meets_result_msrv {
|
||||||
|
return Some(SomeOkCall::OkCall(expr, inner_expr));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
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 {
|
||||||
|
if let Some(variant_id) = cx.tcx.parent(id) {
|
||||||
|
return variant_id == ok_id;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_some_ctor(cx: &LateContext<'_>, res: Res) -> bool {
|
||||||
|
if let Some(some_id) = cx.tcx.lang_items().option_some_variant() {
|
||||||
|
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
|
||||||
|
if let Some(variant_id) = cx.tcx.parent(id) {
|
||||||
|
return variant_id == some_id;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
163
tests/ui/needless_question_mark.fixed
Normal file
163
tests/ui/needless_question_mark.fixed
Normal file
@@ -0,0 +1,163 @@
|
|||||||
|
// run-rustfix
|
||||||
|
|
||||||
|
#![warn(clippy::needless_question_mark)]
|
||||||
|
#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code, unused_must_use)]
|
||||||
|
#![feature(custom_inner_attributes)]
|
||||||
|
|
||||||
|
struct TO {
|
||||||
|
magic: Option<usize>,
|
||||||
|
}
|
||||||
|
|
||||||
|
struct TR {
|
||||||
|
magic: Result<usize, bool>,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_option_bad1(to: TO) -> Option<usize> {
|
||||||
|
// return as a statement
|
||||||
|
return to.magic;
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting will add a semi-colon, which would make
|
||||||
|
// this identical to the test case above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_option_bad2(to: TO) -> Option<usize> {
|
||||||
|
// return as an expression
|
||||||
|
return to.magic
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_option_bad3(to: TO) -> Option<usize> {
|
||||||
|
// block value "return"
|
||||||
|
to.magic
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_option_bad4(to: Option<TO>) -> Option<usize> {
|
||||||
|
// single line closure
|
||||||
|
to.and_then(|t| t.magic)
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting this will remove the block brackets, making
|
||||||
|
// this test identical to the one above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_option_bad5(to: Option<TO>) -> Option<usize> {
|
||||||
|
// closure with body
|
||||||
|
to.and_then(|t| {
|
||||||
|
t.magic
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_result_bad1(tr: TR) -> Result<usize, bool> {
|
||||||
|
return tr.magic;
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting will add a semi-colon, which would make
|
||||||
|
// this identical to the test case above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_result_bad2(tr: TR) -> Result<usize, bool> {
|
||||||
|
return tr.magic
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_result_bad3(tr: TR) -> Result<usize, bool> {
|
||||||
|
tr.magic
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_result_bad4(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
tr.and_then(|t| t.magic)
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting this will remove the block brackets, making
|
||||||
|
// this test identical to the one above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_result_bad5(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
tr.and_then(|t| {
|
||||||
|
t.magic
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
fn also_bad(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
if tr.is_ok() {
|
||||||
|
let t = tr.unwrap();
|
||||||
|
return t.magic;
|
||||||
|
}
|
||||||
|
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();
|
||||||
|
}
|
||||||
|
}
|
||||||
163
tests/ui/needless_question_mark.rs
Normal file
163
tests/ui/needless_question_mark.rs
Normal file
@@ -0,0 +1,163 @@
|
|||||||
|
// run-rustfix
|
||||||
|
|
||||||
|
#![warn(clippy::needless_question_mark)]
|
||||||
|
#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code, unused_must_use)]
|
||||||
|
#![feature(custom_inner_attributes)]
|
||||||
|
|
||||||
|
struct TO {
|
||||||
|
magic: Option<usize>,
|
||||||
|
}
|
||||||
|
|
||||||
|
struct TR {
|
||||||
|
magic: Result<usize, bool>,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_option_bad1(to: TO) -> Option<usize> {
|
||||||
|
// return as a statement
|
||||||
|
return Some(to.magic?);
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting will add a semi-colon, which would make
|
||||||
|
// this identical to the test case above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_option_bad2(to: TO) -> Option<usize> {
|
||||||
|
// return as an expression
|
||||||
|
return Some(to.magic?)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_option_bad3(to: TO) -> Option<usize> {
|
||||||
|
// block value "return"
|
||||||
|
Some(to.magic?)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_option_bad4(to: Option<TO>) -> Option<usize> {
|
||||||
|
// single line closure
|
||||||
|
to.and_then(|t| Some(t.magic?))
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting this will remove the block brackets, making
|
||||||
|
// this test identical to the one above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_option_bad5(to: Option<TO>) -> Option<usize> {
|
||||||
|
// closure with body
|
||||||
|
to.and_then(|t| {
|
||||||
|
Some(t.magic?)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_result_bad1(tr: TR) -> Result<usize, bool> {
|
||||||
|
return Ok(tr.magic?);
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting will add a semi-colon, which would make
|
||||||
|
// this identical to the test case above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_result_bad2(tr: TR) -> Result<usize, bool> {
|
||||||
|
return Ok(tr.magic?)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_result_bad3(tr: TR) -> Result<usize, bool> {
|
||||||
|
Ok(tr.magic?)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn simple_result_bad4(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
tr.and_then(|t| Ok(t.magic?))
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatting this will remove the block brackets, making
|
||||||
|
// this test identical to the one above
|
||||||
|
#[rustfmt::skip]
|
||||||
|
fn simple_result_bad5(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
tr.and_then(|t| {
|
||||||
|
Ok(t.magic?)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
fn also_bad(tr: Result<TR, bool>) -> Result<usize, bool> {
|
||||||
|
if tr.is_ok() {
|
||||||
|
let t = tr.unwrap();
|
||||||
|
return Ok(t.magic?);
|
||||||
|
}
|
||||||
|
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) };
|
||||||
|
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();
|
||||||
|
}
|
||||||
|
}
|
||||||
88
tests/ui/needless_question_mark.stderr
Normal file
88
tests/ui/needless_question_mark.stderr
Normal file
@@ -0,0 +1,88 @@
|
|||||||
|
error: Question mark operator is useless here
|
||||||
|
--> $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:25:12
|
||||||
|
|
|
||||||
|
LL | return Some(to.magic?)
|
||||||
|
| ^^^^^^^^^^^^^^^ help: try: `to.magic`
|
||||||
|
|
||||||
|
error: Question mark operator is useless here
|
||||||
|
--> $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: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:44:9
|
||||||
|
|
|
||||||
|
LL | Some(t.magic?)
|
||||||
|
| ^^^^^^^^^^^^^^ help: try: `t.magic`
|
||||||
|
|
||||||
|
error: Question mark operator is useless here
|
||||||
|
--> $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:56:12
|
||||||
|
|
|
||||||
|
LL | return Ok(tr.magic?)
|
||||||
|
| ^^^^^^^^^^^^^ help: try: `tr.magic`
|
||||||
|
|
||||||
|
error: Question mark operator is useless here
|
||||||
|
--> $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: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:72:9
|
||||||
|
|
|
||||||
|
LL | Ok(t.magic?)
|
||||||
|
| ^^^^^^^^^^^^ help: try: `t.magic`
|
||||||
|
|
||||||
|
error: Question mark operator is useless here
|
||||||
|
--> $DIR/needless_question_mark.rs:79:16
|
||||||
|
|
|
||||||
|
LL | return Ok(t.magic?);
|
||||||
|
| ^^^^^^^^^^^^ help: try: `t.magic`
|
||||||
|
|
||||||
|
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
|
||||||
|
|
||||||
@@ -2,7 +2,7 @@
|
|||||||
// aux-build:macro_rules.rs
|
// aux-build:macro_rules.rs
|
||||||
|
|
||||||
#![deny(clippy::try_err)]
|
#![deny(clippy::try_err)]
|
||||||
#![allow(clippy::unnecessary_wraps)]
|
#![allow(clippy::unnecessary_wraps, clippy::needless_question_mark)]
|
||||||
|
|
||||||
#[macro_use]
|
#[macro_use]
|
||||||
extern crate macro_rules;
|
extern crate macro_rules;
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
// aux-build:macro_rules.rs
|
// aux-build:macro_rules.rs
|
||||||
|
|
||||||
#![deny(clippy::try_err)]
|
#![deny(clippy::try_err)]
|
||||||
#![allow(clippy::unnecessary_wraps)]
|
#![allow(clippy::unnecessary_wraps, clippy::needless_question_mark)]
|
||||||
|
|
||||||
#[macro_use]
|
#[macro_use]
|
||||||
extern crate macro_rules;
|
extern crate macro_rules;
|
||||||
|
|||||||
@@ -5,7 +5,8 @@
|
|||||||
unused_variables,
|
unused_variables,
|
||||||
clippy::unused_unit,
|
clippy::unused_unit,
|
||||||
clippy::unnecessary_wraps,
|
clippy::unnecessary_wraps,
|
||||||
clippy::or_fun_call
|
clippy::or_fun_call,
|
||||||
|
clippy::needless_question_mark
|
||||||
)]
|
)]
|
||||||
|
|
||||||
use std::fmt::Debug;
|
use std::fmt::Debug;
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
error: passing a unit value to a function
|
error: passing a unit value to a function
|
||||||
--> $DIR/unit_arg.rs:30:5
|
--> $DIR/unit_arg.rs:31:5
|
||||||
|
|
|
|
||||||
LL | / foo({
|
LL | / foo({
|
||||||
LL | | 1;
|
LL | | 1;
|
||||||
@@ -20,7 +20,7 @@ LL | foo(());
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing a unit value to a function
|
error: passing a unit value to a function
|
||||||
--> $DIR/unit_arg.rs:33:5
|
--> $DIR/unit_arg.rs:34:5
|
||||||
|
|
|
|
||||||
LL | foo(foo(1));
|
LL | foo(foo(1));
|
||||||
| ^^^^^^^^^^^
|
| ^^^^^^^^^^^
|
||||||
@@ -32,7 +32,7 @@ LL | foo(());
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing a unit value to a function
|
error: passing a unit value to a function
|
||||||
--> $DIR/unit_arg.rs:34:5
|
--> $DIR/unit_arg.rs:35:5
|
||||||
|
|
|
|
||||||
LL | / foo({
|
LL | / foo({
|
||||||
LL | | foo(1);
|
LL | | foo(1);
|
||||||
@@ -54,7 +54,7 @@ LL | foo(());
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing a unit value to a function
|
error: passing a unit value to a function
|
||||||
--> $DIR/unit_arg.rs:39:5
|
--> $DIR/unit_arg.rs:40:5
|
||||||
|
|
|
|
||||||
LL | / b.bar({
|
LL | / b.bar({
|
||||||
LL | | 1;
|
LL | | 1;
|
||||||
@@ -74,7 +74,7 @@ LL | b.bar(());
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing unit values to a function
|
error: passing unit values to a function
|
||||||
--> $DIR/unit_arg.rs:42:5
|
--> $DIR/unit_arg.rs:43:5
|
||||||
|
|
|
|
||||||
LL | taking_multiple_units(foo(0), foo(1));
|
LL | taking_multiple_units(foo(0), foo(1));
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
@@ -87,7 +87,7 @@ LL | taking_multiple_units((), ());
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing unit values to a function
|
error: passing unit values to a function
|
||||||
--> $DIR/unit_arg.rs:43:5
|
--> $DIR/unit_arg.rs:44:5
|
||||||
|
|
|
|
||||||
LL | / taking_multiple_units(foo(0), {
|
LL | / taking_multiple_units(foo(0), {
|
||||||
LL | | foo(1);
|
LL | | foo(1);
|
||||||
@@ -110,7 +110,7 @@ LL | taking_multiple_units((), ());
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing unit values to a function
|
error: passing unit values to a function
|
||||||
--> $DIR/unit_arg.rs:47:5
|
--> $DIR/unit_arg.rs:48:5
|
||||||
|
|
|
|
||||||
LL | / taking_multiple_units(
|
LL | / taking_multiple_units(
|
||||||
LL | | {
|
LL | | {
|
||||||
@@ -140,7 +140,7 @@ LL | foo(2);
|
|||||||
...
|
...
|
||||||
|
|
||||||
error: passing a unit value to a function
|
error: passing a unit value to a function
|
||||||
--> $DIR/unit_arg.rs:58:13
|
--> $DIR/unit_arg.rs:59:13
|
||||||
|
|
|
|
||||||
LL | None.or(Some(foo(2)));
|
LL | None.or(Some(foo(2)));
|
||||||
| ^^^^^^^^^^^^
|
| ^^^^^^^^^^^^
|
||||||
@@ -154,7 +154,7 @@ LL | });
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing a unit value to a function
|
error: passing a unit value to a function
|
||||||
--> $DIR/unit_arg.rs:61:5
|
--> $DIR/unit_arg.rs:62:5
|
||||||
|
|
|
|
||||||
LL | foo(foo(()))
|
LL | foo(foo(()))
|
||||||
| ^^^^^^^^^^^^
|
| ^^^^^^^^^^^^
|
||||||
@@ -166,7 +166,7 @@ LL | foo(())
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: passing a unit value to a function
|
error: passing a unit value to a function
|
||||||
--> $DIR/unit_arg.rs:94:5
|
--> $DIR/unit_arg.rs:95:5
|
||||||
|
|
|
|
||||||
LL | Some(foo(1))
|
LL | Some(foo(1))
|
||||||
| ^^^^^^^^^^^^
|
| ^^^^^^^^^^^^
|
||||||
|
|||||||
Reference in New Issue
Block a user