Detect items that hash to same value in duplicate sets

Author: charliermarsh

crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B033.py

@@ -19,6 +19,7 @@
     1,
     1,
 }
+incorrect_set = {False, 1, 0}
 
 ###
 # Non-errors.

crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_value.rs

@@ -1,10 +1,12 @@
 use anyhow::{Context, Result};
-use rustc_hash::FxHashSet;
+use rustc_hash::{FxHashMap, FxHashSet};
 
 use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast as ast;
 use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::hashable::HashableExpr;
+use ruff_python_ast::Expr;
 use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
 use ruff_text_size::Ranged;
 
@@ -30,15 +32,22 @@ use crate::checkers::ast::Checker;
 #[violation]
 pub struct DuplicateValue {
     value: String,
+    existing: String,
 }
 
 impl Violation for DuplicateValue {
     const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
 
     #[derive_message_formats]
     fn message(&self) -> String {
-        let DuplicateValue { value } = self;
-        format!("Sets should not contain duplicate item `{value}`")
+        let DuplicateValue { value, existing } = self;
+        if value == existing {
+            format!("Sets should not contain duplicate item `{value}`")
+        } else {
+            format!(
+                "Sets should not contain duplicate items, but `{existing}` and `{value}` has the same value"
+            )
+        }
     }
 
     fn fix_title(&self) -> Option<String> {
@@ -48,15 +57,14 @@ impl Violation for DuplicateValue {
 
 /// B033
 pub(crate) fn duplicate_value(checker: &mut Checker, set: &ast::ExprSet) {
-    let mut seen_values: FxHashSet<ComparableExpr> = FxHashSet::default();
+    let mut seen_values: FxHashMap<HashableExpr, &Expr> = FxHashMap::default();
     for (index, value) in set.iter().enumerate() {
         if value.is_literal_expr() {
-            let comparable_value = ComparableExpr::from(value);
-
-            if !seen_values.insert(comparable_value) {
+            if let Some(existing) = seen_values.insert(HashableExpr::from(value), value) {
                 let mut diagnostic = Diagnostic::new(
                     DuplicateValue {
                         value: checker.generator().expr(value),
+                        existing: checker.generator().expr(existing),
                     },
                     value.range(),
                 );

crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B033_B033.py.snap

@@ -154,6 +154,7 @@ B033.py:20:5: B033 [*] Sets should not contain duplicate item `1`
 20 |     1,
    |     ^ B033
 21 | }
+22 | incorrect_set = {False, 1, 0}
    |
    = help: Remove duplicate item
 
@@ -163,7 +164,26 @@ B033.py:20:5: B033 [*] Sets should not contain duplicate item `1`
 19 19 |     1,
 20    |-    1,
 21 20 | }
-22 21 | 
-23 22 | ###
+22 21 | incorrect_set = {False, 1, 0}
+23 22 | 
 
+B033.py:22:28: B033 [*] Sets should not contain duplicate items, but `False` and `0` has the same value
+   |
+20 |     1,
+21 | }
+22 | incorrect_set = {False, 1, 0}
+   |                            ^ B033
+23 | 
+24 | ###
+   |
+   = help: Remove duplicate item
 
+ℹ Safe fix
+19 19 |     1,
+20 20 |     1,
+21 21 | }
+22    |-incorrect_set = {False, 1, 0}
+   22 |+incorrect_set = {False, 1}
+23 23 | 
+24 24 | ###
+25 25 | # Non-errors.