2021-04-08 10:50:13 -05:00
|
|
|
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
|
|
|
|
use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
|
|
|
|
use clippy_utils::{
|
2021-04-22 04:31:13 -05:00
|
|
|
both, count_eq, eq_expr_value, get_enclosing_block, get_parent_expr, if_sequence, in_macro, is_else_clause,
|
2021-07-15 03:44:10 -05:00
|
|
|
is_lint_allowed, search_same, ContainsName, SpanlessEq, SpanlessHash,
|
2021-04-08 10:50:13 -05:00
|
|
|
};
|
|
|
|
use if_chain::if_chain;
|
|
|
|
use rustc_data_structures::fx::FxHashSet;
|
|
|
|
use rustc_errors::{Applicability, DiagnosticBuilder};
|
|
|
|
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
|
|
|
|
use rustc_hir::{Block, Expr, ExprKind, HirId};
|
2020-01-12 00:08:41 -06:00
|
|
|
use rustc_lint::{LateContext, LateLintPass};
|
2021-04-08 10:50:13 -05:00
|
|
|
use rustc_middle::hir::map::Map;
|
2020-01-11 05:37:08 -06:00
|
|
|
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
2021-04-08 10:50:13 -05:00
|
|
|
use rustc_span::{source_map::Span, symbol::Symbol, BytePos};
|
|
|
|
use std::borrow::Cow;
|
2016-01-30 11:03:53 -06:00
|
|
|
|
2018-03-28 08:24:26 -05:00
|
|
|
declare_clippy_lint! {
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### What it does
|
|
|
|
/// Checks for consecutive `if`s with the same condition.
|
2019-03-05 10:50:33 -06:00
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Why is this bad?
|
|
|
|
/// This is probably a copy & paste error.
|
2019-03-05 10:50:33 -06:00
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Example
|
2019-03-05 16:23:50 -06:00
|
|
|
/// ```ignore
|
2019-03-05 10:50:33 -06:00
|
|
|
/// if a == b {
|
|
|
|
/// …
|
|
|
|
/// } else if a == b {
|
|
|
|
/// …
|
|
|
|
/// }
|
|
|
|
/// ```
|
|
|
|
///
|
|
|
|
/// Note that this lint ignores all conditions with a function call as it could
|
|
|
|
/// have side effects:
|
|
|
|
///
|
2019-03-05 16:23:50 -06:00
|
|
|
/// ```ignore
|
2019-03-05 10:50:33 -06:00
|
|
|
/// if foo() {
|
|
|
|
/// …
|
|
|
|
/// } else if foo() { // not linted
|
|
|
|
/// …
|
|
|
|
/// }
|
|
|
|
/// ```
|
2016-01-30 11:03:53 -06:00
|
|
|
pub IFS_SAME_COND,
|
2018-03-28 08:24:26 -05:00
|
|
|
correctness,
|
2020-01-06 00:30:43 -06:00
|
|
|
"consecutive `if`s with the same condition"
|
2016-01-30 11:03:53 -06:00
|
|
|
}
|
|
|
|
|
2019-11-13 23:06:34 -06:00
|
|
|
declare_clippy_lint! {
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### What it does
|
|
|
|
/// Checks for consecutive `if`s with the same function call.
|
2019-11-13 23:06:34 -06:00
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Why is this bad?
|
|
|
|
/// This is probably a copy & paste error.
|
2019-11-13 23:06:34 -06:00
|
|
|
/// Despite the fact that function can have side effects and `if` works as
|
|
|
|
/// intended, such an approach is implicit and can be considered a "code smell".
|
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Example
|
2019-11-13 23:06:34 -06:00
|
|
|
/// ```ignore
|
|
|
|
/// if foo() == bar {
|
|
|
|
/// …
|
|
|
|
/// } else if foo() == bar {
|
|
|
|
/// …
|
|
|
|
/// }
|
|
|
|
/// ```
|
|
|
|
///
|
|
|
|
/// This probably should be:
|
|
|
|
/// ```ignore
|
|
|
|
/// if foo() == bar {
|
|
|
|
/// …
|
|
|
|
/// } else if foo() == baz {
|
|
|
|
/// …
|
|
|
|
/// }
|
|
|
|
/// ```
|
|
|
|
///
|
|
|
|
/// or if the original code was not a typo and called function mutates a state,
|
|
|
|
/// consider move the mutation out of the `if` condition to avoid similarity to
|
|
|
|
/// a copy & paste error:
|
|
|
|
///
|
|
|
|
/// ```ignore
|
|
|
|
/// let first = foo();
|
|
|
|
/// if first == bar {
|
|
|
|
/// …
|
|
|
|
/// } else {
|
|
|
|
/// let second = foo();
|
|
|
|
/// if second == bar {
|
|
|
|
/// …
|
|
|
|
/// }
|
|
|
|
/// }
|
|
|
|
/// ```
|
|
|
|
pub SAME_FUNCTIONS_IN_IF_CONDITION,
|
|
|
|
pedantic,
|
2020-01-06 00:30:43 -06:00
|
|
|
"consecutive `if`s with the same function call"
|
2019-11-13 23:06:34 -06:00
|
|
|
}
|
|
|
|
|
2018-03-28 08:24:26 -05:00
|
|
|
declare_clippy_lint! {
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### What it does
|
|
|
|
/// Checks for `if/else` with the same body as the *then* part
|
2019-03-05 10:50:33 -06:00
|
|
|
/// and the *else* part.
|
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Why is this bad?
|
|
|
|
/// This is probably a copy & paste error.
|
2019-03-05 10:50:33 -06:00
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Example
|
2019-03-05 16:23:50 -06:00
|
|
|
/// ```ignore
|
2019-03-05 10:50:33 -06:00
|
|
|
/// let foo = if … {
|
|
|
|
/// 42
|
|
|
|
/// } else {
|
|
|
|
/// 42
|
|
|
|
/// };
|
|
|
|
/// ```
|
2016-01-30 12:16:49 -06:00
|
|
|
pub IF_SAME_THEN_ELSE,
|
2018-03-28 08:24:26 -05:00
|
|
|
correctness,
|
2020-01-06 00:30:43 -06:00
|
|
|
"`if` with the same `then` and `else` blocks"
|
2016-01-30 12:16:49 -06:00
|
|
|
}
|
|
|
|
|
2021-04-08 10:50:13 -05:00
|
|
|
declare_clippy_lint! {
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### What it does
|
|
|
|
/// Checks if the `if` and `else` block contain shared code that can be
|
2021-04-08 10:50:13 -05:00
|
|
|
/// moved out of the blocks.
|
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Why is this bad?
|
|
|
|
/// Duplicate code is less maintainable.
|
2021-04-08 10:50:13 -05:00
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Known problems
|
2021-07-15 03:44:10 -05:00
|
|
|
/// * The lint doesn't check if the moved expressions modify values that are beeing used in
|
|
|
|
/// the if condition. The suggestion can in that case modify the behavior of the program.
|
|
|
|
/// See [rust-clippy#7452](https://github.com/rust-lang/rust-clippy/issues/7452)
|
2021-04-08 10:50:13 -05:00
|
|
|
///
|
2021-07-29 05:16:06 -05:00
|
|
|
/// ### Example
|
2021-04-08 10:50:13 -05:00
|
|
|
/// ```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 BRANCHES_SHARING_CODE,
|
2021-09-08 09:31:47 -05:00
|
|
|
nursery,
|
2021-04-08 10:50:13 -05:00
|
|
|
"`if` statement with shared code in all blocks"
|
|
|
|
}
|
|
|
|
|
|
|
|
declare_lint_pass!(CopyAndPaste => [
|
|
|
|
IFS_SAME_COND,
|
|
|
|
SAME_FUNCTIONS_IN_IF_CONDITION,
|
|
|
|
IF_SAME_THEN_ELSE,
|
|
|
|
BRANCHES_SHARING_CODE
|
|
|
|
]);
|
2016-01-30 11:03:53 -06:00
|
|
|
|
2020-06-25 15:41:36 -05:00
|
|
|
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
|
|
|
|
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
2019-08-19 11:30:32 -05:00
|
|
|
if !expr.span.from_expansion() {
|
2021-04-08 10:50:13 -05:00
|
|
|
if let ExprKind::If(_, _, _) = expr.kind {
|
|
|
|
// skip ifs directly in else, it will be checked in the parent if
|
|
|
|
if let Some(&Expr {
|
|
|
|
kind: ExprKind::If(_, _, Some(else_expr)),
|
|
|
|
..
|
|
|
|
}) = get_parent_expr(cx, expr)
|
|
|
|
{
|
|
|
|
if else_expr.hir_id == expr.hir_id {
|
|
|
|
return;
|
|
|
|
}
|
2016-02-09 08:18:27 -06:00
|
|
|
}
|
2021-04-08 10:50:13 -05:00
|
|
|
|
|
|
|
let (conds, blocks) = if_sequence(expr);
|
|
|
|
// 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);
|
2016-02-09 08:18:27 -06:00
|
|
|
}
|
2021-04-08 10:50:13 -05:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2016-02-09 08:18:27 -06:00
|
|
|
|
2021-04-08 10:50:13 -05:00
|
|
|
/// Implementation of `BRANCHES_SHARING_CODE` and `IF_SAME_THEN_ELSE` if the blocks are equal.
|
|
|
|
fn lint_same_then_else<'tcx>(
|
|
|
|
cx: &LateContext<'tcx>,
|
|
|
|
blocks: &[&Block<'tcx>],
|
|
|
|
has_conditional_else: bool,
|
|
|
|
expr: &'tcx Expr<'_>,
|
|
|
|
) {
|
|
|
|
// We only lint ifs with multiple blocks
|
2021-04-22 04:31:13 -05:00
|
|
|
if blocks.len() < 2 || is_else_clause(cx.tcx, expr) {
|
2021-04-08 10:50:13 -05:00
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
// Check if each block has shared code
|
|
|
|
let has_expr = blocks[0].expr.is_some();
|
2021-04-22 04:31:13 -05:00
|
|
|
|
|
|
|
let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, blocks) {
|
|
|
|
(block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq)
|
|
|
|
} else {
|
|
|
|
return;
|
|
|
|
};
|
2021-04-08 10:50:13 -05:00
|
|
|
|
|
|
|
// BRANCHES_SHARING_CODE prerequisites
|
|
|
|
if has_conditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
// Only the start is the same
|
|
|
|
if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) {
|
|
|
|
let block = blocks[0];
|
|
|
|
let start_stmts = block.stmts.split_at(start_eq).0;
|
|
|
|
|
|
|
|
let mut start_walker = UsedValueFinderVisitor::new(cx);
|
|
|
|
for stmt in start_stmts {
|
|
|
|
intravisit::walk_stmt(&mut start_walker, stmt);
|
2016-01-30 11:03:53 -06:00
|
|
|
}
|
2021-04-08 10:50:13 -05:00
|
|
|
|
|
|
|
emit_branches_sharing_code_lint(
|
|
|
|
cx,
|
|
|
|
start_eq,
|
|
|
|
0,
|
|
|
|
false,
|
|
|
|
check_for_warn_of_moved_symbol(cx, &start_walker.def_symbols, expr),
|
|
|
|
blocks,
|
|
|
|
expr,
|
|
|
|
);
|
|
|
|
} else if end_eq != 0 || (has_expr && expr_eq) {
|
|
|
|
let block = blocks[blocks.len() - 1];
|
|
|
|
let (start_stmts, block_stmts) = block.stmts.split_at(start_eq);
|
|
|
|
let (block_stmts, end_stmts) = block_stmts.split_at(block_stmts.len() - end_eq);
|
|
|
|
|
|
|
|
// Scan start
|
|
|
|
let mut start_walker = UsedValueFinderVisitor::new(cx);
|
|
|
|
for stmt in start_stmts {
|
|
|
|
intravisit::walk_stmt(&mut start_walker, stmt);
|
|
|
|
}
|
|
|
|
let mut moved_syms = start_walker.def_symbols;
|
|
|
|
|
|
|
|
// Scan block
|
|
|
|
let mut block_walker = UsedValueFinderVisitor::new(cx);
|
|
|
|
for stmt in block_stmts {
|
|
|
|
intravisit::walk_stmt(&mut block_walker, stmt);
|
|
|
|
}
|
|
|
|
let mut block_defs = block_walker.defs;
|
|
|
|
|
|
|
|
// Scan moved stmts
|
|
|
|
let mut moved_start: Option<usize> = None;
|
|
|
|
let mut end_walker = UsedValueFinderVisitor::new(cx);
|
|
|
|
for (index, stmt) in end_stmts.iter().enumerate() {
|
|
|
|
intravisit::walk_stmt(&mut end_walker, stmt);
|
|
|
|
|
|
|
|
for value in &end_walker.uses {
|
|
|
|
// Well we can't move this and all prev statements. So reset
|
|
|
|
if block_defs.contains(value) {
|
|
|
|
moved_start = Some(index + 1);
|
|
|
|
end_walker.defs.drain().for_each(|x| {
|
|
|
|
block_defs.insert(x);
|
|
|
|
});
|
|
|
|
|
|
|
|
end_walker.def_symbols.clear();
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
end_walker.uses.clear();
|
|
|
|
}
|
|
|
|
|
|
|
|
if let Some(moved_start) = moved_start {
|
|
|
|
end_eq -= moved_start;
|
|
|
|
}
|
|
|
|
|
|
|
|
let end_linable = block.expr.map_or_else(
|
|
|
|
|| end_eq != 0,
|
|
|
|
|expr| {
|
|
|
|
intravisit::walk_expr(&mut end_walker, expr);
|
|
|
|
end_walker.uses.iter().any(|x| !block_defs.contains(x))
|
|
|
|
},
|
|
|
|
);
|
|
|
|
|
|
|
|
if end_linable {
|
|
|
|
end_walker.def_symbols.drain().for_each(|x| {
|
|
|
|
moved_syms.insert(x);
|
|
|
|
});
|
|
|
|
}
|
|
|
|
|
|
|
|
emit_branches_sharing_code_lint(
|
|
|
|
cx,
|
|
|
|
start_eq,
|
|
|
|
end_eq,
|
|
|
|
end_linable,
|
|
|
|
check_for_warn_of_moved_symbol(cx, &moved_syms, expr),
|
|
|
|
blocks,
|
|
|
|
expr,
|
|
|
|
);
|
2016-01-30 12:16:49 -06:00
|
|
|
}
|
|
|
|
}
|
2016-01-30 11:03:53 -06:00
|
|
|
|
2021-04-22 04:31:13 -05:00
|
|
|
struct BlockEqual {
|
|
|
|
/// The amount statements that are equal from the start
|
|
|
|
start_eq: usize,
|
|
|
|
/// The amount statements that are equal from the end
|
|
|
|
end_eq: usize,
|
|
|
|
/// An indication if the block expressions are the same. This will also be true if both are
|
|
|
|
/// `None`
|
|
|
|
expr_eq: bool,
|
|
|
|
}
|
|
|
|
|
|
|
|
/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to
|
|
|
|
/// abort any further processing and avoid duplicate lint triggers.
|
|
|
|
fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option<BlockEqual> {
|
2021-04-08 10:50:13 -05:00
|
|
|
let mut start_eq = usize::MAX;
|
|
|
|
let mut end_eq = usize::MAX;
|
|
|
|
let mut expr_eq = true;
|
2021-08-08 09:49:13 -05:00
|
|
|
let mut iter = blocks.windows(2);
|
|
|
|
while let Some(&[win0, win1]) = iter.next() {
|
|
|
|
let l_stmts = win0.stmts;
|
|
|
|
let r_stmts = win1.stmts;
|
2016-02-09 09:45:47 -06:00
|
|
|
|
2021-04-08 10:50:13 -05:00
|
|
|
// `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
|
|
|
|
// The comparison therefore needs to be done in a way that builds the correct context.
|
|
|
|
let mut evaluator = SpanlessEq::new(cx);
|
|
|
|
let mut evaluator = evaluator.inter_expr();
|
|
|
|
|
|
|
|
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 = {
|
|
|
|
// We skip the middle statements which can't be equal
|
|
|
|
let end_comparison_count = l_stmts.len().min(r_stmts.len()) - current_start_eq;
|
|
|
|
let it1 = l_stmts.iter().skip(l_stmts.len() - end_comparison_count);
|
|
|
|
let it2 = r_stmts.iter().skip(r_stmts.len() - end_comparison_count);
|
|
|
|
it1.zip(it2)
|
|
|
|
.fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
|
|
|
|
};
|
2021-08-08 09:49:13 -05:00
|
|
|
let block_expr_eq = both(&win0.expr, &win1.expr, |l, r| evaluator.eq_expr(l, r));
|
2021-04-08 10:50:13 -05:00
|
|
|
|
|
|
|
// IF_SAME_THEN_ELSE
|
|
|
|
if_chain! {
|
|
|
|
if block_expr_eq;
|
|
|
|
if l_stmts.len() == r_stmts.len();
|
|
|
|
if l_stmts.len() == current_start_eq;
|
2021-08-08 09:49:13 -05:00
|
|
|
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win0.hir_id);
|
|
|
|
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win1.hir_id);
|
2021-04-08 10:50:13 -05:00
|
|
|
then {
|
|
|
|
span_lint_and_note(
|
|
|
|
cx,
|
|
|
|
IF_SAME_THEN_ELSE,
|
2021-08-08 09:49:13 -05:00
|
|
|
win0.span,
|
2021-04-08 10:50:13 -05:00
|
|
|
"this `if` has identical blocks",
|
2021-08-08 09:49:13 -05:00
|
|
|
Some(win1.span),
|
2021-04-08 10:50:13 -05:00
|
|
|
"same as this",
|
|
|
|
);
|
|
|
|
|
2021-04-22 04:31:13 -05:00
|
|
|
return None;
|
2021-04-08 10:50:13 -05:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
start_eq = start_eq.min(current_start_eq);
|
|
|
|
end_eq = end_eq.min(current_end_eq);
|
|
|
|
expr_eq &= block_expr_eq;
|
|
|
|
}
|
|
|
|
|
2021-07-15 03:44:10 -05:00
|
|
|
if !expr_eq {
|
2021-04-08 10:50:13 -05:00
|
|
|
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;
|
|
|
|
}
|
|
|
|
|
2021-04-22 04:31:13 -05:00
|
|
|
Some(BlockEqual {
|
|
|
|
start_eq,
|
|
|
|
end_eq,
|
|
|
|
expr_eq,
|
|
|
|
})
|
2021-04-08 10:50:13 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
fn check_for_warn_of_moved_symbol(
|
|
|
|
cx: &LateContext<'tcx>,
|
|
|
|
symbols: &FxHashSet<Symbol>,
|
|
|
|
if_expr: &'tcx Expr<'_>,
|
|
|
|
) -> bool {
|
|
|
|
get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| {
|
|
|
|
let ignore_span = block.span.shrink_to_lo().to(if_expr.span);
|
|
|
|
|
|
|
|
symbols
|
|
|
|
.iter()
|
|
|
|
.filter(|sym| !sym.as_str().starts_with('_'))
|
|
|
|
.any(move |sym| {
|
|
|
|
let mut walker = ContainsName {
|
|
|
|
name: *sym,
|
|
|
|
result: false,
|
|
|
|
};
|
|
|
|
|
|
|
|
// Scan block
|
|
|
|
block
|
|
|
|
.stmts
|
|
|
|
.iter()
|
|
|
|
.filter(|stmt| !ignore_span.overlaps(stmt.span))
|
|
|
|
.for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
|
|
|
|
|
|
|
|
if let Some(expr) = block.expr {
|
|
|
|
intravisit::walk_expr(&mut walker, expr);
|
|
|
|
}
|
|
|
|
|
|
|
|
walker.result
|
|
|
|
})
|
|
|
|
})
|
|
|
|
}
|
|
|
|
|
|
|
|
fn emit_branches_sharing_code_lint(
|
|
|
|
cx: &LateContext<'tcx>,
|
|
|
|
start_stmts: usize,
|
|
|
|
end_stmts: usize,
|
|
|
|
lint_end: bool,
|
|
|
|
warn_about_moved_symbol: 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![];
|
|
|
|
let mut add_expr_note = false;
|
|
|
|
|
|
|
|
// Construct suggestions
|
|
|
|
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", 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 = block
|
|
|
|
.expr
|
|
|
|
.map_or_else(|| block.stmts[block.stmts.len() - 1].span, |expr| expr.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 mut span = moved_start.to(span_end);
|
|
|
|
// Improve formatting if the inner block has indention (i.e. normal Rust formatting)
|
2021-04-18 07:27:04 -05:00
|
|
|
let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt(), span.parent());
|
2021-04-08 10:50:13 -05:00
|
|
|
if snippet_opt(cx, test_span)
|
|
|
|
.map(|snip| snip == " ")
|
|
|
|
.unwrap_or_default()
|
|
|
|
{
|
|
|
|
span = span.with_lo(test_span.lo());
|
|
|
|
}
|
|
|
|
|
|
|
|
suggestions.push(("end", span, suggestion.to_string()));
|
2021-06-03 01:41:37 -05:00
|
|
|
add_expr_note = !cx.typeck_results().expr_ty(if_expr).is_unit();
|
2021-04-08 10:50:13 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
let add_optional_msgs = |diag: &mut DiagnosticBuilder<'_>| {
|
|
|
|
if add_expr_note {
|
|
|
|
diag.note("The end suggestion probably needs some adjustments to use the expression result correctly");
|
|
|
|
}
|
|
|
|
|
|
|
|
if warn_about_moved_symbol {
|
|
|
|
diag.warn("Some moved values might need to be renamed to avoid wrong references");
|
|
|
|
}
|
|
|
|
};
|
|
|
|
|
|
|
|
// Emit lint
|
|
|
|
if suggestions.len() == 1 {
|
|
|
|
let (place_str, span, sugg) = suggestions.pop().unwrap();
|
|
|
|
let msg = format!("all if blocks contain the same code at the {}", place_str);
|
|
|
|
let help = format!("consider moving the {} statements out like this", place_str);
|
|
|
|
span_lint_and_then(cx, BRANCHES_SHARING_CODE, span, msg.as_str(), |diag| {
|
|
|
|
diag.span_suggestion(span, help.as_str(), sugg, Applicability::Unspecified);
|
|
|
|
|
|
|
|
add_optional_msgs(diag);
|
|
|
|
});
|
|
|
|
} else if suggestions.len() == 2 {
|
|
|
|
let (_, end_span, end_sugg) = suggestions.pop().unwrap();
|
|
|
|
let (_, start_span, start_sugg) = suggestions.pop().unwrap();
|
|
|
|
span_lint_and_then(
|
2017-08-09 02:30:56 -05:00
|
|
|
cx,
|
2021-04-08 10:50:13 -05:00
|
|
|
BRANCHES_SHARING_CODE,
|
|
|
|
start_span,
|
|
|
|
"all if blocks contain the same code at the start and the end. Here at the start",
|
|
|
|
move |diag| {
|
|
|
|
diag.span_note(end_span, "and here at the end");
|
|
|
|
|
|
|
|
diag.span_suggestion(
|
|
|
|
start_span,
|
|
|
|
"consider moving the start statements out like this",
|
|
|
|
start_sugg,
|
|
|
|
Applicability::Unspecified,
|
|
|
|
);
|
|
|
|
|
|
|
|
diag.span_suggestion(
|
|
|
|
end_span,
|
|
|
|
"and consider moving the end statements out like this",
|
|
|
|
end_sugg,
|
|
|
|
Applicability::Unspecified,
|
|
|
|
);
|
|
|
|
|
|
|
|
add_optional_msgs(diag);
|
|
|
|
},
|
2017-08-09 02:30:56 -05:00
|
|
|
);
|
2016-01-30 12:16:49 -06:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2021-04-08 10:50:13 -05:00
|
|
|
/// This visitor collects `HirId`s and Symbols of defined symbols and `HirId`s of used values.
|
|
|
|
struct UsedValueFinderVisitor<'a, 'tcx> {
|
|
|
|
cx: &'a LateContext<'tcx>,
|
|
|
|
|
|
|
|
/// The `HirId`s of defined values in the scanned statements
|
|
|
|
defs: FxHashSet<HirId>,
|
|
|
|
|
|
|
|
/// The Symbols of the defined symbols in the scanned statements
|
|
|
|
def_symbols: FxHashSet<Symbol>,
|
|
|
|
|
|
|
|
/// The `HirId`s of the used values
|
|
|
|
uses: FxHashSet<HirId>,
|
|
|
|
}
|
|
|
|
|
|
|
|
impl<'a, 'tcx> UsedValueFinderVisitor<'a, 'tcx> {
|
|
|
|
fn new(cx: &'a LateContext<'tcx>) -> Self {
|
|
|
|
UsedValueFinderVisitor {
|
|
|
|
cx,
|
|
|
|
defs: FxHashSet::default(),
|
|
|
|
def_symbols: FxHashSet::default(),
|
|
|
|
uses: FxHashSet::default(),
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
impl<'a, 'tcx> Visitor<'tcx> for UsedValueFinderVisitor<'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(sym) = l.pat.simple_ident() {
|
|
|
|
self.def_symbols.insert(sym.name);
|
|
|
|
}
|
|
|
|
|
|
|
|
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(_, 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);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2016-01-30 12:16:49 -06:00
|
|
|
/// Implementation of `IFS_SAME_COND`.
|
2020-06-25 15:41:36 -05:00
|
|
|
fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
|
2019-12-27 01:12:26 -06:00
|
|
|
let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
|
2020-06-25 21:55:23 -05:00
|
|
|
let mut h = SpanlessHash::new(cx);
|
2016-02-09 09:45:47 -06:00
|
|
|
h.hash_expr(expr);
|
|
|
|
h.finish()
|
|
|
|
};
|
2016-02-09 18:22:53 -06:00
|
|
|
|
2020-08-28 09:10:16 -05:00
|
|
|
let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) };
|
2016-02-09 09:45:47 -06:00
|
|
|
|
2019-05-20 03:22:13 -05:00
|
|
|
for (i, j) in search_same(conds, hash, eq) {
|
2020-01-26 20:26:42 -06:00
|
|
|
span_lint_and_note(
|
2017-08-09 02:30:56 -05:00
|
|
|
cx,
|
|
|
|
IFS_SAME_COND,
|
|
|
|
j.span,
|
2020-01-06 00:30:43 -06:00
|
|
|
"this `if` has the same condition as a previous `if`",
|
2020-04-18 05:29:36 -05:00
|
|
|
Some(i.span),
|
2017-08-09 02:30:56 -05:00
|
|
|
"same as this",
|
|
|
|
);
|
2016-02-09 18:22:53 -06:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2019-11-13 23:06:34 -06:00
|
|
|
/// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
|
2020-06-25 15:41:36 -05:00
|
|
|
fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
|
2019-12-27 01:12:26 -06:00
|
|
|
let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
|
2020-06-25 21:55:23 -05:00
|
|
|
let mut h = SpanlessHash::new(cx);
|
2019-11-13 23:06:34 -06:00
|
|
|
h.hash_expr(expr);
|
|
|
|
h.finish()
|
|
|
|
};
|
|
|
|
|
2019-12-27 01:12:26 -06:00
|
|
|
let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
|
2020-10-23 15:16:59 -05:00
|
|
|
// Do not lint if any expr originates from a macro
|
|
|
|
if in_macro(lhs.span) || in_macro(rhs.span) {
|
|
|
|
return false;
|
|
|
|
}
|
2019-11-13 23:06:34 -06:00
|
|
|
// Do not spawn warning if `IFS_SAME_COND` already produced it.
|
2020-08-28 09:10:16 -05:00
|
|
|
if eq_expr_value(cx, lhs, rhs) {
|
2019-11-13 23:06:34 -06:00
|
|
|
return false;
|
|
|
|
}
|
|
|
|
SpanlessEq::new(cx).eq_expr(lhs, rhs)
|
|
|
|
};
|
|
|
|
|
|
|
|
for (i, j) in search_same(conds, hash, eq) {
|
2020-01-26 20:26:42 -06:00
|
|
|
span_lint_and_note(
|
2019-11-13 23:06:34 -06:00
|
|
|
cx,
|
|
|
|
SAME_FUNCTIONS_IN_IF_CONDITION,
|
|
|
|
j.span,
|
2020-01-06 00:30:43 -06:00
|
|
|
"this `if` has the same function call as a previous `if`",
|
2020-04-18 05:29:36 -05:00
|
|
|
Some(i.span),
|
2019-11-13 23:06:34 -06:00
|
|
|
"same as this",
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|