Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn about explicit self-assignment #5894

Merged
merged 4 commits into from
Aug 16, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1690,6 +1690,7 @@ Released 2018-09-13
[`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition
[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
[`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
[`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
14 changes: 6 additions & 8 deletions clippy_lints/src/assign_ops.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{
get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method, SpanlessEq,
eq_expr_value, get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method,
};
use crate::utils::{higher, sugg};
use if_chain::if_chain;
@@ -70,11 +70,11 @@ impl<'tcx> LateLintPass<'tcx> for AssignOps {
return;
}
// lhs op= l op r
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, l) {
if eq_expr_value(cx, lhs, l) {
lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, r);
}
// lhs op= l commutative_op r
if is_commutative(op.node) && SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, r) {
if is_commutative(op.node) && eq_expr_value(cx, lhs, r) {
lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, l);
}
}
@@ -161,14 +161,12 @@ impl<'tcx> LateLintPass<'tcx> for AssignOps {

if visitor.counter == 1 {
// a = a op b
if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, l) {
if eq_expr_value(cx, assignee, l) {
lint(assignee, r);
}
// a = b commutative_op a
// Limited to primitive type as these ops are know to be commutative
if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, r)
&& cx.typeck_results().expr_ty(assignee).is_primitive_ty()
{
if eq_expr_value(cx, assignee, r) && cx.typeck_results().expr_ty(assignee).is_primitive_ty() {
match op.node {
hir::BinOpKind::Add
| hir::BinOpKind::Mul
@@ -253,7 +251,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ExprVisitor<'a, 'tcx> {
type Map = Map<'tcx>;

fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
if SpanlessEq::new(self.cx).ignore_fn().eq_expr(self.assignee, expr) {
if eq_expr_value(self.cx, self.assignee, expr) {
self.counter += 1;
}

10 changes: 5 additions & 5 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::utils::{
get_trait_def_id, implements_trait, in_macro, is_type_diagnostic_item, paths, snippet_opt, span_lint_and_sugg,
span_lint_and_then, SpanlessEq,
eq_expr_value, get_trait_def_id, implements_trait, in_macro, is_type_diagnostic_item, paths, snippet_opt,
span_lint_and_sugg, span_lint_and_then,
};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
@@ -128,7 +128,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
}
}
for (n, expr) in self.terminals.iter().enumerate() {
if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e, expr) {
if eq_expr_value(self.cx, e, expr) {
#[allow(clippy::cast_possible_truncation)]
return Ok(Bool::Term(n as u8));
}
@@ -138,8 +138,8 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
if implements_ord(self.cx, e_lhs);
if let ExprKind::Binary(expr_binop, expr_lhs, expr_rhs) = &expr.kind;
if negate(e_binop.node) == Some(expr_binop.node);
if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e_lhs, expr_lhs);
if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e_rhs, expr_rhs);
if eq_expr_value(self.cx, e_lhs, expr_lhs);
if eq_expr_value(self.cx, e_rhs, expr_rhs);
then {
#[allow(clippy::cast_possible_truncation)]
return Ok(Bool::Not(Box::new(Bool::Term(n as u8))));
7 changes: 3 additions & 4 deletions clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{eq_expr_value, SpanlessEq, SpanlessHash};
use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then};
use crate::utils::{SpanlessEq, SpanlessHash};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{Arm, Block, Expr, ExprKind, MatchSource, Pat, PatKind};
use rustc_lint::{LateContext, LateLintPass};
@@ -197,8 +197,7 @@ fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
h.finish()
};

let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool =
&|&lhs, &rhs| -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) };
let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) };

for (i, j) in search_same(conds, hash, eq) {
span_lint_and_note(
@@ -222,7 +221,7 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {

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) {
if eq_expr_value(cx, lhs, rhs) {
return false;
}
SpanlessEq::new(cx).eq_expr(lhs, rhs)
5 changes: 2 additions & 3 deletions clippy_lints/src/double_comparison.rs
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@ use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;

use crate::utils::{snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
use crate::utils::{eq_expr_value, snippet_with_applicability, span_lint_and_sugg};

declare_clippy_lint! {
/// **What it does:** Checks for double comparisons that could be simplified to a single expression.
@@ -46,8 +46,7 @@ impl<'tcx> DoubleComparisons {
},
_ => return,
};
let mut spanless_eq = SpanlessEq::new(cx).ignore_fn();
if !(spanless_eq.eq_expr(&llhs, &rlhs) && spanless_eq.eq_expr(&lrhs, &rrhs)) {
if !(eq_expr_value(cx, &llhs, &rlhs) && eq_expr_value(cx, &lrhs, &rrhs)) {
return;
}
macro_rules! lint_double_comparison {
4 changes: 2 additions & 2 deletions clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{
implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then, SpanlessEq,
eq_expr_value, implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then,
};
use rustc_errors::Applicability;
use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind};
@@ -69,7 +69,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) {
return;
}
if is_valid_operator(op) && SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) {
if is_valid_operator(op) && eq_expr_value(cx, left, right) {
span_lint(
cx,
EQ_OP,
24 changes: 10 additions & 14 deletions clippy_lints/src/floating_point_arithmetic.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ use crate::consts::{
constant, constant_simple, Constant,
Constant::{Int, F32, F64},
};
use crate::utils::{get_parent_expr, higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq};
use crate::utils::{eq_expr_value, get_parent_expr, higher, numeric_literal, span_lint_and_sugg, sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp};
@@ -363,8 +363,8 @@ fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option<String> {
if_chain! {
if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref lmul_lhs, ref lmul_rhs) = add_lhs.kind;
if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref rmul_lhs, ref rmul_rhs) = add_rhs.kind;
if are_exprs_equal(cx, lmul_lhs, lmul_rhs);
if are_exprs_equal(cx, rmul_lhs, rmul_rhs);
if eq_expr_value(cx, lmul_lhs, lmul_rhs);
if eq_expr_value(cx, rmul_lhs, rmul_rhs);
then {
return Some(format!("{}.hypot({})", Sugg::hir(cx, &lmul_lhs, ".."), Sugg::hir(cx, &rmul_lhs, "..")));
}
@@ -502,8 +502,8 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) {
fn is_testing_positive(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
match op {
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, right) && are_exprs_equal(cx, left, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, left) && are_exprs_equal(cx, right, test),
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, right) && eq_expr_value(cx, left, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, left) && eq_expr_value(cx, right, test),
_ => false,
}
} else {
@@ -515,19 +515,15 @@ fn is_testing_positive(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -
fn is_testing_negative(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
match op {
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, left) && are_exprs_equal(cx, right, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, right) && are_exprs_equal(cx, left, test),
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, left) && eq_expr_value(cx, right, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, right) && eq_expr_value(cx, left, test),
_ => false,
}
} else {
false
}
}

fn are_exprs_equal(cx: &LateContext<'_>, expr1: &Expr<'_>, expr2: &Expr<'_>) -> bool {
SpanlessEq::new(cx).ignore_fn().eq_expr(expr1, expr2)
}

/// Returns true iff expr is some zero literal
fn is_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
match constant_simple(cx, cx.typeck_results(), expr) {
@@ -546,12 +542,12 @@ fn is_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
/// returns None.
fn are_negated<'a>(cx: &LateContext<'_>, expr1: &'a Expr<'a>, expr2: &'a Expr<'a>) -> Option<(bool, &'a Expr<'a>)> {
if let ExprKind::Unary(UnOp::UnNeg, expr1_negated) = &expr1.kind {
if are_exprs_equal(cx, expr1_negated, expr2) {
if eq_expr_value(cx, expr1_negated, expr2) {
return Some((false, expr2));
}
}
if let ExprKind::Unary(UnOp::UnNeg, expr2_negated) = &expr2.kind {
if are_exprs_equal(cx, expr1, expr2_negated) {
if eq_expr_value(cx, expr1, expr2_negated) {
return Some((true, expr1));
}
}
@@ -614,7 +610,7 @@ fn are_same_base_logs(cx: &LateContext<'_>, expr_a: &Expr<'_>, expr_b: &Expr<'_>
args_a.len() == args_b.len() &&
(
["ln", "log2", "log10"].contains(&&*method_name_a.as_str()) ||
method_name_a.as_str() == "log" && args_a.len() == 2 && are_exprs_equal(cx, &args_a[1], &args_b[1])
method_name_a.as_str() == "log" && args_a.len() == 2 && eq_expr_value(cx, &args_a[1], &args_b[1])
);
}
}
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -284,6 +284,7 @@ mod reference;
mod regex;
mod repeat_once;
mod returns;
mod self_assignment;
mod serde_api;
mod shadow;
mod single_component_path_imports;
@@ -773,6 +774,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&repeat_once::REPEAT_ONCE,
&returns::LET_AND_RETURN,
&returns::NEEDLESS_RETURN,
&self_assignment::SELF_ASSIGNMENT,
&serde_api::SERDE_API_MISUSE,
&shadow::SHADOW_REUSE,
&shadow::SHADOW_SAME,
@@ -1090,6 +1092,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);
store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive);
store.register_late_pass(|| box repeat_once::RepeatOnce);
store.register_late_pass(|| box self_assignment::SelfAssignment);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1421,6 +1424,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&repeat_once::REPEAT_ONCE),
LintId::of(&returns::LET_AND_RETURN),
LintId::of(&returns::NEEDLESS_RETURN),
LintId::of(&self_assignment::SELF_ASSIGNMENT),
LintId::of(&serde_api::SERDE_API_MISUSE),
LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
@@ -1714,6 +1718,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&ptr::MUT_FROM_REF),
LintId::of(&ranges::REVERSED_EMPTY_RANGES),
LintId::of(&regex::INVALID_REGEX),
LintId::of(&self_assignment::SELF_ASSIGNMENT),
LintId::of(&serde_api::SERDE_API_MISUSE),
LintId::of(&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
LintId::of(&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
6 changes: 3 additions & 3 deletions clippy_lints/src/question_mark.rs
Original file line number Diff line number Diff line change
@@ -7,8 +7,8 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};

use crate::utils::sugg::Sugg;
use crate::utils::{
higher, is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet_with_applicability,
span_lint_and_sugg, SpanlessEq,
eq_expr_value, higher, is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet_with_applicability,
span_lint_and_sugg,
};

declare_clippy_lint! {
@@ -65,7 +65,7 @@ impl QuestionMark {
if let ExprKind::Block(block, None) = &else_.kind;
if block.stmts.is_empty();
if let Some(block_expr) = &block.expr;
if SpanlessEq::new(cx).ignore_fn().eq_expr(subject, block_expr);
if eq_expr_value(cx, subject, block_expr);
then {
replacement = Some(format!("Some({}?)", receiver_str));
}
51 changes: 51 additions & 0 deletions clippy_lints/src/self_assignment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use crate::utils::{eq_expr_value, snippet, span_lint};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:** Checks for explicit self-assignments.
///
/// **Why is this bad?** Self-assignments are redundant and unlikely to be
/// intentional.
///
/// **Known problems:** If expression contains any deref coercions or
/// indexing operations they are assumed not to have any side effects.
///
/// **Example:**
///
/// ```rust
/// struct Event {
/// id: usize,
/// x: i32,
/// y: i32,
/// }
///
/// fn copy_position(a: &mut Event, b: &Event) {
/// a.x = b.x;
/// a.y = a.y;
/// }
/// ```
pub SELF_ASSIGNMENT,
correctness,
"explicit self-assignment"
}

declare_lint_pass!(SelfAssignment => [SELF_ASSIGNMENT]);

impl<'tcx> LateLintPass<'tcx> for SelfAssignment {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Assign(lhs, rhs, _) = &expr.kind {
if eq_expr_value(cx, lhs, rhs) {
let lhs = snippet(cx, lhs.span, "<lhs>");
let rhs = snippet(cx, rhs.span, "<rhs>");
span_lint(
cx,
SELF_ASSIGNMENT,
expr.span,
&format!("self-assignment of `{}` to `{}`", rhs, lhs),
);
}
}
}
}
14 changes: 7 additions & 7 deletions clippy_lints/src/swap.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::utils::sugg::Sugg;
use crate::utils::{
differing_macro_contexts, is_type_diagnostic_item, snippet_with_applicability, span_lint_and_then, walk_ptrs_ty,
SpanlessEq,
differing_macro_contexts, eq_expr_value, is_type_diagnostic_item, snippet_with_applicability, span_lint_and_then,
walk_ptrs_ty,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
@@ -92,8 +92,8 @@ fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) {
if rhs2.segments.len() == 1;

if ident.as_str() == rhs2.segments[0].ident.as_str();
if SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1);
if SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2);
if eq_expr_value(cx, tmp_init, lhs1);
if eq_expr_value(cx, rhs1, lhs2);
then {
if let ExprKind::Field(ref lhs1, _) = lhs1.kind {
if let ExprKind::Field(ref lhs2, _) = lhs2.kind {
@@ -193,7 +193,7 @@ enum Slice<'a> {
fn check_for_slice<'a>(cx: &LateContext<'_>, lhs1: &'a Expr<'_>, lhs2: &'a Expr<'_>) -> Slice<'a> {
if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.kind {
if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.kind {
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) {
if eq_expr_value(cx, lhs1, lhs2) {
let ty = walk_ptrs_ty(cx.typeck_results().expr_ty(lhs1));

if matches!(ty.kind, ty::Slice(_))
@@ -221,8 +221,8 @@ fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) {
if !differing_macro_contexts(first.span, second.span);
if let ExprKind::Assign(ref lhs0, ref rhs0, _) = first.kind;
if let ExprKind::Assign(ref lhs1, ref rhs1, _) = second.kind;
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1);
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0);
if eq_expr_value(cx, lhs0, rhs1);
if eq_expr_value(cx, lhs1, rhs0);
then {
let lhs0 = Sugg::hir_opt(cx, lhs0);
let rhs0 = Sugg::hir_opt(cx, rhs0);
26 changes: 15 additions & 11 deletions clippy_lints/src/utils/hir_utils.rs
Original file line number Diff line number Diff line change
@@ -23,23 +23,22 @@ pub struct SpanlessEq<'a, 'tcx> {
/// Context used to evaluate constant expressions.
cx: &'a LateContext<'tcx>,
maybe_typeck_results: Option<&'tcx TypeckResults<'tcx>>,
/// If is true, never consider as equal expressions containing function
/// calls.
ignore_fn: bool,
allow_side_effects: bool,
}

impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
pub fn new(cx: &'a LateContext<'tcx>) -> Self {
Self {
cx,
maybe_typeck_results: cx.maybe_typeck_results(),
ignore_fn: false,
allow_side_effects: true,
}
}

pub fn ignore_fn(self) -> Self {
/// Consider expressions containing potential side effects as not equal.
pub fn deny_side_effects(self) -> Self {
Self {
ignore_fn: true,
allow_side_effects: false,
..self
}
}
@@ -67,7 +66,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {

#[allow(clippy::similar_names)]
pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
if self.ignore_fn && differing_macro_contexts(left.span, right.span) {
if !self.allow_side_effects && differing_macro_contexts(left.span, right.span) {
return false;
}

@@ -90,10 +89,10 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
both(&li.label, &ri.label, |l, r| l.ident.as_str() == r.ident.as_str())
},
(&ExprKind::Assign(ref ll, ref lr, _), &ExprKind::Assign(ref rl, ref rr, _)) => {
self.eq_expr(ll, rl) && self.eq_expr(lr, rr)
self.allow_side_effects && self.eq_expr(ll, rl) && self.eq_expr(lr, rr)
},
(&ExprKind::AssignOp(ref lo, ref ll, ref lr), &ExprKind::AssignOp(ref ro, ref rl, ref rr)) => {
lo.node == ro.node && self.eq_expr(ll, rl) && self.eq_expr(lr, rr)
self.allow_side_effects && lo.node == ro.node && self.eq_expr(ll, rl) && self.eq_expr(lr, rr)
},
(&ExprKind::Block(ref l, _), &ExprKind::Block(ref r, _)) => self.eq_block(l, r),
(&ExprKind::Binary(l_op, ref ll, ref lr), &ExprKind::Binary(r_op, ref rl, ref rr)) => {
@@ -108,7 +107,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
},
(&ExprKind::Box(ref l), &ExprKind::Box(ref r)) => self.eq_expr(l, r),
(&ExprKind::Call(l_fun, l_args), &ExprKind::Call(r_fun, r_args)) => {
!self.ignore_fn && self.eq_expr(l_fun, r_fun) && self.eq_exprs(l_args, r_args)
self.allow_side_effects && self.eq_expr(l_fun, r_fun) && self.eq_exprs(l_args, r_args)
},
(&ExprKind::Cast(ref lx, ref lt), &ExprKind::Cast(ref rx, ref rt))
| (&ExprKind::Type(ref lx, ref lt), &ExprKind::Type(ref rx, ref rt)) => {
@@ -134,7 +133,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
})
},
(&ExprKind::MethodCall(l_path, _, l_args, _), &ExprKind::MethodCall(r_path, _, r_args, _)) => {
!self.ignore_fn && self.eq_path_segment(l_path, r_path) && self.eq_exprs(l_args, r_args)
self.allow_side_effects && self.eq_path_segment(l_path, r_path) && self.eq_exprs(l_args, r_args)
},
(&ExprKind::Repeat(ref le, ref ll_id), &ExprKind::Repeat(ref re, ref rl_id)) => {
let mut celcx = constant_context(self.cx, self.cx.tcx.typeck_body(ll_id.body));
@@ -340,6 +339,11 @@ 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))
}

/// 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)
}

/// Type used to hash an ast element. This is different from the `Hash` trait
/// on ast types as this
/// trait would consider IDs and spans.
5 changes: 2 additions & 3 deletions clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::utils::SpanlessEq;
use crate::utils::{
is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, run_lints, snippet, span_lint,
span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty,
span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, NodeId};
@@ -493,7 +492,7 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleCalls {
if let StmtKind::Semi(only_expr) = &stmts[0].kind;
if let ExprKind::MethodCall(ref ps, _, ref span_call_args, _) = &only_expr.kind;
let and_then_snippets = get_and_then_snippets(cx, and_then_args);
let mut sle = SpanlessEq::new(cx).ignore_fn();
let mut sle = SpanlessEq::new(cx).deny_side_effects();
then {
match &*ps.ident.as_str() {
"span_suggestion" if sle.eq_expr(&and_then_args[2], &span_call_args[1]) => {
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ pub mod sugg;
pub mod usage;
pub use self::attrs::*;
pub use self::diagnostics::*;
pub use self::hir_utils::{both, over, SpanlessEq, SpanlessHash};
pub use self::hir_utils::{both, eq_expr_value, over, SpanlessEq, SpanlessHash};

use std::borrow::Cow;
use std::mem;
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
@@ -1956,6 +1956,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "methods",
},
Lint {
name: "self_assignment",
group: "correctness",
desc: "explicit self-assignment",
deprecation: None,
module: "self_assignment",
},
Lint {
name: "serde_api_misuse",
group: "correctness",
67 changes: 67 additions & 0 deletions tests/ui/self_assignment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#![warn(clippy::self_assignment)]

pub struct S<'a> {
a: i32,
b: [i32; 10],
c: Vec<Vec<i32>>,
e: &'a mut i32,
f: &'a mut i32,
}

pub fn positives(mut a: usize, b: &mut u32, mut s: S) {
a = a;
*b = *b;
s = s;
s.a = s.a;
s.b[10] = s.b[5 + 5];
s.c[0][1] = s.c[0][1];
s.b[a] = s.b[a];
*s.e = *s.e;
s.b[a + 10] = s.b[10 + a];

let mut t = (0, 1);
t.1 = t.1;
t.0 = (t.0);
}

pub fn negatives_not_equal(mut a: usize, b: &mut usize, mut s: S) {
dbg!(&a);
a = *b;
dbg!(&a);
s.b[1] += s.b[1];
s.b[1] = s.b[2];
s.c[1][0] = s.c[0][1];
s.b[a] = s.b[*b];
s.b[a + 10] = s.b[a + 11];
*s.e = *s.f;

let mut t = (0, 1);
t.0 = t.1;
}

#[allow(clippy::eval_order_dependence)]
pub fn negatives_side_effects() {
let mut v = vec![1, 2, 3, 4, 5];
let mut i = 0;
v[{
i += 1;
i
}] = v[{
i += 1;
i
}];

fn next(n: &mut usize) -> usize {
let v = *n;
*n += 1;
v
}

let mut w = vec![1, 2, 3, 4, 5];
let mut i = 0;
let i = &mut i;
w[next(i)] = w[next(i)];
w[next(i)] = w[next(i)];
}

fn main() {}
70 changes: 70 additions & 0 deletions tests/ui/self_assignment.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
error: self-assignment of `a` to `a`
--> $DIR/self_assignment.rs:12:5
|
LL | a = a;
| ^^^^^
|
= note: `-D clippy::self-assignment` implied by `-D warnings`

error: self-assignment of `*b` to `*b`
--> $DIR/self_assignment.rs:13:5
|
LL | *b = *b;
| ^^^^^^^

error: self-assignment of `s` to `s`
--> $DIR/self_assignment.rs:14:5
|
LL | s = s;
| ^^^^^

error: self-assignment of `s.a` to `s.a`
--> $DIR/self_assignment.rs:15:5
|
LL | s.a = s.a;
| ^^^^^^^^^

error: self-assignment of `s.b[5 + 5]` to `s.b[10]`
--> $DIR/self_assignment.rs:16:5
|
LL | s.b[10] = s.b[5 + 5];
| ^^^^^^^^^^^^^^^^^^^^

error: self-assignment of `s.c[0][1]` to `s.c[0][1]`
--> $DIR/self_assignment.rs:17:5
|
LL | s.c[0][1] = s.c[0][1];
| ^^^^^^^^^^^^^^^^^^^^^

error: self-assignment of `s.b[a]` to `s.b[a]`
--> $DIR/self_assignment.rs:18:5
|
LL | s.b[a] = s.b[a];
| ^^^^^^^^^^^^^^^

error: self-assignment of `*s.e` to `*s.e`
--> $DIR/self_assignment.rs:19:5
|
LL | *s.e = *s.e;
| ^^^^^^^^^^^

error: self-assignment of `s.b[10 + a]` to `s.b[a + 10]`
--> $DIR/self_assignment.rs:20:5
|
LL | s.b[a + 10] = s.b[10 + a];
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: self-assignment of `t.1` to `t.1`
--> $DIR/self_assignment.rs:23:5
|
LL | t.1 = t.1;
| ^^^^^^^^^

error: self-assignment of `(t.0)` to `t.0`
--> $DIR/self_assignment.rs:24:5
|
LL | t.0 = (t.0);
| ^^^^^^^^^^^

error: aborting due to 11 previous errors