Skip to content

Commit 25bf103

Browse files
committed
fix collapsable_if when the inner if is in parens
1 parent e62e27b commit 25bf103

File tree

4 files changed

+86
-2
lines changed

4 files changed

+86
-2
lines changed

clippy_lints/src/collapsible_if.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ impl CollapsibleIf {
187187
.with_leading_whitespace(cx)
188188
.into_span()
189189
};
190-
let inner_if = inner.span.split_at(2).0;
190+
let (paren_start, inner_if_span, paren_end) = peel_parens(cx.tcx.sess.source_map(), inner.span);
191+
let inner_if = inner_if_span.split_at(2).0;
191192
let mut sugg = vec![
192193
// Remove the outer then block `{`
193194
(then_open_bracket, String::new()),
@@ -196,6 +197,17 @@ impl CollapsibleIf {
196197
// Replace inner `if` by `&&`
197198
(inner_if, String::from("&&")),
198199
];
200+
201+
if !paren_start.is_empty() {
202+
// Remove any leading parentheses '('
203+
sugg.push((paren_start, String::new()))
204+
}
205+
206+
if !paren_end.is_empty() {
207+
// Remove any trailing parentheses ')'
208+
sugg.push((paren_end, String::new()))
209+
}
210+
199211
sugg.extend(parens_around(check));
200212
sugg.extend(parens_around(check_inner));
201213

@@ -285,3 +297,40 @@ fn span_extract_keyword(sm: &SourceMap, span: Span, keyword: &str) -> Option<Spa
285297
})
286298
.next()
287299
}
300+
301+
fn peel_parens(sm: &SourceMap, mut span: Span) -> (Span, Span, Span) {
302+
use crate::rustc_span::Pos;
303+
use rustc_span::SpanData;
304+
305+
let start = span.shrink_to_lo();
306+
let end = span.shrink_to_hi();
307+
308+
loop {
309+
let data = span.data();
310+
let snippet = sm.span_to_snippet(span).unwrap();
311+
312+
let trim_start = snippet.len() - snippet.trim_start().len();
313+
let trim_end = snippet.len() - snippet.trim_end().len();
314+
315+
let trimmed = snippet.trim();
316+
317+
if trimmed.starts_with('(') && trimmed.ends_with(')') {
318+
// Try to remove one layer of parens by adjusting the span
319+
span = SpanData {
320+
lo: data.lo + BytePos::from_usize(trim_start + 1),
321+
hi: data.hi - BytePos::from_usize(trim_end + 1),
322+
ctxt: data.ctxt,
323+
parent: data.parent,
324+
}
325+
.span();
326+
327+
continue;
328+
}
329+
330+
break;
331+
}
332+
333+
(start.with_hi(span.lo()),
334+
span,
335+
end.with_lo(span.hi()))
336+
}

tests/ui/collapsible_if.fixed

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,3 +163,11 @@ fn issue14799() {
163163
if true {}
164164
};
165165
}
166+
167+
fn in_parens() {
168+
if true
169+
&& true {
170+
println!("In parens, linted");
171+
}
172+
//~^^^^^ collapsible_if
173+
}

tests/ui/collapsible_if.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,12 @@ fn issue14799() {
173173
if true {}
174174
};
175175
}
176+
177+
fn in_parens() {
178+
if true {
179+
(if true {
180+
println!("In parens, linted");
181+
})
182+
}
183+
//~^^^^^ collapsible_if
184+
}

tests/ui/collapsible_if.stderr

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,5 +190,23 @@ LL | // This is a comment, do not collapse code to it
190190
LL ~ ; 3
191191
|
192192

193-
error: aborting due to 11 previous errors
193+
error: this `if` statement can be collapsed
194+
--> tests/ui/collapsible_if.rs:178:5
195+
|
196+
LL | / if true {
197+
LL | | (if true {
198+
LL | | println!("In parens, linted");
199+
LL | | })
200+
LL | | }
201+
| |_____^
202+
|
203+
help: collapse nested if block
204+
|
205+
LL ~ if true
206+
LL ~ && true {
207+
LL | println!("In parens, linted");
208+
LL ~ }
209+
|
210+
211+
error: aborting due to 12 previous errors
194212

0 commit comments

Comments
 (0)