Skip to content

Commit c74e49e

Browse files
xFrednetxFrednet
committed
Adapted the lint to use the new SpanlessEq
1 parent 65ed5a6 commit c74e49e

File tree

10 files changed

+111
-196
lines changed

10 files changed

+111
-196
lines changed

‎clippy_lints/src/copies.rs‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -295,11 +295,21 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize,
295295
let l_stmts = win[0].stmts;
296296
let r_stmts = win[1].stmts;
297297

298+
// `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
299+
// The comparison therefor needs to be done in a way that builds the correct context.
298300
let mut evaluator = SpanlessEq::new(cx);
301+
let mut evaluator = evaluator.inter_expr();
302+
299303
let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));
300-
let current_end_eq = count_eq(&mut l_stmts.iter().rev(), &mut r_stmts.iter().rev(), |l, r| {
301-
evaluator.eq_stmt(l, r)
302-
});
304+
305+
let current_end_eq = {
306+
// We skip the middle statements which can't be equal
307+
let end_comparison_count = l_stmts.len().min(r_stmts.len()) - current_start_eq;
308+
let it1 = l_stmts.iter().skip(l_stmts.len() - end_comparison_count);
309+
let it2 = r_stmts.iter().skip(r_stmts.len() - end_comparison_count);
310+
it1.zip(it2)
311+
.fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
312+
};
303313
let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
304314

