Skip to content

Commit 8753e56

Browse files
committed
Check for comments in collapsible ifs
1 parent 1264bb6 commit 8753e56

File tree

3 files changed

+71
-1
lines changed

3 files changed

+71
-1
lines changed

clippy_lints/src/collapsible_if.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,17 @@ fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
112112
}
113113
}
114114

115+
fn block_starts_with_comment(cx: &EarlyContext<'_>, expr: &ast::Block) -> bool {
116+
// The zeroth character in the trimmed block text is "{", which marks the beginning of the block.
117+
// Therefore, we check if the first string after that is a comment, i.e. starts with //.
118+
let trimmed_block_text = snippet_block(cx, expr.span, "..").trim_left().to_owned();
119+
trimmed_block_text[1..trimmed_block_text.len()].trim_left().starts_with("//")
120+
}
121+
115122
fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
116123
if_chain! {
117124
if let ast::ExprKind::Block(ref block, _) = else_.node;
125+
if !block_starts_with_comment(cx, block);
118126
if let Some(else_) = expr_block(block);
119127
if !in_macro(else_.span);
120128
then {
@@ -135,6 +143,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
135143

136144
fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &ast::Expr, then: &ast::Block) {
137145
if_chain! {
146+
if !block_starts_with_comment(cx, then);
138147
if let Some(inner) = expr_block(then);
139148
if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node;
140149
then {

tests/ui/collapsible_if.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,4 +151,49 @@ fn main() {
151151
} else {
152152
assert!(true); // assert! is just an `if`
153153
}
154+
155+
156+
// The following tests check for the fix of https://github.com/rust-lang-nursery/rust-clippy/issues/798
157+
if x == "hello" {// Not collapsible
158+
if y == "world" {
159+
println!("Hello world!");
160+
}
161+
}
162+
163+
if x == "hello" { // Not collapsible
164+
if y == "world" {
165+
println!("Hello world!");
166+
}
167+
}
168+
169+
if x == "hello" {
170+
// Not collapsible
171+
if y == "world" {
172+
println!("Hello world!");
173+
}
174+
}
175+
176+
if x == "hello" {
177+
if y == "world" { // Collapsible
178+
println!("Hello world!");
179+
}
180+
}
181+
182+
if x == "hello" {
183+
print!("Hello ");
184+
} else {
185+
// Not collapsible
186+
if y == "world" {
187+
println!("world!")
188+
}
189+
}
190+
191+
if x == "hello" {
192+
print!("Hello ");
193+
} else {
194+
// Not collapsible
195+
if let Some(42) = Some(42) {
196+
println!("world!")
197+
}
198+
}
154199
}

tests/ui/collapsible_if.stderr

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,5 +240,21 @@ help: try
240240
122 | }
241241
|
242242

243-
error: aborting due to 13 previous errors
243+
error: this if statement can be collapsed
244+
--> $DIR/collapsible_if.rs:176:5
245+
|
246+
176 | / if x == "hello" {
247+
177 | | if y == "world" { // Collapsible
248+
178 | | println!("Hello world!");
249+
179 | | }
250+
180 | | }
251+
| |_____^
252+
help: try
253+
|
254+
176 | if x == "hello" && y == "world" { // Collapsible
255+
177 | println!("Hello world!");
256+
178 | }
257+
|
258+
259+
error: aborting due to 14 previous errors
244260

0 commit comments

Comments
 (0)