Skip to content

Commit

Permalink
needless_question_mark: don't lint if Some(..) is inside a macro def …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
matthiaskrgr committed Mar 18, 2021
1 parent 36aee1c commit b42ec5e
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 2 deletions.
7 changes: 6 additions & 1 deletion clippy_lints/src/needless_question_mark.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
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 rustc_errors::Applicability;
use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath};
Expand Down Expand Up @@ -173,6 +173,11 @@ fn is_some_or_ok_call<'a>(
// question mark operator
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 outer_ty = cx.typeck_results().expr_ty(expr);

Expand Down
25 changes: 25 additions & 0 deletions tests/ui/needless_question_mark.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,28 @@ mod question_mark_both {
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?);
}
25 changes: 25 additions & 0 deletions tests/ui/needless_question_mark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,28 @@ mod question_mark_both {
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?);
}
13 changes: 12 additions & 1 deletion tests/ui/needless_question_mark.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,16 @@ error: question mark operator is useless here
LL | Ok(to.magic?) // should be triggered
| ^^^^^^^^^^^^^ 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

0 comments on commit b42ec5e

Please sign in to comment.