305315
// IF_SAME_THEN_ELSE
@@ -458,8 +468,8 @@ fn emit_shared_code_in_if_blocks_lint(
458468
// Emit lint
459469
if suggestions.len() == 1 {
460470
let (place_str, span, sugg) = suggestions.pop().unwrap();
461-
let msg = format!("All if blocks contain the same code at the {}", place_str);
462-
let help = format!("Consider moving the {} statements out like this", place_str);
471+
let msg = format!("all if blocks contain the same code at the {}", place_str);
472+
let help = format!("consider moving the {} statements out like this", place_str);
463473
span_lint_and_then(cx, SHARED_CODE_IN_IF_BLOCKS, span, msg.as_str(), |diag| {
464474
diag.span_suggestion(span, help.as_str(), sugg, Applicability::Unspecified);
465475

@@ -472,20 +482,20 @@ fn emit_shared_code_in_if_blocks_lint(
472482
cx,
473483
SHARED_CODE_IN_IF_BLOCKS,
474484
start_span,
475-
"All if blocks contain the same code at the start and the end. Here at the start:",
485+
"all if blocks contain the same code at the start and the end. Here at the start",
476486
move |diag| {
477-
diag.span_note(end_span, "And here at the end:");
487+
diag.span_note(end_span, "and here at the end");
478488

479489
diag.span_suggestion(
480490
start_span,
481-
"Consider moving the start statements out like this:",
491+
"consider moving the start statements out like this",
482492
start_sugg,
483493
Applicability::Unspecified,
484494
);
485495

486496
diag.span_suggestion(
487497
end_span,
488-
"And consider moving the end statements out like this:",
498+
"and consider moving the end statements out like this",
489499
end_sugg,
490500
Applicability::Unspecified,
491501
);

‎clippy_utils/src/hir_utils.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,14 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
5858

5959
/// Use this method to wrap comparisons that may involve inter-expression context.
6060
/// See `self.locals`.
61-
fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> {
61+
pub fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> {
6262
HirEqInterExpr {
6363
inner: self,
6464
locals: HirIdMap::default(),
6565
}
6666
}
6767

68+
#[allow(dead_code)]
6869
pub fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
6970
self.inter_expr().eq_block(left, right)
7071
}
@@ -82,7 +83,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
8283
}
8384
}
8485

85-
struct HirEqInterExpr<'a, 'b, 'tcx> {
86+
pub struct HirEqInterExpr<'a, 'b, 'tcx> {
8687
inner: &'a mut SpanlessEq<'b, 'tcx>,
8788

8889
// When binding are declared, the binding ID in the left expression is mapped to the one on the
@@ -92,7 +93,7 @@ struct HirEqInterExpr<'a, 'b, 'tcx> {
9293
}
9394

9495
impl HirEqInterExpr<'_, '_, '_> {
95-
fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool {
96+
pub fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool {
9697
match (&left.kind, &right.kind) {
9798
(&StmtKind::Local(ref l), &StmtKind::Local(ref r)) => {
9899
self.eq_pat(&l.pat, &r.pat)
@@ -159,7 +160,7 @@ impl HirEqInterExpr<'_, '_, '_> {
159160
}
160161

161162
#[allow(clippy::similar_names)]
162-
fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
163+
pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
163164
if !self.inner.allow_side_effects && differing_macro_contexts(left.span, right.span) {
164165
return false;
165166
}

‎tests/ui/if_same_then_else.stderr‎

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -82,31 +82,5 @@ LL | | 42
8282
LL | | };
8383
| |_____^
8484

85-
error: this `if` has identical blocks
86-
--> $DIR/if_same_then_else.rs:95:13
87-
|
88-
LL | if true {
89-
| _____________^
90-
LL | | let bar = if true { 42 } else { 43 };
91-
LL | |
92-
LL | | while foo() {
93-
... |
94-
LL | | bar + 1;
95-
LL | | } else {
96-
| |_____^
97-
|
98-
note: same as this
99-
--> $DIR/if_same_then_else.rs:102:12
100-
|
101-
LL | } else {
102-
| ____________^
103-
LL | | //~ ERROR same body as `if` block
104-
LL | | let bar = if true { 42 } else { 43 };
105-
LL | |
106-
... |
107-
LL | | bar + 1;
108-
LL | | }
109-
| |_____^
110-
111-
error: aborting due to 5 previous errors
85+
error: aborting due to 4 previous errors
11286

‎tests/ui/if_same_then_else2.stderr‎

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,5 @@
11
error: this `if` has identical blocks
2-
--> $DIR/if_same_then_else2.rs:21:12
3-
|
4-
LL | } else {
5-
| ____________^
6-
LL | | //~ ERROR same body as `if` block
7-
LL | | for _ in &[42] {
8-
LL | | let bar: &Option<_> = &Some::<u8>(42);
9-
... |
10-
LL | | }
11-
LL | | }
12-
| |_____^
13-
|
14-
= note: `-D clippy::if-same-then-else` implied by `-D warnings`
15-
note: same as this
16-
--> $DIR/if_same_then_else2.rs:12:13
2+
--> $DIR/if_same_then_else2.rs:13:13
173
|
184
LL | if true {
195
| _____________^
@@ -33,7 +19,7 @@ LL | } else {
3319
| ____________^
3420
LL | | //~ ERROR same body as `if` block
3521
LL | | for _ in &[42] {
36-
LL | | let foo: &Option<_> = &Some::<u8>(42);
22+
LL | | let bar: &Option<_> = &Some::<u8>(42);
3723
... |
3824
LL | | }
3925
LL | | }
@@ -115,25 +101,5 @@ LL | | Ok("foo")?;
115101
LL | | }
116102
| |_____^
117103

118-
error: this `if` has identical blocks
119-
--> $DIR/if_same_then_else2.rs:122:20
120-
|
121-
LL | } else if true {
122-
| ____________________^
123-
LL | | let foo = "";
124-
LL | | return Ok(&foo[0..]);
125-
LL | | } else {
126-
| |_____^
127-
|
128-
note: same as this
129-
--> $DIR/if_same_then_else2.rs:125:12
130-
|
131-
LL | } else {
132-
| ____________^
133-
LL | | let foo = "";
134-
LL | | return Ok(&foo[0..]);
135-
LL | | }
136-
| |_____^
137-
138-
error: aborting due to 6 previous errors
104+
error: aborting due to 5 previous errors
139105

‎tests/ui/shared_code_in_if_blocks/shared_at_bot.rs‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
// This tests the shared_code_in_if_blocks lint at the end of blocks
55

66
fn simple_examples() {
7-
// TODO xFrednet 2021-01-06: Test with const literals at the end
87
let x = 1;
98

109
let _ = if x == 7 {
@@ -45,8 +44,8 @@ fn simple_examples() {
4544
println!("This is also eq with the else block");
4645
println!("Same end of block");
4746
} else {
48-
println!("Same end of block");
4947
println!("This is also eq with the else block");
48+
println!("Same end of block");
5049
}
5150

5251
// Use of outer scope value
@@ -69,21 +68,11 @@ fn simple_examples() {
6968
);
7069
}
7170

72-
// TODO xFrednet 2021-01.13: Fix lint for `if let`
73-
let index = Some(8);
74-
if let Some(index) = index {
75-
println!("The index is: {}", index);
76-
77-
println!("Same end of block");
78-
} else {
79-
println!("Same end of block");
80-
}
81-
8271
if x == 9 {
8372
if x == 8 {
8473
// No parent!!
85-
println!("Hello World");
8674
println!("---");
75+
println!("Hello World");
8776
} else {
8877
println!("Hello World");
8978
}

‎tests/ui/shared_code_in_if_blocks/shared_at_bot.stderr‎

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
error: All if blocks contain the same code at the end
2-
--> $DIR/shared_at_bot.rs:31:5
1+
error: all if blocks contain the same code at the end
2+
--> $DIR/shared_at_bot.rs:30:5
33
|
44
LL | / let result = false;
55
LL | | println!("Block end!");
@@ -13,16 +13,29 @@ note: the lint level is defined here
1313
LL | #![deny(clippy::if_same_then_else, clippy::shared_code_in_if_blocks)]
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1515
= note: The end suggestion probably needs some adjustments to use the expression result correctly.
16-
help: Consider moving the end statements out like this
16+
help: consider moving the end statements out like this
1717
|
1818
LL | }
1919
LL | let result = false;
2020
LL | println!("Block end!");
2121
LL | result;
2222
|
2323

24-
error: All if blocks contain the same code at the end
25-
--> $DIR/shared_at_bot.rs:66:5
24+
error: all if blocks contain the same code at the end
25+
--> $DIR/shared_at_bot.rs:48:5
26+
|
27+
LL | / println!("Same end of block");
28+
LL | | }
29+
| |_____^
30+
|
31+
help: consider moving the end statements out like this
32+
|
33+
LL | }
34+
LL | println!("Same end of block");
35+
|
36+
37+
error: all if blocks contain the same code at the end
38+
--> $DIR/shared_at_bot.rs:65:5
2639
|
2740
LL | / println!(
2841
LL | | "I'm moveable because I know: `outer_scope_value`: '{}'",
@@ -31,7 +44,7 @@ LL | | );
3144
LL | | }
3245
| |_____^
3346
|
34-
help: Consider moving the end statements out like this
47+
help: consider moving the end statements out like this
3548
|
3649
LL | }
3750
LL | println!(
@@ -40,22 +53,21 @@ LL | outer_scope_value
4053
LL | );
4154
|
4255

