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

A new lint for shared code in if blocks

Browse Source 
* Added expression check for shared_code_in_if_blocks
* Finishing touches for the shared_code_in_if_blocks lint
* Applying PR suggestions
* Update lints yay
* Moved test into subfolder
This commit is contained in:
xFrednet 2020-12-11 22:29:53 +00:00
parent 232e2b79f2
commit d1df73228a
17 changed files with 780 additions and 207 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -2455,6 +2455,7 @@ Released 2018-09-13
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse
[`shadow_same`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_same
[`shadow_unrelated`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_unrelated
[`shared_code_in_if_blocks`]: https://rust-lang.github.io/rust-clippy/master/index.html#shared_code_in_if_blocks
[`short_circuit_statement`]: https://rust-lang.github.io/rust-clippy/master/index.html#short_circuit_statement
[`should_assert_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_assert_eq
[`should_implement_trait`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait

305
clippy_lints/src/copies.rs
View File

@ -3,7 +3,10 @@
use clippy_utils::{get_parent_expr, if_sequence};
use rustc_hir::{Block, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use std::borrow::Cow;
declare_clippy_lint! {
/// **What it does:** Checks for consecutive `if`s with the same condition.
@ -104,7 +107,45 @@
"`if` with the same `then` and `else` blocks"
}
declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, SAME_FUNCTIONS_IN_IF_CONDITION, IF_SAME_THEN_ELSE]);
declare_clippy_lint! {
/// **What it does:** Checks if the `if` and `else` block contain shared code that can be
/// moved out of the blocks.
///
/// **Why is this bad?** Duplicate code is less maintainable.
///
/// **Known problems:** Hopefully none.
///
/// **Example:**
/// ```ignore
/// let foo = if … {
/// println!("Hello World");
/// 13
/// } else {
/// println!("Hello World");
/// 42
/// };
/// ```
///
/// Could be written as:
/// ```ignore
/// println!("Hello World");
/// let foo = if … {
/// 13
/// } else {
/// 42
/// };
/// ```
pub SHARED_CODE_IN_IF_BLOCKS,
pedantic,
"`if` statement with shared code in all blocks"
}
declare_lint_pass!(CopyAndPaste => [
IFS_SAME_COND,
SAME_FUNCTIONS_IN_IF_CONDITION,
IF_SAME_THEN_ELSE,
SHARED_CODE_IN_IF_BLOCKS
]);
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
@ -121,27 +162,253 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
let (conds, blocks) = if_sequence(expr);
lint_same_then_else(cx, &blocks);
// Conditions
lint_same_cond(cx, &conds);
lint_same_fns_in_if_cond(cx, &conds);
// Block duplication
lint_same_then_else(cx, &blocks, conds.len() != blocks.len(), expr);
}
}
}
/// Implementation of `IF_SAME_THEN_ELSE`.
fn lint_same_then_else(cx: &LateContext<'_>, blocks: &[&Block<'_>]) {
let eq: &dyn Fn(&&Block<'_>, &&Block<'_>) -> bool =
&|&lhs, &rhs| -> bool { SpanlessEq::new(cx).eq_block(lhs, rhs) };
/// Implementation of `SHARED_CODE_IN_IF_BLOCKS` and `IF_SAME_THEN_ELSE` if the blocks are equal.
fn lint_same_then_else<'tcx>(
cx: &LateContext<'tcx>,
blocks: &[&Block<'tcx>],
has_unconditional_else: bool,
expr: &'tcx Expr<'_>,
) {
// We only lint ifs with multiple blocks
// TODO xFrednet 2021-01-01: Check if it's an else if block
if blocks.len() < 2 {
return;
}
if let Some((i, j)) = search_same_sequenced(blocks, eq) {
span_lint_and_note(
let has_expr = blocks[0].expr.is_some();
// Check if each block has shared code
let mut start_eq = usize::MAX;
let mut end_eq = usize::MAX;
let mut expr_eq = true;
for (index, win) in blocks.windows(2).enumerate() {
let l_stmts = win[0].stmts;
let r_stmts = win[1].stmts;
let mut evaluator = SpanlessEq::new(cx);
let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));
let current_end_eq = count_eq(&mut l_stmts.iter().rev(), &mut r_stmts.iter().rev(), |l, r| {
evaluator.eq_stmt(l, r)
});
let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
// IF_SAME_THEN_ELSE
// We only lint the first two blocks (index == 0). Further blocks will be linted when that if
// statement is checked
if index == 0 && block_expr_eq && l_stmts.len() == r_stmts.len() && l_stmts.len() == current_start_eq {
span_lint_and_note(
cx,
IF_SAME_THEN_ELSE,
win[0].span,
"this `if` has identical blocks",
Some(win[1].span),
"same as this",
);
return;
}
start_eq = start_eq.min(current_start_eq);
end_eq = end_eq.min(current_end_eq);
expr_eq &= block_expr_eq;
// We can return if the eq count is 0 from both sides or if it has no unconditional else case
if !has_unconditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
return;
}
}
if has_expr && !expr_eq {
end_eq = 0;
}
// Check if the regions are overlapping. Set `end_eq` to prevent the overlap
let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap();
if (start_eq + end_eq) > min_block_size {
end_eq = min_block_size - start_eq;
}
// Only the start is the same
if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) {
emit_shared_code_in_if_blocks_lint(cx, start_eq, 0, false, blocks, expr);
} else if end_eq != 0 && (!has_expr || !expr_eq) {
let block = blocks[blocks.len() - 1];
let stmts = block.stmts.split_at(start_eq).1;
let (block_stmts, moved_stmts) = stmts.split_at(stmts.len() - end_eq);
// Scan block
let mut walker = SymbolFinderVisitor::new(cx);
for stmt in block_stmts {
intravisit::walk_stmt(&mut walker, stmt);
}
let mut block_defs = walker.defs;
// Scan moved stmts
let mut moved_start: Option<usize> = None;
let mut walker = SymbolFinderVisitor::new(cx);
for (index, stmt) in moved_stmts.iter().enumerate() {
intravisit::walk_stmt(&mut walker, stmt);
for value in &walker.uses {
// Well we can't move this and all prev statements. So reset
if block_defs.contains(&value) {
moved_start = Some(index + 1);
walker.defs.drain().for_each(|x| {
block_defs.insert(x);
});
}
}
walker.uses.clear();
}
if let Some(moved_start) = moved_start {
end_eq -= moved_start;
}
let mut end_linable = true;
if let Some(expr) = block.expr {
intravisit::walk_expr(&mut walker, expr);
end_linable = walker.uses.iter().any(|x| !block_defs.contains(x));
}
emit_shared_code_in_if_blocks_lint(cx, start_eq, end_eq, end_linable, blocks, expr);
}
}
fn emit_shared_code_in_if_blocks_lint(
cx: &LateContext<'tcx>,
start_stmts: usize,
end_stmts: usize,
lint_end: bool,
blocks: &[&Block<'tcx>],
if_expr: &'tcx Expr<'_>,
) {
if start_stmts == 0 && !lint_end {
return;
}
// (help, span, suggestion)
let mut suggestions: Vec<(&str, Span, String)> = vec![];
if start_stmts > 0 {
let block = blocks[0];
let span_start = first_line_of_span(cx, if_expr.span).shrink_to_lo();
let span_end = block.stmts[start_stmts - 1].span.source_callsite();
let cond_span = first_line_of_span(cx, if_expr.span).until(block.span);
let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
let cond_indent = indent_of(cx, cond_span);
let moved_span = block.stmts[0].span.source_callsite().to(span_end);
let moved_snippet = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
let span = span_start.to(span_end);
suggestions.push(("START HELP", span, suggestion.to_string()));
}
if lint_end {
let block = blocks[blocks.len() - 1];
let span_end = block.span.shrink_to_hi();
let moved_start = if end_stmts == 0 && block.expr.is_some() {
block.expr.unwrap().span
} else {
block.stmts[block.stmts.len() - end_stmts].span
}
.source_callsite();
let moved_end = if let Some(expr) = block.expr {
expr.span
} else {
block.stmts[block.stmts.len() - 1].span
}
.source_callsite();
let moved_span = moved_start.to(moved_end);
let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
let indent = indent_of(cx, if_expr.span.shrink_to_hi());
let suggestion = "}\n".to_string() + &moved_snipped;
let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
let span = moved_start.to(span_end);
suggestions.push(("END_RANGE", span, suggestion.to_string()));
}
if suggestions.len() == 1 {
let (_, span, sugg) = &suggestions[0];
span_lint_and_sugg(
cx,
IF_SAME_THEN_ELSE,
j.span,
"this `if` has identical blocks",
Some(i.span),
"same as this",
SHARED_CODE_IN_IF_BLOCKS,
*span,
"All code blocks contain the same code",
"Consider moving the code out like this",
sugg.clone(),
Applicability::Unspecified,
);
} else {
span_lint_and_then(
cx,
SHARED_CODE_IN_IF_BLOCKS,
if_expr.span,
"All if blocks contain the same code",
move |diag| {
for (help, span, sugg) in suggestions {
diag.span_suggestion(span, help, sugg, Applicability::Unspecified);
}
},
);
}
}
pub struct SymbolFinderVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
defs: FxHashSet<HirId>,
uses: FxHashSet<HirId>,
}
impl<'a, 'tcx> SymbolFinderVisitor<'a, 'tcx> {
fn new(cx: &'a LateContext<'tcx>) -> Self {
SymbolFinderVisitor {
cx,
defs: FxHashSet::default(),
uses: FxHashSet::default(),
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for SymbolFinderVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::All(self.cx.tcx.hir())
}
fn visit_local(&mut self, l: &'tcx rustc_hir::Local<'tcx>) {
let local_id = l.pat.hir_id;
self.defs.insert(local_id);
if let Some(expr) = l.init {
intravisit::walk_expr(self, expr);
}
}
fn visit_qpath(&mut self, qpath: &'tcx rustc_hir::QPath<'tcx>, id: HirId, _span: rustc_span::Span) {
if let rustc_hir::QPath::Resolved(_, ref path) = *qpath {
if path.segments.len() == 1 {
if let rustc_hir::def::Res::Local(var) = self.cx.qpath_res(qpath, id) {
self.uses.insert(var);
}
}
}
}
}
@ -198,15 +465,3 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
);
}
}
fn search_same_sequenced<T, Eq>(exprs: &[T], eq: Eq) -> Option<(&T, &T)>
where
Eq: Fn(&T, &T) -> bool,
{
for win in exprs.windows(2) {
if eq(&win[0], &win[1]) {
return Some((&win[0], &win[1]));
}
}
None
}

