Skip to content

Commit 47c1b32

Browse files
authored
fix(needless_ifs): handle vertical tab as whitespace to avoid false negative (rust-lang#16845)
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16845)* `needless_ifs` has a false negative because rust lexer considers vertical tab as a whitespace but `is_ascii_whitespace()` method doesn't handle vertical tab which is a bug. For reference https://doc.rust-lang.org/reference/whitespace.html. Reference Outreachy tracking issue rustfoundation/interop-initiative#53. Should we also switch from `is_ascii_whitespace()` to `is_whitespace()` to handle full Unicode `Pattern_White_Space` characters like `U+0085`, `U+200E`, `U+200F`, `U+2028`, `U+2029`? Fixes:- rustfoundation/interop-initiative#53 changelog: [`needless_ifs`]: Fix false negative in `needless_ifs` when code contains vertical tab (`\x0B`) whitespace CC:- @teor2345
2 parents 84bb31a + 146f9ab commit 47c1b32

4 files changed

Lines changed: 35 additions & 13 deletions

File tree

clippy_lints/src/needless_ifs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl LateLintPass<'_> for NeedlessIfs {
5555
// - comments
5656
// - #[cfg]'d out code
5757
src.bytes()
58-
.all(|ch| matches!(ch, b'{' | b'}') || ch.is_ascii_whitespace())
58+
.all(|ch| matches!(ch, b'{' | b'}') || ch.is_ascii_whitespace() || ch == b'\x0b')
5959
})
6060
&& let Some(cond_span) = walk_span_to_context(cond.span, expr.span.ctxt())
6161
&& let Some(cond_snippet) = cond_span.get_source_text(cx)

tests/ui/needless_ifs.fixed

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
unused
1414
)]
1515
#![warn(clippy::needless_ifs)]
16-
1716
extern crate proc_macros;
1817
use proc_macros::{external, with_span};
1918

@@ -113,3 +112,12 @@ fn issue15960() -> i32 {
113112

114113
1 // put something here so that `if` is a statement not an expression
115114
}
115+
116+
#[rustfmt::skip]
117+
fn issue_16845() {
118+
119+
// Vertical tab (U+000B) should be treated as whitespace,
120+
121+
//~^ needless_ifs
122+
let () = if maybe_side_effect() {};
123+
}

tests/ui/needless_ifs.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
unused
1414
)]
1515
#![warn(clippy::needless_ifs)]
16-
1716
extern crate proc_macros;
1817
use proc_macros::{external, with_span};
1918

@@ -114,3 +113,12 @@ fn issue15960() -> i32 {
114113

115114
1 // put something here so that `if` is a statement not an expression
116115
}
116+
117+
#[rustfmt::skip]
118+
fn issue_16845() {
119+
120+
// Vertical tab (U+000B) should be treated as whitespace,
121+
if true { }
122+
//~^ needless_ifs
123+
let () = if maybe_side_effect() {};
124+
}

tests/ui/needless_ifs.stderr

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `if` branch is empty
2-
--> tests/ui/needless_ifs.rs:26:5
2+
--> tests/ui/needless_ifs.rs:25:5
33
|
44
LL | if (true) {}
55
| ^^^^^^^^^^^^ help: you can remove it
@@ -8,13 +8,13 @@ LL | if (true) {}
88
= help: to override `-D warnings` add `#[allow(clippy::needless_ifs)]`
99

1010
error: this `if` branch is empty
11-
--> tests/ui/needless_ifs.rs:29:5
11+
--> tests/ui/needless_ifs.rs:28:5
1212
|
1313
LL | if maybe_side_effect() {}
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `maybe_side_effect();`
1515

1616
error: this `if` branch is empty
17-
--> tests/ui/needless_ifs.rs:35:5
17+
--> tests/ui/needless_ifs.rs:34:5
1818
|
1919
LL | / if {
2020
LL | |
@@ -31,7 +31,7 @@ LL + });
3131
|
3232

3333
error: this `if` branch is empty
34-
--> tests/ui/needless_ifs.rs:50:5
34+
--> tests/ui/needless_ifs.rs:49:5
3535
|
3636
LL | / if {
3737
LL | |
@@ -57,34 +57,40 @@ LL + } && true);
5757
|
5858

5959
error: this `if` branch is empty
60-
--> tests/ui/needless_ifs.rs:95:5
60+
--> tests/ui/needless_ifs.rs:94:5
6161
|
6262
LL | if { maybe_side_effect() } {}
6363
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `({ maybe_side_effect() });`
6464

6565
error: this `if` branch is empty
66-
--> tests/ui/needless_ifs.rs:98:5
66+
--> tests/ui/needless_ifs.rs:97:5
6767
|
6868
LL | if { maybe_side_effect() } && true {}
6969
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `({ maybe_side_effect() } && true);`
7070

7171
error: this `if` branch is empty
72-
--> tests/ui/needless_ifs.rs:103:5
72+
--> tests/ui/needless_ifs.rs:102:5
7373
|
7474
LL | if true {}
7575
| ^^^^^^^^^^ help: you can remove it: `true;`
7676

7777
error: this `if` branch is empty
78-
--> tests/ui/needless_ifs.rs:110:5
78+
--> tests/ui/needless_ifs.rs:109:5
7979
|
8080
LL | if matches!(2, 3) {}
8181
| ^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `matches!(2, 3);`
8282

8383
error: this `if` branch is empty
84-
--> tests/ui/needless_ifs.rs:112:5
84+
--> tests/ui/needless_ifs.rs:111:5
8585
|
8686
LL | if matches!(2, 3) == (2 * 2 == 5) {}
8787
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `matches!(2, 3) == (2 * 2 == 5);`
8888

89-
error: aborting due to 9 previous errors
89+
error: this `if` branch is empty
90+
--> tests/ui/needless_ifs.rs:121:5
91+
|
92+
LL | if true {␋}
93+
| ^^^^^^^^^^^ help: you can remove it
94+
95+
error: aborting due to 10 previous errors
9096

0 commit comments

Comments
 (0)