Skip to content

Commit

Permalink
lint in nested bodies if try is in outer body
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Jun 28, 2023
1 parent 716305d commit 70610c0
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 5 deletions.
22 changes: 18 additions & 4 deletions clippy_lints/src/question_mark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub struct QuestionMark {
/// very quickly, without having to walk up the parent chain, by simply checking
/// if it is greater than zero.
/// As for why we need this in the first place: <https://github.com/rust-lang/rust-clippy/issues/8628>
try_block_depth: u32,
try_block_depth_stack: Vec<u32>,
}
impl_lint_pass!(QuestionMark => [QUESTION_MARK]);

Expand Down Expand Up @@ -150,7 +150,7 @@ fn expr_return_none_or_err(

impl QuestionMark {
fn inside_try_block(&self) -> bool {
self.try_block_depth > 0
self.try_block_depth_stack.last() > Some(&0)
}

/// Checks if the given expression on the given context matches the following structure:
Expand Down Expand Up @@ -268,13 +268,27 @@ impl<'tcx> LateLintPass<'tcx> for QuestionMark {

fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx rustc_hir::Block<'tcx>) {
if is_try_block(cx, block) {
self.try_block_depth += 1;
*self
.try_block_depth_stack
.last_mut()
.expect("blocks are always part of bodies and must have a depth") += 1;
}
}

fn check_body(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::Body<'tcx>) {
self.try_block_depth_stack.push(0);
}

fn check_body_post(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::Body<'tcx>) {
self.try_block_depth_stack.pop();
}

fn check_block_post(&mut self, cx: &LateContext<'tcx>, block: &'tcx rustc_hir::Block<'tcx>) {
if is_try_block(cx, block) {
self.try_block_depth -= 1;
*self
.try_block_depth_stack
.last_mut()
.expect("blocks are always part of bodies and must have a depth") -= 1;
}
}
}
10 changes: 10 additions & 0 deletions tests/ui/question_mark.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,16 @@ fn issue8628(a: Option<u32>) -> Option<u32> {
b.or(Some(128))
}

fn issue6828_nested_body() -> Option<u32> {
try {
fn f2(a: Option<i32>) -> Option<i32> {
a?;
Some(32)
}
123
}
}

// should not lint, `?` operator not available in const context
const fn issue9175(option: Option<()>) -> Option<()> {
if option.is_none() {
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/question_mark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,20 @@ fn issue8628(a: Option<u32>) -> Option<u32> {
b.or(Some(128))
}

fn issue6828_nested_body() -> Option<u32> {
try {
fn f2(a: Option<i32>) -> Option<i32> {
if a.is_none() {
return None;
// do lint here, the outer `try` is not relevant here
// https://github.com/rust-lang/rust-clippy/pull/11001#issuecomment-1610636867
}
Some(32)
}
123
}
}

// should not lint, `?` operator not available in const context
const fn issue9175(option: Option<()>) -> Option<()> {
if option.is_none() {
Expand Down
12 changes: 11 additions & 1 deletion tests/ui/question_mark.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,15 @@ LL | | return Err(err);
LL | | }
| |_____^ help: replace it with: `func_returning_result()?;`

error: aborting due to 15 previous errors
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:281:13
|
LL | / if a.is_none() {
LL | | return None;
LL | | // do lint here, the outer `try` is not relevant here
LL | | // https://github.com/rust-lang/rust-clippy/pull/11001#issuecomment-1610636867
LL | | }
| |_____________^ help: replace it with: `a?;`

error: aborting due to 16 previous errors

0 comments on commit 70610c0

Please sign in to comment.