2
clippy_lints/src/lib.rs
View File

@ -616,6 +616,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&copies::IFS_SAME_COND,
&copies::IF_SAME_THEN_ELSE,
&copies::SAME_FUNCTIONS_IN_IF_CONDITION,
&copies::SHARED_CODE_IN_IF_BLOCKS,
&copy_iterator::COPY_ITERATOR,
&create_dir::CREATE_DIR,
&dbg_macro::DBG_MACRO,
@ -1365,6 +1366,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&casts::PTR_AS_PTR),
LintId::of(&checked_conversions::CHECKED_CONVERSIONS),
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
LintId::of(&copies::SHARED_CODE_IN_IF_BLOCKS),
LintId::of(&copy_iterator::COPY_ITERATOR),
LintId::of(&default::DEFAULT_TRAIT_ACCESS),
LintId::of(&dereference::EXPLICIT_DEREF_METHODS),

40
clippy_lints/src/literal_representation.rs
View File

@ -464,32 +464,28 @@ fn do_lint(digits: &str) -> Result<(), WarningType> {
{
return Err(WarningType::DecimalRepresentation);
}
} else if digits.len() < 4 {
// Lint for Literals with a hex-representation of 2 or 3 digits
let f = &digits[0..1]; // first digit
let s = &digits[1..]; // suffix
// Powers of 2
if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && s.chars().all(|c| c == '0'))
// Powers of 2 minus 1
|| ((f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && s.chars().all(|c| c == 'F'))
{
return Err(WarningType::DecimalRepresentation);
}
} else {
// Lint for Literals with a hex-representation of 4 digits or more
let f = &digits[0..1]; // first digit
let m = &digits[1..digits.len() - 1]; // middle digits, except last
let s = &digits[1..]; // suffix
// Powers of 2 with a margin of +15/-16
if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && m.chars().all(|c| c == '0'))
|| ((f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && m.chars().all(|c| c == 'F'))
// Lint for representations with only 0s and Fs, while allowing 7 as the first
// digit
|| ((f.eq("7") || f.eq("F")) && s.chars().all(|c| c == '0' || c == 'F'))
{
return Err(WarningType::DecimalRepresentation);
if digits.len() < 4 {
// Powers of 2
if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && s.chars().all(|c| c == '0'))
// Powers of 2 minus 1
|| ((f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && s.chars().all(|c| c == 'F'))
{
return Err(WarningType::DecimalRepresentation);
}
} else {
// Powers of 2 with a margin of +15/-16
if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && m.chars().all(|c| c == '0'))
|| ((f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && m.chars().all(|c| c == 'F'))
// Lint for representations with only 0s and Fs, while allowing 7 as the first
// digit
|| ((f.eq("7") || f.eq("F")) && s.chars().all(|c| c == '0' || c == 'F'))
{
return Err(WarningType::DecimalRepresentation);
}
}
}

48
clippy_lints/src/methods/manual_saturating_arithmetic.rs
View File

@ -44,44 +44,28 @@ pub fn check(
// "mul" is omitted because lhs can be negative.
_ => return,
}
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
super::MANUAL_SATURATING_ARITHMETIC,
expr.span,
"manual saturating arithmetic",
&format!("try using `saturating_{}`", arith),
format!(
"{}.saturating_{}({})",
snippet_with_applicability(cx, arith_lhs.span, "..", &mut applicability),
arith,
snippet_with_applicability(cx, arith_rhs.span, "..", &mut applicability),
),
applicability,
);
} else {
match (mm, arith) {
(MinMax::Max, "add" | "mul") | (MinMax::Min, "sub") => (),
_ => return,
}
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
super::MANUAL_SATURATING_ARITHMETIC,
expr.span,
"manual saturating arithmetic",
&format!("try using `saturating_{}`", arith),
format!(
"{}.saturating_{}({})",
snippet_with_applicability(cx, arith_lhs.span, "..", &mut applicability),
arith,
snippet_with_applicability(cx, arith_rhs.span, "..", &mut applicability),
),
applicability,
);
}
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
super::MANUAL_SATURATING_ARITHMETIC,
expr.span,
"manual saturating arithmetic",
&format!("try using `saturating_{}`", arith),
format!(
"{}.saturating_{}({})",
snippet_with_applicability(cx, arith_lhs.span, "..", &mut applicability),
arith,
snippet_with_applicability(cx, arith_rhs.span, "..", &mut applicability),
),
applicability,
);
}
#[derive(PartialEq, Eq)]

9
clippy_utils/src/hir_utils.rs
View File

@ -483,6 +483,15 @@ pub fn over<X>(left: &[X], right: &[X], mut eq_fn: impl FnMut(&X, &X) -> bool) -
left.len() == right.len() && left.iter().zip(right).all(|(x, y)| eq_fn(x, y))
}
/// Counts how many elements of the slices are equal as per `eq_fn`.
pub fn count_eq<X: Sized>(
left: &mut dyn Iterator<Item = X>,
right: &mut dyn Iterator<Item = X>,
mut eq_fn: impl FnMut(&X, &X) -> bool,
) -> usize {
left.zip(right).take_while(|(l, r)| eq_fn(l, r)).count()
}
/// Checks if two expressions evaluate to the same value, and don't contain any side effects.
pub fn eq_expr_value(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>) -> bool {
SpanlessEq::new(cx).deny_side_effects().eq_expr(left, right)

2
tests/ui/checked_unwrap/complex_conditionals.rs
View File

@ -1,5 +1,5 @@
#![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
#![allow(clippy::if_same_then_else)]
#![allow(clippy::if_same_then_else, clippy::shared_code_in_if_blocks)]
fn test_complex_conditions() {
let x: Result<(), ()> = Ok(());

3
tests/ui/if_same_then_else.rs
View File

@ -5,7 +5,8 @@
clippy::never_loop,
clippy::no_effect,
clippy::unused_unit,
clippy::zero_divided_by_zero
clippy::zero_divided_by_zero,
clippy::shared_code_in_if_blocks
)]
struct Foo {

154
tests/ui/if_same_then_else.stderr
View File

@ -1,19 +1,5 @@
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:28:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | Foo { bar: 42 };
LL | | 0..10;
... |
LL | | foo();
LL | | }
| |_____^
|
= note: `-D clippy::if-same-then-else` implied by `-D warnings`
note: same as this
--> $DIR/if_same_then_else.rs:20:13
--> $DIR/if_same_then_else.rs:21:13
|
LL | if true {
| _____________^
@ -24,79 +10,80 @@ LL | | ..;
LL | | foo();
LL | | } else {
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:66:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | 0.0
LL | | };
| |_____^
|
= note: `-D clippy::if-same-then-else` implied by `-D warnings`
note: same as this
--> $DIR/if_same_then_else.rs:64:21
|
LL | let _ = if true {
| _____________________^
LL | | 0.0
LL | | } else {
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:73:12
--> $DIR/if_same_then_else.rs:29:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | -0.0
LL | | };
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:71:21
|
LL | let _ = if true {
| _____________________^
LL | | -0.0
LL | | } else {
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:89:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | 42
LL | | };
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:87:21
|
LL | let _ = if true {
| _____________________^
LL | | 42
LL | | } else {
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:101:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | let bar = if true { 42 } else { 43 };
LL | |
LL | | Foo { bar: 42 };
LL | | 0..10;
... |
LL | | bar + 1;
LL | | foo();
LL | | }
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:65:21
|
LL | let _ = if true {
| _____________________^
LL | | 0.0
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:94:13
--> $DIR/if_same_then_else.rs:67:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | 0.0
LL | | };
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:72:21
|
LL | let _ = if true {
| _____________________^
LL | | -0.0
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:74:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | -0.0
LL | | };
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:88:21
|
LL | let _ = if true {
| _____________________^
LL | | 42
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:90:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | 42
LL | | };
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else.rs:95:13
|
LL | if true {
| _____________^
@ -107,6 +94,19 @@ LL | | while foo() {
LL | | bar + 1;
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else.rs:102:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | let bar = if true { 42 } else { 43 };
LL | |
... |
LL | | bar + 1;
LL | | }
| |_____^
error: aborting due to 5 previous errors

3
tests/ui/if_same_then_else2.rs
View File

@ -5,7 +5,8 @@
clippy::collapsible_if,
clippy::ifs_same_cond,
clippy::needless_return,
clippy::single_element_loop
clippy::single_element_loop,
clippy::shared_code_in_if_blocks
)]
fn if_same_then_else2() -> Result<&'static str, ()> {

100
tests/ui/if_same_then_else2.stderr
View File

@ -24,8 +24,31 @@ LL | | if foo.is_some() {
LL | | }
LL | | } else {
| |_____^
|
= note: `-D clippy::if-same-then-else` implied by `-D warnings`
note: same as this
--> $DIR/if_same_then_else2.rs:21:12
|
LL | } else {
| ____________^
LL | | //~ ERROR same body as `if` block
LL | | for _ in &[42] {
LL | | let foo: &Option<_> = &Some::<u8>(42);
... |
LL | | }
LL | | }
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:33:13
|
LL | if true {
| _____________^
LL | | if let Some(a) = Some(42) {}
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:35:12
|
LL | } else {
@ -34,17 +57,17 @@ LL | | //~ ERROR same body as `if` block
LL | | if let Some(a) = Some(42) {}
LL | | }
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:33:13
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:40:13
|
LL | if true {
| _____________^
LL | | if let Some(a) = Some(42) {}
LL | | if let (1, .., 3) = (1, 2, 3) {}
LL | | } else {
| |_____^
error: this `if` has identical blocks
|
note: same as this
--> $DIR/if_same_then_else2.rs:42:12
|
LL | } else {
@ -53,17 +76,17 @@ LL | | //~ ERROR same body as `if` block
LL | | if let (1, .., 3) = (1, 2, 3) {}
LL | | }
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:40:13
|
LL | if true {
| _____________^
LL | | if let (1, .., 3) = (1, 2, 3) {}
LL | | } else {
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:90:21
|
LL | let _ = if true {
| _____________________^
LL | | f32::NAN
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:92:12
|
LL | } else {
@ -72,17 +95,17 @@ LL | | //~ ERROR same body as `if` block
LL | | f32::NAN
LL | | };
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:90:21
|
LL | let _ = if true {
| _____________________^
LL | | f32::NAN
LL | | } else {
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:97:13
|
LL | if true {
| _____________^
LL | | Ok("foo")?;
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:99:12
|
LL | } else {
@ -91,27 +114,8 @@ LL | | //~ ERROR same body as `if` block
LL | | Ok("foo")?;
LL | | }
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:97:13
|
LL | if true {
| _____________^
LL | | Ok("foo")?;
LL | | } else {
| |_____^
error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:124:12
|
LL | } else {
| ____________^
LL | | let foo = "";
LL | | return Ok(&foo[0..]);
LL | | }
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:121:20
|
LL | } else if true {
@ -120,6 +124,16 @@ LL | | let foo = "";
LL | | return Ok(&foo[0..]);
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:124:12
|
LL | } else {
| ____________^
LL | | let foo = "";
LL | | return Ok(&foo[0..]);
LL | | }
| |_____^
error: aborting due to 6 previous errors

3
tests/ui/let_if_seq.rs
View File

@ -2,7 +2,8 @@
unused_variables,
unused_assignments,
clippy::similar_names,
clippy::blacklisted_name
clippy::blacklisted_name,
clippy::shared_code_in_if_blocks
)]
#![warn(clippy::useless_let_if_seq)]