43-
error: All if blocks contain the same code at the start
44-
--> $DIR/shared_at_bot.rs:83:9
56+
error: all if blocks contain the same code at the end
57+
--> $DIR/shared_at_bot.rs:77:9
4558
|
46-
LL | / if x == 8 {
47-
LL | | // No parent!!
48-
LL | | println!("Hello World");
49-
| |____________________________________^
59+
LL | / println!("Hello World");
60+
LL | | }
61+
| |_________^
5062
|
51-
help: Consider moving the start statements out like this
63+
help: consider moving the end statements out like this
5264
|
65+
LL | }
5366
LL | println!("Hello World");
54-
LL | if x == 8 {
5567
|
5668

57-
error: All if blocks contain the same code at the end
58-
--> $DIR/shared_at_bot.rs:104:5
69+
error: all if blocks contain the same code at the end
70+
--> $DIR/shared_at_bot.rs:93:5
5971
|
6072
LL | / let later_used_value = "A string value";
6173
LL | | println!("{}", later_used_value);
@@ -64,68 +76,68 @@ LL | | }
6476
| |_____^
6577
|
6678
= warning: Some moved values might need to be renamed to avoid wrong references.
67-
help: Consider moving the end statements out like this
79+
help: consider moving the end statements out like this
6880
|
6981
LL | }
7082
LL | let later_used_value = "A string value";
7183
LL | println!("{}", later_used_value);
7284
|
7385

74-
error: All if blocks contain the same code at the end
75-
--> $DIR/shared_at_bot.rs:117:5
86+
error: all if blocks contain the same code at the end
87+
--> $DIR/shared_at_bot.rs:106:5
7688
|
7789
LL | / let simple_examples = "I now identify as a &str :)";
7890
LL | | println!("This is the new simple_example: {}", simple_examples);
7991
LL | | }
8092
| |_____^
8193
|
8294
= warning: Some moved values might need to be renamed to avoid wrong references.
83-
help: Consider moving the end statements out like this
95+
help: consider moving the end statements out like this
8496
|
8597
LL | }
8698
LL | let simple_examples = "I now identify as a &str :)";
8799
LL | println!("This is the new simple_example: {}", simple_examples);
88100
|
89101

90-
error: All if blocks contain the same code at the end
91-
--> $DIR/shared_at_bot.rs:182:5
102+
error: all if blocks contain the same code at the end
103+
--> $DIR/shared_at_bot.rs:171:5
92104
|
93105
LL | / x << 2
94106
LL | | };
95107
| |_____^
96108
|
97109
= note: The end suggestion probably needs some adjustments to use the expression result correctly.
98-
help: Consider moving the end statements out like this
110+
help: consider moving the end statements out like this
99111
|
100112
LL | }
101113
LL | x << 2;
102114
|
103115

104-
error: All if blocks contain the same code at the end
105-
--> $DIR/shared_at_bot.rs:189:5
116+
error: all if blocks contain the same code at the end
117+
--> $DIR/shared_at_bot.rs:178:5
106118
|
107119
LL | / x * 4
108120
LL | | }
109121
| |_____^
110122
|
111123
= note: The end suggestion probably needs some adjustments to use the expression result correctly.
112-
help: Consider moving the end statements out like this
124+
help: consider moving the end statements out like this
113125
|
114126
LL | }
115127
LL | x * 4
116128
|
117129

118-
error: All if blocks contain the same code at the end
119-
--> $DIR/shared_at_bot.rs:201:44
130+
error: all if blocks contain the same code at the end
131+
--> $DIR/shared_at_bot.rs:190:44
120132
|
121133
LL | if x == 17 { b = 1; a = 0x99; } else { a = 0x99; }
122134
| ^^^^^^^^^^^
123135
|
124-
help: Consider moving the end statements out like this
136+
help: consider moving the end statements out like this
125137
|
126138
LL | if x == 17 { b = 1; a = 0x99; } else { }
127139
LL | a = 0x99;
128140
|
129141

130-
error: aborting due to 8 previous errors
142+
error: aborting due to 9 previous errors
131143

0 commit comments

Comments
 (0)