From 4f4abf4e0640edbb1614f3dcb8ff62e8afc54801 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= <tomasz.miasko@gmail.com>
Date: Sun, 16 Aug 2020 00:00:00 +0000
Subject: [PATCH] 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.md | 1 +
clippy_lints/src/lib.rs | 5 +++
clippy_lints/src/self_assignment.rs | 51 +++++++++++++++++++++
src/lintlist/mod.rs | 7 +++
tests/ui/self_assignment.rs | 67 +++++++++++++++++++++++++++
tests/ui/self_assignment.stderr | 70 +++++++++++++++++++++++++++++
6 files changed, 201 insertions(+)
create mode 100644 clippy_lints/src/self_assignment.rs
create mode 100644 tests/ui/self_assignment.rs
create mode 100644 tests/ui/self_assignment.stderr
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f662de122f9..5ce63c0a157 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 17501e8e6da..87c297e72eb 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -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(®ex::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),
diff --git a/clippy_lints/src/self_assignment.rs b/clippy_lints/src/self_assignment.rs
new file mode 100644
index 00000000000..e096c9aebc1
--- /dev/null
+++ b/clippy_lints/src/self_assignment.rs
@@ -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),
+ );
+ }
+ }
+ }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 3229c8da507..bf58c117aaa 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -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",
diff --git a/tests/ui/self_assignment.rs b/tests/ui/self_assignment.rs
new file mode 100644
index 00000000000..a7cbb9cd78b
--- /dev/null
+++ b/tests/ui/self_assignment.rs
@@ -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() {}
diff --git a/tests/ui/self_assignment.stderr b/tests/ui/self_assignment.stderr
new file mode 100644
index 00000000000..826e0d0ba88
--- /dev/null
+++ b/tests/ui/self_assignment.stderr
@@ -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
+