Skip to content

Commit 8aaae3a

Browse files
xFrednetxFrednet
committed
Updated code for dogfood
1 parent b39b064 commit 8aaae3a

File tree

3 files changed

+134
-68
lines changed

3 files changed

+134
-68
lines changed

‎clippy_lints/src/copies.rs‎

Lines changed: 75 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
use crate::utils::{both, count_eq, eq_expr_value, in_macro, search_same, SpanlessEq, SpanlessHash};
21
use crate::utils::{
3-
first_line_of_span, get_enclosing_block, get_parent_expr, if_sequence, indent_of, parent_node_is_if_expr,
4-
reindent_multiline, snippet, snippet_opt, span_lint_and_note, span_lint_and_then, ContainsName,
2+
both, count_eq, eq_expr_value, first_line_of_span, get_enclosing_block, get_parent_expr, if_sequence, in_macro,
3+
indent_of, parent_node_is_if_expr, reindent_multiline, run_lints, search_same, snippet, snippet_opt,
4+
span_lint_and_note, span_lint_and_then, ContainsName, SpanlessEq, SpanlessHash,
55
};
6+
use if_chain::if_chain;
67
use rustc_data_structures::fx::FxHashSet;
78
use rustc_errors::{Applicability, DiagnosticBuilder};
89
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
@@ -188,57 +189,15 @@ fn lint_same_then_else<'tcx>(
188189
return;
189190
}
190191

191-
let has_expr = blocks[0].expr.is_some();
192-
193192
// Check if each block has shared code
194-
let mut start_eq = usize::MAX;
195-
let mut end_eq = usize::MAX;
196-
let mut expr_eq = true;
197-
for win in blocks.windows(2) {
198-
let l_stmts = win[0].stmts;
199-
let r_stmts = win[1].stmts;
200-
201-
let mut evaluator = SpanlessEq::new(cx);
202-
let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));
203-
let current_end_eq = count_eq(&mut l_stmts.iter().rev(), &mut r_stmts.iter().rev(), |l, r| {
204-
evaluator.eq_stmt(l, r)
205-
});
206-
let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
207-
208-
// IF_SAME_THEN_ELSE
209-
if block_expr_eq && l_stmts.len() == r_stmts.len() && l_stmts.len() == current_start_eq {
210-
span_lint_and_note(
211-
cx,
212-
IF_SAME_THEN_ELSE,
213-
win[0].span,
214-
"this `if` has identical blocks",
215-
Some(win[1].span),
216-
"same as this",
217-
);
218-
219-
return;
220-
}
221-
222-
start_eq = start_eq.min(current_start_eq);
223-
end_eq = end_eq.min(current_end_eq);
224-
expr_eq &= block_expr_eq;
225-
}
193+
let has_expr = blocks[0].expr.is_some();
194+
let (start_eq, mut end_eq, expr_eq) = scan_block_for_eq(cx, blocks);
226195

227196
// SHARED_CODE_IN_IF_BLOCKS prerequisites
228197
if !has_unconditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
229198
return;
230199
}
231200

232-
if has_expr && !expr_eq {
233-
end_eq = 0;
234-
}
235-
236-
// Check if the regions are overlapping. Set `end_eq` to prevent the overlap
237-
let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap();
238-
if (start_eq + end_eq) > min_block_size {
239-
end_eq = min_block_size - start_eq;
240-
}
241-
242201
// Only the start is the same
243202
if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) {
244203
let block = blocks[0];
@@ -302,14 +261,13 @@ fn lint_same_then_else<'tcx>(
302261
end_eq -= moved_start;
303262
}
304263

305-
let end_linable = if let Some(expr) = block.expr {
306-
intravisit::walk_expr(&mut end_walker, expr);
307-
end_walker.uses.iter().any(|x| !block_defs.contains(x))
308-
} else if end_eq == 0 {
309-
false
310-
} else {
311-
true
312-
};
264+
let end_linable = block.expr.map_or_else(
265+
|| end_eq != 0,
266+
|expr| {
267+
intravisit::walk_expr(&mut end_walker, expr);
268+
end_walker.uses.iter().any(|x| !block_defs.contains(x))
269+
},
270+
);
313271

314272
if end_linable {
315273
end_walker.def_symbols.drain().for_each(|x| {
@@ -329,13 +287,67 @@ fn lint_same_then_else<'tcx>(
329287
}
330288
}
331289

290+
fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize, usize, bool) {
291+
let mut start_eq = usize::MAX;
292+
let mut end_eq = usize::MAX;
293+
let mut expr_eq = true;
294+
for win in blocks.windows(2) {
295+
let l_stmts = win[0].stmts;
296+
let r_stmts = win[1].stmts;
297+
298+
let mut evaluator = SpanlessEq::new(cx);
299+
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+
});
303+
let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
304+
305+
// IF_SAME_THEN_ELSE
306+
if_chain! {
307+
if block_expr_eq;
308+
if l_stmts.len() == r_stmts.len();
309+
if l_stmts.len() == current_start_eq;
310+
if run_lints(cx, &[IF_SAME_THEN_ELSE], win[0].hir_id);
311+
if run_lints(cx, &[IF_SAME_THEN_ELSE], win[1].hir_id);
312+
then {
313+
span_lint_and_note(
314+
cx,
315+
IF_SAME_THEN_ELSE,
316+
win[0].span,
317+
"this `if` has identical blocks",
318+
Some(win[1].span),
319+
"same as this",
320+
);
321+
322+
return (0, 0, false);
323+
}
324+
}
325+
326+
start_eq = start_eq.min(current_start_eq);
327+
end_eq = end_eq.min(current_end_eq);
328+
expr_eq &= block_expr_eq;
329+
}
330+
331+
let has_expr = blocks[0].expr.is_some();
332+
if has_expr && !expr_eq {
333+
end_eq = 0;
334+
}
335+
336+
// Check if the regions are overlapping. Set `end_eq` to prevent the overlap
337+
let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap();
338+
if (start_eq + end_eq) > min_block_size {
339+
end_eq = min_block_size - start_eq;
340+
}
341+
342+
(start_eq, end_eq, expr_eq)
343+
}
344+
332345
fn check_for_warn_of_moved_symbol(
333346
cx: &LateContext<'tcx>,
334347
symbols: &FxHashSet<Symbol>,
335348
if_expr: &'tcx Expr<'_>,
336349
) -> bool {
337-
// Obs true as we include the current if block
338-
if let Some(block) = get_enclosing_block(cx, if_expr.hir_id) {
350+
get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| {
339351
let ignore_span = block.span.shrink_to_lo().to(if_expr.span);
340352

341353
symbols
@@ -360,9 +372,7 @@ fn check_for_warn_of_moved_symbol(
360372

361373
walker.result
362374
})
363-
} else {
364-
false
365-
}
375+
})
366376
}
367377

