Implement if_same_cond_fn lint
Run ./util/dev Revert changelog entry Rename lint to same_functions_in_if_condition and add a doc example Add testcases with different arg in fn invocation
This commit is contained in:
parent
b4f1769734
commit
bbb8cd4fbb
7 changed files with 244 additions and 3 deletions
|
|
@ -40,6 +40,53 @@ declare_clippy_lint! {
|
|||
"consecutive `ifs` with the same condition"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for consecutive `if`s with the same function call.
|
||||
///
|
||||
/// **Why is this bad?** This is probably a copy & paste error.
|
||||
/// 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".
|
||||
///
|
||||
/// **Known problems:** Hopefully none.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```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,
|
||||
"consecutive `ifs` with the same function call"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for `if/else` with the same body as the *then* part
|
||||
/// and the *else* part.
|
||||
|
|
@ -102,7 +149,7 @@ declare_clippy_lint! {
|
|||
"`match` with identical arm bodies"
|
||||
}
|
||||
|
||||
declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]);
|
||||
declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, SAME_FUNCTIONS_IN_IF_CONDITION, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]);
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
|
||||
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
|
||||
|
|
@ -119,6 +166,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
|
|||
let (conds, blocks) = if_sequence(expr);
|
||||
lint_same_then_else(cx, &blocks);
|
||||
lint_same_cond(cx, &conds);
|
||||
lint_same_fns_in_if_cond(cx, &conds);
|
||||
lint_match_arms(cx, expr);
|
||||
}
|
||||
}
|
||||
|
|
@ -163,6 +211,34 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
|
|||
}
|
||||
}
|
||||
|
||||
/// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
|
||||
fn lint_same_fns_in_if_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
|
||||
let hash: &dyn Fn(&&Expr) -> u64 = &|expr| -> u64 {
|
||||
let mut h = SpanlessHash::new(cx, cx.tables);
|
||||
h.hash_expr(expr);
|
||||
h.finish()
|
||||
};
|
||||
|
||||
let eq: &dyn Fn(&&Expr, &&Expr) -> bool = &|&lhs, &rhs| -> bool {
|
||||
// Do not spawn warning if `IFS_SAME_COND` already produced it.
|
||||
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) {
|
||||
return false;
|
||||
}
|
||||
SpanlessEq::new(cx).eq_expr(lhs, rhs)
|
||||
};
|
||||
|
||||
for (i, j) in search_same(conds, hash, eq) {
|
||||
span_note_and_lint(
|
||||
cx,
|
||||
SAME_FUNCTIONS_IN_IF_CONDITION,
|
||||
j.span,
|
||||
"this `if` has the same function call as a previous if",
|
||||
i.span,
|
||||
"same as this",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Implementation of `MATCH_SAME_ARMS`.
|
||||
fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) {
|
||||
fn same_bindings<'tcx>(
|
||||
|
|
|
|||
|
|
@ -471,6 +471,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
|
|||
&collapsible_if::COLLAPSIBLE_IF,
|
||||
&comparison_chain::COMPARISON_CHAIN,
|
||||
&copies::IFS_SAME_COND,
|
||||
&copies::SAME_FUNCTIONS_IN_IF_CONDITION,
|
||||
&copies::IF_SAME_THEN_ELSE,
|
||||
&copies::MATCH_SAME_ARMS,
|
||||
©_iterator::COPY_ITERATOR,
|
||||
|
|
@ -989,6 +990,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
|
|||
store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
|
||||
LintId::of(&attrs::INLINE_ALWAYS),
|
||||
LintId::of(&checked_conversions::CHECKED_CONVERSIONS),
|
||||
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
|
||||
LintId::of(&copies::MATCH_SAME_ARMS),
|
||||
LintId::of(©_iterator::COPY_ITERATOR),
|
||||
LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue