Remove HashableExpr abstraction (#14057)

## Summary

It looks like `ComparableExpr` now implements `Hash` so we can just
remove this.
This commit is contained in:
Charlie Marsh 2024-11-02 16:28:35 -04:00 committed by GitHub
parent 35c6dfe481
commit 4a3eeeff86
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 11 additions and 53 deletions

View file

@ -5,11 +5,10 @@ use ast::ExprContext;
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::hashable::HashableExpr;
use ruff_python_ast::helpers::{any_over_expr, contains_effect};
use ruff_python_ast::{self as ast, BoolOp, CmpOp, Expr};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::{Ranged, TextRange, TextSize};
use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;
@ -71,7 +70,7 @@ impl AlwaysFixableViolation for RepeatedEqualityComparison {
/// PLR1714
pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast::ExprBoolOp) {
// Map from expression hash to (starting offset, number of comparisons, list
let mut value_to_comparators: FxHashMap<HashableExpr, (TextSize, Vec<&Expr>, Vec<usize>)> =
let mut value_to_comparators: FxHashMap<ComparableExpr, (&Expr, Vec<&Expr>, Vec<usize>)> =
FxHashMap::with_capacity_and_hasher(bool_op.values.len() * 2, FxBuildHasher);
for (i, value) in bool_op.values.iter().enumerate() {
@ -82,7 +81,7 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
if matches!(left, Expr::Name(_) | Expr::Attribute(_)) {
let (_, left_matches, index_matches) = value_to_comparators
.entry(left.into())
.or_insert_with(|| (left.start(), Vec::new(), Vec::new()));
.or_insert_with(|| (left, Vec::new(), Vec::new()));
left_matches.push(right);
index_matches.push(i);
}
@ -90,15 +89,15 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
if matches!(right, Expr::Name(_) | Expr::Attribute(_)) {
let (_, right_matches, index_matches) = value_to_comparators
.entry(right.into())
.or_insert_with(|| (right.start(), Vec::new(), Vec::new()));
.or_insert_with(|| (right, Vec::new(), Vec::new()));
right_matches.push(left);
index_matches.push(i);
}
}
for (value, (_, comparators, indices)) in value_to_comparators
.iter()
.sorted_by_key(|(_, (start, _, _))| *start)
for (expr, comparators, indices) in value_to_comparators
.into_values()
.sorted_by_key(|(expr, _, _)| expr.start())
{
// If there's only one comparison, there's nothing to merge.
if comparators.len() == 1 {
@ -112,9 +111,9 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
if last.is_some_and(|last| last + 1 == *index) {
let (indices, comparators) = sequences.last_mut().unwrap();
indices.push(*index);
comparators.push(*comparator);
comparators.push(comparator);
} else {
sequences.push((vec![*index], vec![*comparator]));
sequences.push((vec![*index], vec![comparator]));
}
last = Some(*index);
}
@ -127,7 +126,7 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
let mut diagnostic = Diagnostic::new(
RepeatedEqualityComparison {
expression: SourceCodeSnippet::new(merged_membership_test(
value.as_expr(),
expr,
bool_op.op,
&comparators,
checker.locator(),
@ -148,7 +147,7 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
op: bool_op.op,
values: before
.chain(std::iter::once(Expr::Compare(ast::ExprCompare {
left: Box::new(value.as_expr().clone()),
left: Box::new(expr.clone()),
ops: match bool_op.op {
BoolOp::Or => Box::from([CmpOp::In]),
BoolOp::And => Box::from([CmpOp::NotIn]),