OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #5894 - tmiasko:self-assignment, r=Manishearth

Browse Source 
Warn about explicit self-assignment

Warn about assignments where left-hand side place expression is the same
as right-hand side value expression. For example, warn about assignment in:

```rust
pub struct Event {
    id: usize,
    x: i32,
    y: i32,
}

pub fn copy_position(a: &mut Event, b: &Event) {
    a.x = b.x;
    a.y = a.y;
}
```

changelog: New lint `self_assignment`, checks for explicit self-assignments.
This commit is contained in:
bors 2020-08-16 21:56:37 +00:00
commit 8332fe81d8
17 changed files with 257 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -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
clippy_lints/src/assign_ops.rs
View File

@ -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
clippy_lints/src/booleans.rs
View File

@ -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
clippy_lints/src/copies.rs
View File

@ -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
clippy_lints/src/double_comparison.rs
View File

@ -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
clippy_lints/src/eq_op.rs
View File

@ -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
clippy_lints/src/floating_point_arithmetic.rs
View File

@ -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,8 +515,8 @@ 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 {
@ -524,10 +524,6 @@ fn is_testing_negative(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -
}
}
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
clippy_lints/src/lib.rs
View File

@ -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
clippy_lints/src/question_mark.rs
View File

@ -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
clippy_lints/src/self_assignment.rs Normal file
View File

@ -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
clippy_lints/src/swap.rs
View File

@ -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
clippy_lints/src/utils/hir_utils.rs
View File

@ -23,9 +23,7 @@ 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> {
@ -33,13 +31,14 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
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
clippy_lints/src/utils/internal_lints.rs
View File

@ -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
clippy_lints/src/utils/mod.rs
View File

@ -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
src/lintlist/mod.rs
View File

@ -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
tests/ui/self_assignment.rs Normal file
View File

@ -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
tests/ui/self_assignment.stderr Normal file
View File

@ -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