mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Make HirEqInterExpr::eq_block take comments into account

Browse Source 
This commit:
- now makes `HirEqInterExpr::eq_block` take comments into account. Identical code with varying comments will no longer be considered equal.
- makes necessary adjustments to UI tests.
This commit is contained in:
Aneesh Kadiyala 2024-01-02 11:30:20 +05:30
parent e73bb00542
commit 543d56938e
11 changed files with 73 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
clippy_utils/src/hir_utils.rs
View File

@ -134,7 +134,7 @@ pub fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool {
/// Checks whether two blocks are the same.
#[expect(clippy::similar_names)]
fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
use TokenKind::{BlockComment, LineComment, Semi, Whitespace};
use TokenKind::{Semi, Whitespace};
if left.stmts.len() != right.stmts.len() {
return false;
}
@ -177,7 +177,7 @@ fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
return false;
}
if !eq_span_tokens(self.inner.cx, lstart..lstmt_span.lo, rstart..rstmt_span.lo, |t| {
!matches!(t, Whitespace | LineComment { .. } | BlockComment { .. } | Semi)
!matches!(t, Whitespace | Semi)
}) {
return false;
}
@ -212,7 +212,7 @@ fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
return false;
}
eq_span_tokens(self.inner.cx, lstart..lend, rstart..rend, |t| {
!matches!(t, Whitespace | LineComment { .. } | BlockComment { .. } | Semi)
!matches!(t, Whitespace | Semi)
})
}

8
tests/ui/branches_sharing_code/shared_at_top.rs
View File

