Skip to content

Commit 063bd09

Browse files
committed
enhance [ifs_same_cond] to lint same immutable method calls as well
1 parent 63562a6 commit 063bd09

File tree

3 files changed

+46
-4
lines changed

3 files changed

+46
-4
lines changed

clippy_lints/src/copies.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, sn
33
use clippy_utils::ty::needs_ordered_drop;
44
use clippy_utils::visitors::for_each_expr;
55
use clippy_utils::{
6-
capture_local_usage, eq_expr_value, get_enclosing_block, hash_expr, hash_stmt, if_sequence, is_else_clause,
7-
is_lint_allowed, path_to_local, search_same, ContainsName, HirEqInterExpr, SpanlessEq,
6+
capture_local_usage, eq_expr_value, find_binding_init, get_enclosing_block, hash_expr, hash_stmt, if_sequence,
7+
is_else_clause, is_lint_allowed, path_to_local, search_same, ContainsName, HirEqInterExpr, SpanlessEq,
88
};
99
use core::iter;
1010
use core::ops::ControlFlow;
@@ -549,7 +549,27 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
549549

550550
/// Implementation of `IFS_SAME_COND`.
551551
fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
552-
for (i, j) in search_same(conds, |e| hash_expr(cx, e), |lhs, rhs| eq_expr_value(cx, lhs, rhs)) {
552+
for (i, j) in search_same(
553+
conds,
554+
|e| hash_expr(cx, e),
555+
|lhs, rhs| {
556+
// If any side (ex. lhs) is a method call, and the caller is not mutable,
557+
// then we can ignore side effects?
558+
if let ExprKind::MethodCall(_, caller, _, _) = lhs.kind {
559+
if path_to_local(caller)
560+
.and_then(|hir_id| find_binding_init(cx, hir_id))
561+
.is_some()
562+
{
563+
// caller is not declared as mutable
564+
SpanlessEq::new(cx).eq_expr(lhs, rhs)
565+
} else {
566+
false
567+
}
568+
} else {
569+
eq_expr_value(cx, lhs, rhs)
570+
}
571+
},
572+
) {
553573
span_lint_and_note(
554574
cx,
555575
IFS_SAME_COND,

tests/ui/ifs_same_cond.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,14 @@ fn ifs_same_cond() {
4343
}
4444
}
4545

46+
fn issue10272() {
47+
let a = String::from("ha");
48+
if a.contains("ah") {
49+
} else if a.contains("ah") {
50+
// Trigger this lint
51+
} else if a.contains("ha") {
52+
} else if a == "wow" {
53+
}
54+
}
55+
4656
fn main() {}

tests/ui/ifs_same_cond.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,17 @@ note: same as this
3535
LL | if 2 * a == 1 {
3636
| ^^^^^^^^^^
3737

38-
error: aborting due to 3 previous errors
38+
error: this `if` has the same condition as a previous `if`
39+
--> $DIR/ifs_same_cond.rs:49:15
40+
|
41+
LL | } else if a.contains("ah") {
42+
| ^^^^^^^^^^^^^^^^
43+
|
44+
note: same as this
45+
--> $DIR/ifs_same_cond.rs:48:8
46+
|
47+
LL | if a.contains("ah") {
48+
| ^^^^^^^^^^^^^^^^
49+
50+
error: aborting due to 4 previous errors
3951

0 commit comments

Comments
 (0)