diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2ac06b360bea..b2b67c13aec4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -708,7 +708,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(fallible_impl_from::FallibleImplFrom)); store.register_late_pass(move |_| Box::new(question_mark::QuestionMark::new(conf))); store.register_late_pass(|_| Box::new(question_mark_used::QuestionMarkUsed)); - store.register_early_pass(|| Box::new(suspicious_operation_groupings::SuspiciousOperationGroupings)); + store.register_early_pass(|| Box::new(suspicious_operation_groupings::SuspiciousOperationGroupings::new())); store.register_late_pass(|_| Box::new(suspicious_trait_impl::SuspiciousImpl)); store.register_late_pass(|_| Box::new(map_unit_fn::MapUnit)); store.register_late_pass(|_| Box::new(inherent_impl::MultipleInherentImpl)); diff --git a/clippy_lints/src/suspicious_operation_groupings.rs b/clippy_lints/src/suspicious_operation_groupings.rs index 1c1de805db0c..b338fb6883a9 100644 --- a/clippy_lints/src/suspicious_operation_groupings.rs +++ b/clippy_lints/src/suspicious_operation_groupings.rs @@ -6,7 +6,7 @@ use rustc_ast::ast::{BinOpKind, Expr, ExprKind, StmtKind}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_session::declare_lint_pass; +use rustc_session::impl_lint_pass; use rustc_span::source_map::Spanned; use rustc_span::symbol::Ident; use rustc_span::Span; @@ -64,7 +64,21 @@ declare_clippy_lint! { "groupings of binary operations that look suspiciously like typos" } -declare_lint_pass!(SuspiciousOperationGroupings => [SUSPICIOUS_OPERATION_GROUPINGS]); +pub struct SuspiciousOperationGroupings { + // We need to keep track of which spans we've already linted, so we don't + // emit the same lint multiple times. + seen: FxHashSet, +} + +impl_lint_pass!(SuspiciousOperationGroupings => [SUSPICIOUS_OPERATION_GROUPINGS]); + +impl SuspiciousOperationGroupings { + pub fn new() -> Self { + Self { + seen: FxHashSet::default(), + } + } +} impl EarlyLintPass for SuspiciousOperationGroupings { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { @@ -73,7 +87,7 @@ impl EarlyLintPass for SuspiciousOperationGroupings { } if let Some(binops) = extract_related_binops(&expr.kind) { - check_binops(cx, &binops.iter().collect::>()); + check_binops(cx, &binops.iter().collect::>(), &mut self.seen); let mut op_types = Vec::with_capacity(binops.len()); // We could use a hashmap, etc. to avoid being O(n*m) here, but @@ -89,13 +103,13 @@ impl EarlyLintPass for SuspiciousOperationGroupings { for op_type in op_types { let ops: Vec<_> = binops.iter().filter(|b| b.op == op_type).collect(); - check_binops(cx, &ops); + check_binops(cx, &ops, &mut self.seen); } } } } -fn check_binops(cx: &EarlyContext<'_>, binops: &[&BinaryOp<'_>]) { +fn check_binops(cx: &EarlyContext<'_>, binops: &[&BinaryOp<'_>], seen: &mut FxHashSet) { let binop_count = binops.len(); if binop_count < 2 { // Single binary operation expressions would likely be false @@ -152,7 +166,7 @@ fn check_binops(cx: &EarlyContext<'_>, binops: &[&BinaryOp<'_>]) { if let Some(expected_loc) = expected_ident_loc { match (no_difference_info, double_difference_info) { - (Some(i), None) => attempt_to_emit_no_difference_lint(cx, binops, i, expected_loc), + (Some(i), None) => attempt_to_emit_no_difference_lint(cx, binops, i, expected_loc, seen), (None, Some((double_difference_index, ident_loc1, ident_loc2))) => { if one_ident_difference_count == binop_count - 1 && let Some(binop) = binops.get(double_difference_index) @@ -168,6 +182,7 @@ fn check_binops(cx: &EarlyContext<'_>, binops: &[&BinaryOp<'_>]) { }; if let Some(sugg) = ident_swap_sugg(cx, &paired_identifiers, binop, changed_loc, &mut applicability) + && seen.insert(binop.span) { emit_suggestion(cx, binop.span, sugg, applicability); } @@ -183,6 +198,7 @@ fn attempt_to_emit_no_difference_lint( binops: &[&BinaryOp<'_>], i: usize, expected_loc: IdentLocation, + seen: &mut FxHashSet, ) { if let Some(binop) = binops.get(i).copied() { // We need to try and figure out which identifier we should @@ -203,6 +219,7 @@ fn attempt_to_emit_no_difference_lint( && old_ident != new_ident && let Some(sugg) = suggestion_with_swapped_ident(cx, binop.left, expected_loc, new_ident, &mut applicability) + && seen.insert(binop.span) { emit_suggestion( cx, @@ -217,6 +234,7 @@ fn attempt_to_emit_no_difference_lint( && old_ident != new_ident && let Some(sugg) = suggestion_with_swapped_ident(cx, binop.right, expected_loc, new_ident, &mut applicability) + && seen.insert(binop.span) { emit_suggestion( cx, diff --git a/tests/ui/suspicious_operation_groupings.fixed b/tests/ui/suspicious_operation_groupings.fixed index 60fde6e22cb6..9d9732307c85 100644 --- a/tests/ui/suspicious_operation_groupings.fixed +++ b/tests/ui/suspicious_operation_groupings.fixed @@ -1,5 +1,3 @@ -//@compile-flags: -Zdeduplicate-diagnostics=yes - #![warn(clippy::suspicious_operation_groupings)] #![allow(dead_code, unused_parens, clippy::eq_op)] diff --git a/tests/ui/suspicious_operation_groupings.rs b/tests/ui/suspicious_operation_groupings.rs index ce37148a853b..201b8e657f4b 100644 --- a/tests/ui/suspicious_operation_groupings.rs +++ b/tests/ui/suspicious_operation_groupings.rs @@ -1,5 +1,3 @@ -//@compile-flags: -Zdeduplicate-diagnostics=yes - #![warn(clippy::suspicious_operation_groupings)] #![allow(dead_code, unused_parens, clippy::eq_op)] diff --git a/tests/ui/suspicious_operation_groupings.stderr b/tests/ui/suspicious_operation_groupings.stderr index 7cb066d57e75..82226d083adf 100644 --- a/tests/ui/suspicious_operation_groupings.stderr +++ b/tests/ui/suspicious_operation_groupings.stderr @@ -1,5 +1,5 @@ error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:17:9 + --> tests/ui/suspicious_operation_groupings.rs:15:9 | LL | self.x == other.y && self.y == other.y && self.z == other.z | ^^^^^^^^^^^^^^^^^ help: did you mean: `self.x == other.x` @@ -8,154 +8,142 @@ LL | self.x == other.y && self.y == other.y && self.z == other.z = help: to override `-D warnings` add `#[allow(clippy::suspicious_operation_groupings)]` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:30:20 + --> tests/ui/suspicious_operation_groupings.rs:28:20 | LL | s1.a < s2.a && s1.a < s2.b | ^^^^^^^^^^^ help: did you mean: `s1.b < s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:78:33 + --> tests/ui/suspicious_operation_groupings.rs:76:33 | LL | s1.a * s2.a + s1.b * s2.b + s1.c * s2.b + s1.d * s2.d | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:83:19 + --> tests/ui/suspicious_operation_groupings.rs:81:19 | LL | s1.a * s2.a + s1.b * s2.c + s1.c * s2.c | ^^^^^^^^^^^ help: did you mean: `s1.b * s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:83:19 - | -LL | s1.a * s2.a + s1.b * s2.c + s1.c * s2.c - | ^^^^^^^^^^^ help: did you mean: `s1.b * s2.b` - -error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:88:19 + --> tests/ui/suspicious_operation_groupings.rs:86:19 | LL | s1.a * s2.a + s2.b * s2.b + s1.c * s2.c | ^^^^^^^^^^^ help: did you mean: `s1.b * s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:93:19 + --> tests/ui/suspicious_operation_groupings.rs:91:19 | LL | s1.a * s2.a + s1.b * s1.b + s1.c * s2.c | ^^^^^^^^^^^ help: did you mean: `s1.b * s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:98:5 + --> tests/ui/suspicious_operation_groupings.rs:96:5 | LL | s1.a * s1.a + s1.b * s2.b + s1.c * s2.c | ^^^^^^^^^^^ help: did you mean: `s1.a * s2.a` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:103:33 + --> tests/ui/suspicious_operation_groupings.rs:101:33 | LL | s1.a * s2.a + s1.b * s2.b + s1.c * s1.c | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:116:20 + --> tests/ui/suspicious_operation_groupings.rs:114:20 | LL | (s1.a * s2.a + s1.b * s1.b) | ^^^^^^^^^^^ help: did you mean: `s1.b * s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:121:34 + --> tests/ui/suspicious_operation_groupings.rs:119:34 | LL | (s1.a * s2.a + s1.b * s2.b + s1.c * s2.b + s1.d * s2.d) | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:126:38 + --> tests/ui/suspicious_operation_groupings.rs:124:38 | LL | (s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d) | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:131:39 + --> tests/ui/suspicious_operation_groupings.rs:129:39 | LL | ((s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d)) | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:136:42 - | -LL | (((s1.a * s2.a) + (s1.b * s2.b)) + ((s1.c * s2.b) + (s1.d * s2.d))) - | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` - -error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:136:42 + --> tests/ui/suspicious_operation_groupings.rs:134:42 | LL | (((s1.a * s2.a) + (s1.b * s2.b)) + ((s1.c * s2.b) + (s1.d * s2.d))) | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:141:40 + --> tests/ui/suspicious_operation_groupings.rs:139:40 | LL | (((s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b)) + (s1.d * s2.d)) | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:146:40 + --> tests/ui/suspicious_operation_groupings.rs:144:40 | LL | ((s1.a * s2.a) + ((s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d))) | ^^^^^^^^^^^ help: did you mean: `s1.c * s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:151:20 + --> tests/ui/suspicious_operation_groupings.rs:149:20 | LL | (s1.a * s2.a + s2.b * s2.b) / 2 | ^^^^^^^^^^^ help: did you mean: `s1.b * s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:156:35 + --> tests/ui/suspicious_operation_groupings.rs:154:35 | LL | i32::swap_bytes(s1.a * s2.a + s2.b * s2.b) | ^^^^^^^^^^^ help: did you mean: `s1.b * s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:161:29 + --> tests/ui/suspicious_operation_groupings.rs:159:29 | LL | s1.a > 0 && s1.b > 0 && s1.d == s2.c && s1.d == s2.d | ^^^^^^^^^^^^ help: did you mean: `s1.c == s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:166:17 + --> tests/ui/suspicious_operation_groupings.rs:164:17 | LL | s1.a > 0 && s1.d == s2.c && s1.b > 0 && s1.d == s2.d | ^^^^^^^^^^^^ help: did you mean: `s1.c == s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:175:77 + --> tests/ui/suspicious_operation_groupings.rs:173:77 | LL | (n1.inner.0).0 == (n2.inner.0).0 && (n1.inner.1).0 == (n2.inner.1).0 && (n1.inner.2).0 == (n2.inner.1).0 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `(n1.inner.2).0 == (n2.inner.2).0` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:189:25 + --> tests/ui/suspicious_operation_groupings.rs:187:25 | LL | s1.a <= s2.a && s1.a <= s2.b | ^^^^^^^^^^^^ help: did you mean: `s1.b <= s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:195:23 + --> tests/ui/suspicious_operation_groupings.rs:193:23 | LL | if s1.a < s2.a && s1.a < s2.b { | ^^^^^^^^^^^ help: did you mean: `s1.b < s2.b` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:202:48 + --> tests/ui/suspicious_operation_groupings.rs:200:48 | LL | -(-(-s1.a * -s2.a) + (-(-s1.b * -s2.b) + -(-s1.c * -s2.b) + -(-s1.d * -s2.d))) | ^^^^^^^^^^^^^ help: did you mean: `-s1.c * -s2.c` error: this sequence of operators looks suspiciously like a bug - --> tests/ui/suspicious_operation_groupings.rs:207:27 + --> tests/ui/suspicious_operation_groupings.rs:205:27 | LL | -(if -s1.a < -s2.a && -s1.a < -s2.b { s1.c } else { s2.a }) | ^^^^^^^^^^^^^ help: did you mean: `-s1.b < -s2.b` -error: aborting due to 26 previous errors +error: aborting due to 24 previous errors