Skip to content

Commit f72ea1f

Browse files
authored
Document and test use of char::is_whitespace in collapsible_if (#16840)
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16840)* changelog: none This PR addresses the whitespace check in collapsible_if. ```rust // ./clippy_lints/src/collapsible_if.rs:145 let requires_space = snippet(cx, up_to_else, "..").ends_with(|c: char| !c.is_whitespace()); ``` My investigation revealed that the current char::is_whitespace check is actually intentional. If we switched to rustc_lexer::is_whitespace, zero-width characters (like \u{200E}) would be treated as valid spacing, which would make Clippy to output suggestions that visibly look like elseif. To prevent future regressions, this PR adds a UI test with a zero-width space and an inline comment explaining why we must keep the current check. The issue is for outreachy applicants and is being tracked here: rustfoundation/interop-initiative#53
2 parents b59f804 + 777bdce commit f72ea1f

4 files changed

Lines changed: 101 additions & 1 deletion

File tree

clippy_lints/src/collapsible_if.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ impl CollapsibleIf {
140140

141141
// Prevent "elseif"
142142
// Check that the "else" is followed by whitespace
143+
// Note: We intentionally use char::is_whitespace instead of rustc_lexer::is_whitespace here to
144+
// avoid visual issues with zero-width spaces. See ui tests.
143145
let requires_space = snippet(cx, up_to_else, "..").ends_with(|c: char| !c.is_whitespace());
144146
let mut applicability = Applicability::MachineApplicable;
145147
diag.span_suggestion(

tests/ui/collapsible_else_if.fixed

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,32 @@ fn in_brackets() {
161161
{ if y == "world" { println!("world") } else { println!("!") } }
162162
}
163163
}
164+
165+
#[rustfmt::skip]
166+
fn ends_with_zero_width_whitespace() {
167+
// Test out snippets ending with the 2 zero-width characters recognized as whitespaces by the lexer,
168+
// but not by char::is_whitespace
169+
// Behaviour shows a whitespace is inserted between else and if here which is desirable in this case
170+
171+
let x = "hello";
172+
let y = "world";
173+
174+
175+
// LRM (U+200E)
176+
if x == "hello" {
177+
println!("hello LRM");
178+
} else‎ if y == "world" {
179+
println!("LRM world");
180+
}
181+
//~^^^^^ collapsible_else_if
182+
183+
// RLM (U+200F)
184+
if x == "hello" {
185+
println!("hello RLM");
186+
} else‏ if y == "world" {
187+
println!("RLM world");
188+
}
189+
//~^^^^^ collapsible_else_if
190+
191+
192+
}

tests/ui/collapsible_else_if.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,36 @@ fn in_brackets() {
181181
{ if y == "world" { println!("world") } else { println!("!") } }
182182
}
183183
}
184+
185+
#[rustfmt::skip]
186+
fn ends_with_zero_width_whitespace() {
187+
// Test out snippets ending with the 2 zero-width characters recognized as whitespaces by the lexer,
188+
// but not by char::is_whitespace
189+
// Behaviour shows a whitespace is inserted between else and if here which is desirable in this case
190+
191+
let x = "hello";
192+
let y = "world";
193+
194+
195+
// LRM (U+200E)
196+
if x == "hello" {
197+
println!("hello LRM");
198+
} else{
199+
if y == "world" {
200+
println!("LRM world");
201+
}
202+
}
203+
//~^^^^^ collapsible_else_if
204+
205+
// RLM (U+200F)
206+
if x == "hello" {
207+
println!("hello RLM");
208+
} else{
209+
if y == "world" {
210+
println!("RLM world");
211+
}
212+
}
213+
//~^^^^^ collapsible_else_if
214+
215+
216+
}

tests/ui/collapsible_else_if.stderr

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,5 +177,41 @@ LL | | (if y == "world" { println!("world") } else { println!("!") })
177177
LL | | }
178178
| |_____^ help: collapse nested if block: `if y == "world" { println!("world") } else { println!("!") }`
179179

180-
error: aborting due to 10 previous errors
180+
error: this `else { if .. }` block can be collapsed
181+
--> tests/ui/collapsible_else_if.rs:198:12
182+
|
183+
LL | } else‎{
184+
| ___________^
185+
LL | | if y == "world" {
186+
LL | | println!("LRM world");
187+
LL | | }
188+
LL | | }
189+
| |_____^
190+
|
191+
help: collapse nested if block
192+
|
193+
LL ~ } else‎ if y == "world" {
194+
LL + println!("LRM world");
195+
LL + }
196+
|
197+
198+
error: this `else { if .. }` block can be collapsed
199+
--> tests/ui/collapsible_else_if.rs:208:12
200+
|
201+
LL | } else‏{
202+
| ___________^
203+
LL | | if y == "world" {
204+
LL | | println!("RLM world");
205+
LL | | }
206+
LL | | }
207+
| |_____^
208+
|
209+
help: collapse nested if block
210+
|
211+
LL ~ } else‏ if y == "world" {
212+
LL + println!("RLM world");
213+
LL + }
214+
|
215+
216+
error: aborting due to 12 previous errors
181217

0 commit comments

Comments
 (0)