Skip to content

Commit

Permalink
Respect hash-equivalent literals in iteration-over-set
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 3, 2024
1 parent 443fd3b commit 44402e2
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,6 @@
for item in {1, 2, 2}: # duplicate literals will be ignored
# B033 catches this
print(f"I like {item}.")

for item in {False, 0, 0.0, 0j, True, 1, 1.0}:
print(item)
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use rustc_hash::{FxBuildHasher, FxHashSet};

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{comparable::ComparableExpr, Expr};
use ruff_python_ast::hashable::HashComparableExpr;
use ruff_python_ast::Expr;
use ruff_text_size::Ranged;
use rustc_hash::{FxBuildHasher, FxHashSet};

use crate::checkers::ast::Checker;

Expand Down Expand Up @@ -54,8 +56,7 @@ pub(crate) fn iteration_over_set(checker: &mut Checker, expr: &Expr) {

let mut seen_values = FxHashSet::with_capacity_and_hasher(set.len(), FxBuildHasher);
for value in set {
let comparable_value = ComparableExpr::from(value);
if !seen_values.insert(comparable_value) {
if !seen_values.insert(HashComparableExpr::from(value)) {
// if the set contains a duplicate literal value, early exit.
// rule `B033` can catch that.
return;
Expand Down
14 changes: 10 additions & 4 deletions crates/ruff_python_ast/src/comparable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,8 +861,14 @@ pub struct ExprNumberLiteral<'a> {
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub struct ExprBoolLiteral<'a> {
value: &'a bool,
pub struct ExprBoolLiteral {
value: bool,
}

impl From<bool> for ExprBoolLiteral {
fn from(value: bool) -> Self {
Self { value }
}
}

#[derive(Debug, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -934,7 +940,7 @@ pub enum ComparableExpr<'a> {
StringLiteral(ExprStringLiteral<'a>),
BytesLiteral(ExprBytesLiteral<'a>),
NumberLiteral(ExprNumberLiteral<'a>),
BoolLiteral(ExprBoolLiteral<'a>),
BoolLiteral(ExprBoolLiteral),
NoneLiteral,
EllipsisLiteral,
Attribute(ExprAttribute<'a>),
Expand Down Expand Up @@ -1109,7 +1115,7 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
})
}
ast::Expr::BooleanLiteral(ast::ExprBooleanLiteral { value, range: _ }) => {
Self::BoolLiteral(ExprBoolLiteral { value })
Self::BoolLiteral(ExprBoolLiteral { value: *value })
}
ast::Expr::NoneLiteral(_) => Self::NoneLiteral,
ast::Expr::EllipsisLiteral(_) => Self::EllipsisLiteral,
Expand Down
65 changes: 65 additions & 0 deletions crates/ruff_python_ast/src/hashable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use std::hash::Hash;

use crate::{Expr, Number};

use crate::comparable::{ComparableExpr, ExprBoolLiteral};

/// Wrapper around [`Expr`] that implements [`Hash`] and [`PartialEq`] according to Python
/// semantics:
///
/// > Values that compare equal (such as 1, 1.0, and True) can be used interchangeably to index the
/// > same dictionary entry.
///
/// For example, considers `True`, `1`, and `1.0` to be equal, as they hash to the same value
/// in Python, along with `False`, `0`, and `0.0`.
///
/// See: <https://docs.python.org/3/library/stdtypes.html#mapping-types-dict>
pub struct HashComparableExpr<'a>(ComparableExpr<'a>);

impl Hash for HashComparableExpr<'_> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.0.hash(state);
}
}

impl PartialEq<Self> for HashComparableExpr<'_> {
fn eq(&self, other: &Self) -> bool {
self.0 == other.0
}
}

impl Eq for HashComparableExpr<'_> {}

impl<'a> From<&'a Expr> for HashComparableExpr<'a> {
fn from(expr: &'a Expr) -> Self {
as_bool(expr)
.map(ExprBoolLiteral::from)
.map(ComparableExpr::BoolLiteral)
.map(Self)
.unwrap_or_else(|| Self(ComparableExpr::from(expr)))
}
}

/// Returns the `bool` value of the given expression, if it has an equivalent hash to `True` or `False`.
fn as_bool(expr: &Expr) -> Option<bool> {
let Expr::NumberLiteral(expr) = expr else {
return None;
};
match &expr.value {
Number::Int(int) => match int.as_u8() {
Some(0) => Some(false),
Some(1) => Some(true),
_ => None,
},
Number::Float(float) => match float {
0.0 => Some(false),
1.0 => Some(true),
_ => None,
},
Number::Complex { real, imag } => match (real, imag) {
(0.0, 0.0) => Some(false),
(1.0, 0.0) => Some(true),
_ => None,
},
}
}
1 change: 1 addition & 0 deletions crates/ruff_python_ast/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub use nodes::*;
pub mod comparable;
pub mod docstrings;
mod expression;
pub mod hashable;
pub mod helpers;
pub mod identifier;
mod int;
Expand Down

0 comments on commit 44402e2

Please sign in to comment.