@ -9,17 +9,16 @@ fn simple_examples() {
// Simple
if true {
//~^ ERROR: all if blocks contain the same code at the start
println!("Hello World!");
println!("I'm branch nr: 1");
} else {
println!("Hello World!");
println!("I'm branch nr: 2");
}
//~^^^^^^^ ERROR: all if blocks contain the same code at the start
// Else if
if x == 0 {
//~^ ERROR: all if blocks contain the same code at the start
let y = 9;
println!("The value y was set to: `{}`", y);
let _z = y;
@ -38,6 +37,7 @@ fn simple_examples() {
println!("Ha, Pascal allows you to start the array where you want")
}
//~^^^^^^^^^^^^^^^^^^^ ERROR: all if blocks contain the same code at the start
// Return a value
let _ = if x == 7 {
@ -60,7 +60,6 @@ fn simple_but_suggestion_is_invalid() {
// Can't be automatically moved because used_value_name is getting used again
let used_value_name = 19;
if x == 10 {
//~^ ERROR: all if blocks contain the same code at the start
let used_value_name = "Different type";
println!("Str: {}", used_value_name);
let _ = 1;
@ -69,6 +68,7 @@ fn simple_but_suggestion_is_invalid() {
println!("Str: {}", used_value_name);
let _ = 2;
}
//~^^^^^^^^^ ERROR: all if blocks contain the same code at the start
let _ = used_value_name;
// This can be automatically moved as `can_be_overridden` is not used again
@ -101,11 +101,11 @@ fn check_if_same_than_else_mask() {
}
if x == 2019 {
//~^ ERROR: this `if` has identical blocks
println!("This should trigger `IS_SAME_THAN_ELSE` as usual");
} else {
println!("This should trigger `IS_SAME_THAN_ELSE` as usual");
}
//~^^^^^ ERROR: this `if` has identical blocks
}
#[allow(clippy::vec_init_then_push)]

6
tests/ui/branches_sharing_code/shared_at_top.stderr
View File

@ -2,7 +2,6 @@ error: all if blocks contain the same code at the start
--> $DIR/shared_at_top.rs:11:5
|
LL | / if true {
LL | |
LL | | println!("Hello World!");
| |_________________________________^
|
@ -21,7 +20,6 @@ error: all if blocks contain the same code at the start
--> $DIR/shared_at_top.rs:21:5
|
LL | / if x == 0 {
LL | |
LL | | let y = 9;
LL | | println!("The value y was set to: `{}`", y);
LL | | let _z = y;
@ -54,7 +52,6 @@ error: all if blocks contain the same code at the start
--> $DIR/shared_at_top.rs:62:5
|
LL | / if x == 10 {
LL | |
LL | | let used_value_name = "Different type";
LL | | println!("Str: {}", used_value_name);
| |_____________________________________________^
@ -105,13 +102,12 @@ error: this `if` has identical blocks
|
LL | if x == 2019 {
| __________________^
LL | |
LL | | println!("This should trigger `IS_SAME_THAN_ELSE` as usual");
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/shared_at_top.rs:106:12
--> $DIR/shared_at_top.rs:105:12
|
LL | } else {
| ____________^

12
tests/ui/branches_sharing_code/valid_if_blocks.rs
View File

@ -107,9 +107,9 @@ fn valid_examples() {
// Let's test empty blocks
if false {
//~^ ERROR: this `if` has identical blocks
} else {
}
//~^^^ ERROR: this `if` has identical blocks
}
/// This makes sure that the `if_same_then_else` masks the `shared_code_in_if_blocks` lint
@ -119,7 +119,6 @@ fn trigger_other_lint() {
// Same block
if x == 0 {
//~^ ERROR: this `if` has identical blocks
let u = 19;
println!("How are u today?");
let _ = "This is a string";
@ -128,6 +127,7 @@ fn trigger_other_lint() {
println!("How are u today?");
let _ = "This is a string";
}
//~^^^^^^^^^ ERROR: this `if` has identical blocks
// Only same expression
let _ = if x == 6 { 7 } else { 7 };
@ -138,28 +138,24 @@ fn trigger_other_lint() {
println!("Well I'm the most important block");
"I'm a pretty string"
} else if x == 68 {
//~^ ERROR: this `if` has identical blocks
println!("I'm a doppelgänger");
// Don't listen to my clone below
if y == 90 { "=^.^=" } else { ":D" }
} else {
// Don't listen to my clone above
println!("I'm a doppelgänger");
if y == 90 { "=^.^=" } else { ":D" }
};
//~^^^^^^^^^ ERROR: this `if` has identical blocks
if x == 0 {
println!("I'm single");
} else if x == 68 {
//~^ ERROR: this `if` has identical blocks
println!("I'm a doppelgänger");
// Don't listen to my clone below
} else {
// Don't listen to my clone above
println!("I'm a doppelgänger");
}
//~^^^^^ ERROR: this `if` has identical blocks
}
fn main() {}

18
tests/ui/branches_sharing_code/valid_if_blocks.stderr
View File

@ -3,12 +3,11 @@ error: this `if` has identical blocks
|
LL | if false {
| ______________^
LL | |
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/valid_if_blocks.rs:111:12
--> $DIR/valid_if_blocks.rs:110:12
|
LL | } else {
| ____________^
@ -25,7 +24,6 @@ error: this `if` has identical blocks
|
LL | if x == 0 {
| _______________^
LL | |
LL | | let u = 19;
LL | | println!("How are u today?");
LL | | let _ = "This is a string";
@ -33,7 +31,7 @@ LL | | } else {
| |_____^
|
note: same as this
--> $DIR/valid_if_blocks.rs:126:12
--> $DIR/valid_if_blocks.rs:125:12
|
LL | } else {
| ____________^
@ -60,20 +58,17 @@ error: this `if` has identical blocks
|
LL | } else if x == 68 {
| _______________________^
LL | |
LL | | println!("I'm a doppelgänger");
LL | | // Don't listen to my clone below
LL | |
LL | | if y == 90 { "=^.^=" } else { ":D" }
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/valid_if_blocks.rs:146:12
--> $DIR/valid_if_blocks.rs:144:12
|
LL | } else {
| ____________^
LL | | // Don't listen to my clone above
LL | | println!("I'm a doppelgänger");
LL | |
LL | | if y == 90 { "=^.^=" } else { ":D" }
@ -81,22 +76,19 @@ LL | | };
| |_____^
error: this `if` has identical blocks
--> $DIR/valid_if_blocks.rs:155:23
--> $DIR/valid_if_blocks.rs:153:23
|
LL | } else if x == 68 {
| _______________________^
LL | |
LL | | println!("I'm a doppelgänger");
LL | | // Don't listen to my clone below
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/valid_if_blocks.rs:159:12
--> $DIR/valid_if_blocks.rs:155:12
|
LL | } else {
| ____________^
LL | | // Don't listen to my clone above
LL | | println!("I'm a doppelgänger");
LL | | }
| |_____^

22
tests/ui/if_same_then_else.rs
View File

@ -21,7 +21,6 @@ fn foo() -> bool {
fn if_same_then_else() {
if true {
//~^ ERROR: this `if` has identical blocks
Foo { bar: 42 };
0..10;
..;
@ -38,6 +37,7 @@ fn if_same_then_else() {
0..=10;
foo();
}
//~^^^^^^^^^^^^^^^^^ ERROR: this `if` has identical blocks
if true {
Foo { bar: 42 };
@ -64,19 +64,11 @@ fn if_same_then_else() {
foo();
}
let _ = if true {
let _ = if true { 0.0 } else { 0.0 };
//~^ ERROR: this `if` has identical blocks
0.0
} else {
0.0
};
let _ = if true {
let _ = if true { -0.0 } else { -0.0 };
//~^ ERROR: this `if` has identical blocks
-0.0
} else {
-0.0
};
let _ = if true { 0.0 } else { -0.0 };
@ -87,15 +79,10 @@ fn if_same_then_else() {
foo();
}
let _ = if true {
let _ = if true { 42 } else { 42 };
//~^ ERROR: this `if` has identical blocks
42
} else {
42
};
if true {
//~^ ERROR: this `if` has identical blocks
let bar = if true { 42 } else { 43 };
while foo() {
@ -110,6 +97,7 @@ fn if_same_then_else() {
}
bar + 1;
}
//~^^^^^^^^^^^^^^^ ERROR: this `if` has identical blocks
if true {
let _ = match 42 {

69
tests/ui/if_same_then_else.stderr
View File

@ -3,16 +3,16 @@ error: this `if` has identical blocks
|
LL | if true {
| _____________^
LL | |
LL | | Foo { bar: 42 };
LL | | 0..10;
LL | | ..;
... |
LL | | foo();
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:32:12
--> $DIR/if_same_then_else.rs:31:12
|
LL | } else {
| ____________^
@ -29,75 +29,54 @@ LL | | }
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:67:21
|
LL | let _ = if true {
| _____________________^
LL | |
LL | | 0.0
LL | | } else {
| |_____^
LL | let _ = if true { 0.0 } else { 0.0 };
| ^^^^^^^
|
note: same as this
--> $DIR/if_same_then_else.rs:70:12
--> $DIR/if_same_then_else.rs:67:34
|
LL | } else {
| ____________^
LL | | 0.0
LL | | };
| |_____^
LL | let _ = if true { 0.0 } else { 0.0 };
| ^^^^^^^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:74:21
--> $DIR/if_same_then_else.rs:70:21
|
LL | let _ = if true {
| _____________________^
LL | |
LL | | -0.0
LL | | } else {
| |_____^
LL | let _ = if true { -0.0 } else { -0.0 };
| ^^^^^^^^
|
note: same as this
--> $DIR/if_same_then_else.rs:77:12
--> $DIR/if_same_then_else.rs:70:35
|
LL | } else {
| ____________^
LL | | -0.0
LL | | };
| |_____^
LL | let _ = if true { -0.0 } else { -0.0 };
| ^^^^^^^^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:90:21
--> $DIR/if_same_then_else.rs:82:21
|
LL | let _ = if true {
| _____________________^
LL | |
LL | | 42
LL | | } else {
| |_____^
LL | let _ = if true { 42 } else { 42 };
| ^^^^^^
|
note: same as this
--> $DIR/if_same_then_else.rs:93:12
--> $DIR/if_same_then_else.rs:82:33
|
LL | } else {
| ____________^
LL | | 42
LL | | };
| |_____^
LL | let _ = if true { 42 } else { 42 };
| ^^^^^^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:97:13
--> $DIR/if_same_then_else.rs:85:13
|
LL | if true {
| _____________^
LL | |
LL | | let bar = if true { 42 } else { 43 };
LL | |
LL | | while foo() {
... |
LL | | bar + 1;
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:105:12
--> $DIR/if_same_then_else.rs:92:12
|
LL | } else {
| ____________^
@ -110,7 +89,7 @@ LL | | }
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:250:14
--> $DIR/if_same_then_else.rs:238:14
|
LL | if x {
| ______________^
@ -119,7 +98,7 @@ LL | | } else {
| |_________^
|
note: same as this
--> $DIR/if_same_then_else.rs:252:16
--> $DIR/if_same_then_else.rs:240:16
|
LL | } else {
| ________________^

16
tests/ui/if_same_then_else2.rs
View File

@ -13,7 +13,6 @@
fn if_same_then_else2() -> Result<&'static str, ()> {
if true {
//~^ ERROR: this `if` has identical blocks
for _ in &[42] {
let foo: &Option<_> = &Some::<u8>(42);
if foo.is_some() {
@ -32,20 +31,21 @@ fn if_same_then_else2() -> Result<&'static str, ()> {
}
}
}
//~^^^^^^^^^^^^^^^^^^^ ERROR: this `if` has identical blocks
if true {
//~^ ERROR: this `if` has identical blocks
if let Some(a) = Some(42) {}
} else {
if let Some(a) = Some(42) {}
}
//~^^^^^ ERROR: this `if` has identical blocks
if true {
//~^ ERROR: this `if` has identical blocks
if let (1, .., 3) = (1, 2, 3) {}
} else {
if let (1, .., 3) = (1, 2, 3) {}
}
//~^^^^^ ERROR: this `if` has identical blocks
if true {
if let (1, .., 3) = (1, 2, 3) {}
@ -90,19 +90,15 @@ fn if_same_then_else2() -> Result<&'static str, ()> {
}
// Same NaNs
let _ = if true {
let _ = if true { f32::NAN } else { f32::NAN };
//~^ ERROR: this `if` has identical blocks
f32::NAN
} else {
f32::NAN
};
if true {
//~^ ERROR: this `if` has identical blocks
Ok("foo")?;
} else {
Ok("foo")?;
}
//~^^^^^ ERROR: this `if` has identical blocks
if true {
let foo = "";
@ -122,13 +118,13 @@ fn if_same_then_else2() -> Result<&'static str, ()> {
let foo = "bar";
return Ok(&foo[0..]);
} else if true {
//~^ ERROR: this `if` has identical blocks
let foo = "";
return Ok(&foo[0..]);
} else {
let foo = "";
return Ok(&foo[0..]);
}
//~^^^^^^^ ERROR: this `if` has identical blocks
// False positive `if_same_then_else`: `let (x, y)` vs. `let (y, x)`; see issue #3559.
if true {

37
tests/ui/if_same_then_else2.stderr
View File

@ -3,16 +3,16 @@ error: this `if` has identical blocks
|
LL | if true {
| _____________^
LL | |
LL | | for _ in &[42] {
LL | | let foo: &Option<_> = &Some::<u8>(42);
LL | | if foo.is_some() {
... |
LL | | }
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:25:12
--> $DIR/if_same_then_else2.rs:24:12
|
LL | } else {
| ____________^
@ -31,13 +31,12 @@ error: this `if` has identical blocks
|
LL | if true {
| _____________^
LL | |
LL | | if let Some(a) = Some(42) {}
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:39:12
--> $DIR/if_same_then_else2.rs:38:12
|
LL | } else {
| ____________^
@ -50,13 +49,12 @@ error: this `if` has identical blocks
|
LL | if true {
| _____________^
LL | |
LL | | if let (1, .., 3) = (1, 2, 3) {}
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:46:12
--> $DIR/if_same_then_else2.rs:45:12
|
LL | } else {
| ____________^
@ -67,34 +65,26 @@ LL | | }
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:93:21
|
LL | let _ = if true {
| _____________________^
LL | |
LL | | f32::NAN
LL | | } else {
| |_____^
LL | let _ = if true { f32::NAN } else { f32::NAN };
| ^^^^^^^^^^^^
|
note: same as this
--> $DIR/if_same_then_else2.rs:96:12
--> $DIR/if_same_then_else2.rs:93:39
|
LL | } else {
| ____________^
LL | | f32::NAN
LL | | };
| |_____^
LL | let _ = if true { f32::NAN } else { f32::NAN };
| ^^^^^^^^^^^^
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:100:13
--> $DIR/if_same_then_else2.rs:96:13
|
LL | if true {
| _____________^
LL | |
LL | | Ok("foo")?;
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:103:12
--> $DIR/if_same_then_else2.rs:98:12
|
LL | } else {
| ____________^
@ -103,18 +93,17 @@ LL | | }
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:124:20
--> $DIR/if_same_then_else2.rs:120:20
|
LL | } else if true {
| ____________________^
LL | |
LL | | let foo = "";
LL | | return Ok(&foo[0..]);
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:128:12
--> $DIR/if_same_then_else2.rs:123:12
|
LL | } else {
| ____________^

4
tests/ui/match_same_arms2.rs
View File

@ -13,7 +13,6 @@ fn foo() -> bool {
fn match_same_arms() {
let _ = match 42 {
42 => {
//~^ ERROR: this match arm has an identical body to the `_` wildcard arm
foo();
let mut a = 42 + [23].len() as i32;
if true {
@ -32,6 +31,7 @@ fn match_same_arms() {
a
},
};
//~^^^^^^^^^^^^^^^^^^^ ERROR: this match arm has an identical body to the `_` wildcard arm
let _ = match 42 {
42 => foo(),
@ -146,13 +146,13 @@ macro_rules! empty {
empty!(0);
},
1 => {
//~^ ERROR: this match arm has an identical body to another arm
empty!(0);
},
x => {
empty!(x);
},
}
//~^^^^^^^ ERROR: this match arm has an identical body to another arm
match_expr_like_matches_macro_priority();
}

5
tests/ui/match_same_arms2.stderr
View File

@ -2,9 +2,9 @@ error: this match arm has an identical body to the `_` wildcard arm
--> $DIR/match_same_arms2.rs:15:9
|
LL | / 42 => {
LL | |
LL | | foo();
LL | | let mut a = 42 + [23].len() as i32;
LL | | if true {
... |
LL | | a
LL | | },
@ -12,7 +12,7 @@ LL | | },
|
= help: or try changing either arm body
note: `_` wildcard arm here
--> $DIR/match_same_arms2.rs:25:9
--> $DIR/match_same_arms2.rs:24:9
|
LL | / _ => {
LL | | foo();
@ -122,7 +122,6 @@ LL | 1 => {
| ^ help: try merging the arm patterns: `1 | 0`
| _________|
| |
LL | |
LL | | empty!(0);
LL | | },
| |_________^