From af19ff8cc89bb6a010faaf110eb63e1fbce49b1a Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Thu, 17 Jul 2025 19:16:39 +0200 Subject: [PATCH] fix `collapsable_if` when the inner `if` is in parens --- clippy_lints/src/collapsible_if.rs | 51 +++++++++++++++++++++++++++++- tests/ui/collapsible_if.fixed | 8 +++++ tests/ui/collapsible_if.rs | 9 ++++++ tests/ui/collapsible_if.stderr | 20 +++++++++++- 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 1854d86c53b2..da0f0b2f1be9 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -187,7 +187,8 @@ impl CollapsibleIf { .with_leading_whitespace(cx) .into_span() }; - let inner_if = inner.span.split_at(2).0; + let (paren_start, inner_if_span, paren_end) = peel_parens(cx.tcx.sess.source_map(), inner.span); + let inner_if = inner_if_span.split_at(2).0; let mut sugg = vec![ // Remove the outer then block `{` (then_open_bracket, String::new()), @@ -196,6 +197,17 @@ impl CollapsibleIf { // Replace inner `if` by `&&` (inner_if, String::from("&&")), ]; + + if !paren_start.is_empty() { + // Remove any leading parentheses '(' + sugg.push((paren_start, String::new())); + } + + if !paren_end.is_empty() { + // Remove any trailing parentheses ')' + sugg.push((paren_end, String::new())); + } + sugg.extend(parens_around(check)); sugg.extend(parens_around(check_inner)); @@ -285,3 +297,40 @@ fn span_extract_keyword(sm: &SourceMap, span: Span, keyword: &str) -> Option (Span, Span, Span) { + use crate::rustc_span::Pos; + use rustc_span::SpanData; + + let start = span.shrink_to_lo(); + let end = span.shrink_to_hi(); + + loop { + let data = span.data(); + let snippet = sm.span_to_snippet(span).unwrap(); + + let trim_start = snippet.len() - snippet.trim_start().len(); + let trim_end = snippet.len() - snippet.trim_end().len(); + + let trimmed = snippet.trim(); + + if trimmed.starts_with('(') && trimmed.ends_with(')') { + // Try to remove one layer of parens by adjusting the span + span = SpanData { + lo: data.lo + BytePos::from_usize(trim_start + 1), + hi: data.hi - BytePos::from_usize(trim_end + 1), + ctxt: data.ctxt, + parent: data.parent, + } + .span(); + + continue; + } + + break; + } + + (start.with_hi(span.lo()), + span, + end.with_lo(span.hi())) +} diff --git a/tests/ui/collapsible_if.fixed b/tests/ui/collapsible_if.fixed index 77bc791ea8e9..f7c840c4cc1e 100644 --- a/tests/ui/collapsible_if.fixed +++ b/tests/ui/collapsible_if.fixed @@ -163,3 +163,11 @@ fn issue14799() { if true {} }; } + +fn in_parens() { + if true + && true { + println!("In parens, linted"); + } + //~^^^^^ collapsible_if +} diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs index d30df157d5eb..4faebcb0ca0d 100644 --- a/tests/ui/collapsible_if.rs +++ b/tests/ui/collapsible_if.rs @@ -173,3 +173,12 @@ fn issue14799() { if true {} }; } + +fn in_parens() { + if true { + (if true { + println!("In parens, linted"); + }) + } + //~^^^^^ collapsible_if +} diff --git a/tests/ui/collapsible_if.stderr b/tests/ui/collapsible_if.stderr index 32c6b0194030..a685cc2e9291 100644 --- a/tests/ui/collapsible_if.stderr +++ b/tests/ui/collapsible_if.stderr @@ -190,5 +190,23 @@ LL | // This is a comment, do not collapse code to it LL ~ ; 3 | -error: aborting due to 11 previous errors +error: this `if` statement can be collapsed + --> tests/ui/collapsible_if.rs:178:5 + | +LL | / if true { +LL | | (if true { +LL | | println!("In parens, linted"); +LL | | }) +LL | | } + | |_____^ + | +help: collapse nested if block + | +LL ~ if true +LL ~ && true { +LL | println!("In parens, linted"); +LL ~ } + | + +error: aborting due to 12 previous errors