368378
fn emit_shared_code_in_if_blocks_lint(
@@ -410,12 +420,10 @@ fn emit_shared_code_in_if_blocks_lint(
410420
block.stmts[block.stmts.len() - end_stmts].span
411421
}
412422
.source_callsite();
413-
let moved_end = if let Some(expr) = block.expr {
414-
expr.span
415-
} else {
416-
block.stmts[block.stmts.len() - 1].span
417-
}
418-
.source_callsite();
423+
let moved_end = block
424+
.expr
425+
.map_or_else(|| block.stmts[block.stmts.len() - 1].span, |expr| expr.span)
426+
.source_callsite();
419427

420428
let moved_span = moved_start.to(moved_end);
421429
let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
@@ -488,7 +496,7 @@ fn emit_shared_code_in_if_blocks_lint(
488496
}
489497
}
490498

491-
/// This visitor collects HirIds and Symbols of defined symbols and HirIds of used values.
499+
/// This visitor collects `HirId`s and Symbols of defined symbols and `HirId`s of used values.
492500
struct UsedValueFinderVisitor<'a, 'tcx> {
493501
cx: &'a LateContext<'tcx>,
494502

‎tests/ui/shared_code_in_if_blocks/shared_at_top.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,24 @@ fn simple_but_suggestion_is_invalid() {
8080
}
8181
}
8282

83+
/// This function tests that the `IS_SAME_THAN_ELSE` only covers the lint if it's enabled.
84+
fn check_if_same_than_else_mask() {
85+
let x = 2021;
86+
87+
#[allow(clippy::if_same_then_else)]
88+
if x == 2020 {
89+
println!("This should trigger the `SHARED_CODE_IN_IF_BLOCKS` lint.");
90+
println!("Because `IF_SAME_THEN_ELSE` is allowed here");
91+
} else {
92+
println!("This should trigger the `SHARED_CODE_IN_IF_BLOCKS` lint.");
93+
println!("Because `IF_SAME_THEN_ELSE` is allowed here");
94+
}
95+
96+
if x == 2019 {
97+
println!("This should trigger `IS_SAME_THAN_ELSE` as usual");
98+
} else {
99+
println!("This should trigger `IS_SAME_THAN_ELSE` as usual");
100+
}
101+
}
102+
83103
fn main() {}

‎tests/ui/shared_code_in_if_blocks/shared_at_top.stderr‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,5 +79,43 @@ LL | println!("I'm also moveable");
7979
LL | if x == 11 {
8080
|
8181

82-
error: aborting due to 5 previous errors
82+
error: All if blocks contain the same code at the start
83+
--> $DIR/shared_at_top.rs:88:5
84+
|
85+
LL | / if x == 2020 {
86+
LL | | println!("This should trigger the `SHARED_CODE_IN_IF_BLOCKS` lint.");
87+
LL | | println!("Because `IF_SAME_THEN_ELSE` is allowed here");
88+
| |________________________________________________________________^
89+
|
90+
help: Consider moving the start statements out like this
91+
|
92+
LL | println!("This should trigger the `SHARED_CODE_IN_IF_BLOCKS` lint.");
93+
LL | println!("Because `IF_SAME_THEN_ELSE` is allowed here");
94+
LL | if x == 2020 {
95+
|
96+
97+
error: this `if` has identical blocks
98+
--> $DIR/shared_at_top.rs:96:18
99+
|
100+
LL | if x == 2019 {
101+
| __________________^
102+
LL | | println!("This should trigger `IS_SAME_THAN_ELSE` as usual");
103+
LL | | } else {
104+
| |_____^
105+
|
106+
note: the lint level is defined here
107+
--> $DIR/shared_at_top.rs:2:9
108+
|
109+
LL | #![deny(clippy::if_same_then_else, clippy::shared_code_in_if_blocks)]
110+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
111+
note: same as this
112+
--> $DIR/shared_at_top.rs:98:12
113+
|
114+
LL | } else {
115+
| ____________^
116+
LL | | println!("This should trigger `IS_SAME_THAN_ELSE` as usual");
117+
LL | | }
118+
| |_____^
119+
120+
error: aborting due to 7 previous errors
83121

0 commit comments

Comments
 (0)