Auto merge of #6935 - matthiaskrgr:qmark_marco, r=flip1995

needless_question_mark: don't lint if Some(..) is inside a macro def and the ? is not.

The suggestion would fail to apply.

Fixes #6921

changelog: needless_question_mark: don't lint if Some(..) is inside a macro def and the ? is not.
This commit is contained in:
bors 2021-03-19 09:55:30 +00:00
commit 0d2e2b5f15
4 changed files with 68 additions and 2 deletions

View file

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_ok_ctor, is_some_ctor, meets_msrv}; use clippy_utils::{differing_macro_contexts, is_ok_ctor, is_some_ctor, meets_msrv};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath}; use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath};
@ -173,6 +173,11 @@ fn is_some_or_ok_call<'a>(
// question mark operator // question mark operator
let inner_expr = &args[0]; let inner_expr = &args[0];
// if the inner expr is inside macro but the outer one is not, do not lint (#6921)
if differing_macro_contexts(expr.span, inner_expr.span) {
return None;
}
let inner_ty = cx.typeck_results().expr_ty(inner_expr); let inner_ty = cx.typeck_results().expr_ty(inner_expr);
let outer_ty = cx.typeck_results().expr_ty(expr); let outer_ty = cx.typeck_results().expr_ty(expr);

View file

@ -167,3 +167,28 @@ mod question_mark_both {
needless_question_mark_result(); needless_question_mark_result();
} }
} }
// #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use,
// the suggestion fails to apply; do not lint
macro_rules! some_in_macro {
($expr:expr) => {
|| -> _ { Some($expr) }()
};
}
pub fn test1() {
let x = Some(3);
let _x = some_in_macro!(x?);
}
// this one is ok because both the ? and the Some are both inside the macro def
macro_rules! some_and_qmark_in_macro {
($expr:expr) => {
|| -> Option<_> { Some($expr) }()
};
}
pub fn test2() {
let x = Some(3);
let _x = some_and_qmark_in_macro!(x?);
}

View file

@ -167,3 +167,28 @@ mod question_mark_both {
needless_question_mark_result(); needless_question_mark_result();
} }
} }
// #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use,
// the suggestion fails to apply; do not lint
macro_rules! some_in_macro {
($expr:expr) => {
|| -> _ { Some($expr) }()
};
}
pub fn test1() {
let x = Some(3);
let _x = some_in_macro!(x?);
}
// this one is ok because both the ? and the Some are both inside the macro def
macro_rules! some_and_qmark_in_macro {
($expr:expr) => {
|| -> Option<_> { Some(Some($expr)?) }()
};
}
pub fn test2() {
let x = Some(3);
let _x = some_and_qmark_in_macro!(x?);
}

View file

@ -84,5 +84,16 @@ error: question mark operator is useless here
LL | Ok(to.magic?) // should be triggered LL | Ok(to.magic?) // should be triggered
| ^^^^^^^^^^^^^ help: try: `to.magic` | ^^^^^^^^^^^^^ help: try: `to.magic`
error: aborting due to 14 previous errors error: question mark operator is useless here
--> $DIR/needless_question_mark.rs:187:27
|
LL | || -> Option<_> { Some(Some($expr)?) }()
| ^^^^^^^^^^^^^^^^^^ help: try: `Some($expr)`
...
LL | let _x = some_and_qmark_in_macro!(x?);
| ---------------------------- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to 15 previous errors