Detect items that hash to same value in duplicate sets (#14064)

## Summary

Like https://github.com/astral-sh/ruff/pull/14063, but ensures that we
catch cases like `{1, True}` in which the items hash to the same value
despite not being identical.
This commit is contained in:
Charlie Marsh 2024-11-03 13:49:11 -05:00 committed by GitHub
parent e00594e8d2
commit 66872a41fc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 38 additions and 10 deletions

View file

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

View file

@ -1,10 +1,11 @@
use anyhow::{Context, Result};
use rustc_hash::FxHashSet;
use rustc_hash::FxHashMap;
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::comparable::HashableExpr;
use ruff_python_ast::Expr;
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::Ranged;
@ -30,6 +31,7 @@ use crate::checkers::ast::Checker;
#[violation]
pub struct DuplicateValue {
value: String,
existing: String,
}
impl Violation for DuplicateValue {
@ -37,8 +39,14 @@ impl Violation for DuplicateValue {
#[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 +56,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(),
);

View file

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