8
tests/ui/let_if_seq.stderr
View File

@ -1,5 +1,5 @@
error: `if _ { .. } else { .. }` is an expression
--> $DIR/let_if_seq.rs:64:5
--> $DIR/let_if_seq.rs:65:5
|
LL | / let mut foo = 0;
LL | | if f() {
@ -11,7 +11,7 @@ LL | | }
= note: you might not need `mut` at all
error: `if _ { .. } else { .. }` is an expression
--> $DIR/let_if_seq.rs:69:5
--> $DIR/let_if_seq.rs:70:5
|
LL | / let mut bar = 0;
LL | | if f() {
@ -25,7 +25,7 @@ LL | | }
= note: you might not need `mut` at all
error: `if _ { .. } else { .. }` is an expression
--> $DIR/let_if_seq.rs:77:5
--> $DIR/let_if_seq.rs:78:5
|
LL | / let quz;
LL | | if f() {
@ -36,7 +36,7 @@ LL | | }
| |_____^ help: it is more idiomatic to write: `let quz = if f() { 42 } else { 0 };`
error: `if _ { .. } else { .. }` is an expression
--> $DIR/let_if_seq.rs:106:5
--> $DIR/let_if_seq.rs:107:5
|
LL | / let mut baz = 0;
LL | | if f() {

47
tests/ui/shared_code_in_if_blocks/shared_at_bot.rs Normal file
View File

@ -0,0 +1,47 @@
#![allow(dead_code)]
#![deny(clippy::if_same_then_else, clippy::shared_code_in_if_blocks)]
// This tests the shared_code_in_if_blocks lint at the end of blocks
fn simple_examples() {
// TODO xFrednet 2021-01-06: Test with const literals at the end
let x = 1;
let _ = if x == 7 {
println!("Branch I");
let start_value = 0;
println!("=^.^=");
// Same but not moveable due to `start_value`
let _ = start_value;
// The rest is self contained and moveable => Only lint the rest
let result = false;
println!("Block end!");
result
} else {
println!("Branch II");
let start_value = 8;
println!("xD");
// Same but not moveable due to `start_value`
let _ = start_value;
// The rest is self contained and moveable => Only lint the rest
let result = false;
println!("Block end!");
result
};
}
/// Simple examples where the move can cause some problems due to moved values
fn simple_but_suggestion_is_invalid() {
// TODO xFrednet 2021-01-12: This
}
/// Tests where the blocks are not linted due to the used value scope
fn not_moveable_due_to_value_scope() {
// TODO xFrednet 2021-01-12: This
}
fn main() {}

83
tests/ui/shared_code_in_if_blocks/shared_at_top.rs Normal file
View File

@ -0,0 +1,83 @@
#![allow(dead_code, clippy::eval_order_dependence)]
#![deny(clippy::if_same_then_else, clippy::shared_code_in_if_blocks)]
// This tests the shared_code_in_if_blocks lint at the start of blocks
fn simple_examples() {
let x = 0;
// Simple
if true {
println!("Hello World!");
println!("I'm branch nr: 1");
} else {
println!("Hello World!");
println!("I'm branch nr: 2");
}
// Else if
if x == 0 {
let y = 9;
println!("The value y was set to: `{}`", y);
let _z = y;
println!("I'm the true start index of arrays");
} else if x == 1 {
let y = 9;
println!("The value y was set to: `{}`", y);
let _z = y;
println!("I start counting from 1 so my array starts from `1`");
} else {
let y = 9;
println!("The value y was set to: `{}`", y);
let _z = y;
println!("Ha, Pascal allows you to start the array where you want")
}
// Return a value
let _ = if x == 7 {
let y = 16;
println!("What can I say except: \"you're welcome?\"");
let _ = y;
x
} else {
let y = 16;
println!("Thank you");
y
};
}
/// Simple examples where the move can cause some problems due to moved values
fn simple_but_suggestion_is_invalid() {
let x = 10;
// Can't be automatically moved because used_value_name is getting used again
let used_value_name = 19;
if x == 10 {
let used_value_name = "Different type";
println!("Str: {}", used_value_name);
let _ = 1;
} else {
let used_value_name = "Different type";
println!("Str: {}", used_value_name);
let _ = 2;
}
let _ = used_value_name;
// This can be automatically moved as `can_be_overridden` is not used again
let can_be_overridden = 8;
let _ = can_be_overridden;
if x == 11 {
let can_be_overridden = "Move me";
println!("I'm also moveable");
let _ = 111;
} else {
let can_be_overridden = "Move me";
println!("I'm also moveable");
let _ = 222;
}
}
fn main() {}

22
tests/ui/shared_code_in_if_blocks/shared_at_top_and_bot.rs Normal file
View File

@ -0,0 +1,22 @@
#![allow(dead_code)]
#![deny(clippy::if_same_then_else, clippy::shared_code_in_if_blocks)]
// shared_code_in_if_blocks at the top and bottom of the if blocks
fn main() {
// TODO xFrednet 2021-01-12: This
}
// General TODOs By xFrednet:
//
// * Make a test with overlapping eq regions (else ifs)
// * Test if as function parameter, tuple constructor, index, while loop condition
// * Test where only the expression is the same
// * Test where the block only has an expression
// * Test with let on a previous line let _ = \n if...
// * Tests with unreadable formatting (Inline if, Not indented)
// * Test multiline condition if x == 9 \n x == 8 {}
// * Test if for return/break (Only move to front)
// * Test in inline closures
// * Test with structs and tuples

157
tests/ui/shared_code_in_if_blocks/valid_if_blocks.rs Normal file
View File

@ -0,0 +1,157 @@
#![allow(dead_code, clippy::eval_order_dependence)]
#![deny(clippy::if_same_then_else, clippy::shared_code_in_if_blocks)]
// This tests the shared_code_in_if_blocks lint at the start of blocks
// Tests with value references are includes in "shared_code_at_bot.rs"
fn valid_examples() {
let x = 2;
// The edge statements are different
if x == 9 {
let y = 1 << 5;
println!("This is the same: vvv");
let _z = y;
println!("The block expression is different");
println!("Different end 1");
} else {
let y = 1 << 7;
println!("This is the same: vvv");
let _z = y;
println!("The block expression is different");
println!("Different end 2");
}
// No else
if x == 2 {
println!("Hello world!");
println!("Hello back, how are you?");
// This is different vvvv
println!("Howdy stranger =^.^=");
println!("Bye Bye World");
} else if x == 9 {
println!("Hello world!");
println!("Hello back, how are you?");
// This is different vvvv
println!("Hello reviewer :D");
println!("Bye Bye World");
}
// Overlapping statements only in else if blocks -> Don't lint
if x == 0 {
println!("I'm important!")
} else if x == 17 {
println!("I share code in else if");
println!("x is 17");
} else {
println!("I share code in else if");
println!("x is nether x nor 17");
}
// Mutability is different
if x == 13 {
let mut y = 9;
println!("Value y is: {}", y);
y += 16;
let _z1 = y;
} else {
let y = 9;
println!("Value y is: {}", y);
let _z2 = y;
}
// Same blocks but at start and bottom so no `if_same_then_else` lint
if x == 418 {
let y = 9;
let z = 8;
let _ = (x, y, z);
// Don't tell the programmer, my code is also in the else block
} else if x == 419 {
println!("+-----------+");
println!("| |");
println!("| O O |");
println!("| ° |");
println!("| \\_____/ |");
println!("| |");
println!("+-----------+");
} else {
let y = 9;
let z = 8;
let _ = (x, y, z);
// I'm so much better than the x == 418 block. Trust me
}
}
/// This makes sure that the `if_same_then_else` masks the `shared_code_in_if_blocks` lint
fn trigger_other_lint() {
let x = 0;
let y = 1;
// Same block
if x == 0 {
let u = 19;
println!("How are u today?");
let _ = "This is a string";
} else {
let u = 19;
println!("How are u today?");
let _ = "This is a string";
}
// More complex same blocks
if x == 17 {
#[derive(Debug)]
struct Duck {
num: u64,
};
let pet = Duck { num: 18 };
println!("{:?}", pet);
} else {
#[derive(Debug)]
struct Duck {
num: u64,
};
let pet = Duck { num: 18 };
println!("{:?}", pet);
}
// Only same expression
let _ = if x == 6 { 7 } else { 7 };
// Same in else if block
let _ = if x == 67 {
println!("Well I'm the most important block");
"I'm a pretty string"
} else if x == 68 {
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"
}
};
}
fn main() {}