Skip to content

Commit

Permalink
Avoid non-augmented-assignment for reversed, non-commutative operat…
Browse files Browse the repository at this point in the history
…ors (#10909)

Closes #10900.
  • Loading branch information
charliermarsh committed Apr 12, 2024
1 parent a013050 commit e9870fe
Show file tree
Hide file tree
Showing 3 changed files with 340 additions and 313 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
a_list = a_list + ["to concat"]
some_set = some_set | {"to concat"}
to_multiply = to_multiply * 5
to_multiply = 5 * to_multiply
to_multiply = to_multiply * to_multiply
to_divide = to_divide / 5
to_divide = to_divide // 5
to_cube = to_cube**3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,11 @@ pub(crate) fn non_augmented_assignment(checker: &mut Checker, assign: &ast::Stmt
return;
};

let operator = AugmentedOperator::from(value.op);

// Match, e.g., `x = x + 1`.
if ComparableExpr::from(target) == ComparableExpr::from(&value.left) {
let mut diagnostic = Diagnostic::new(
NonAugmentedAssignment {
operator: AugmentedOperator::from(value.op),
},
assign.range(),
);
let mut diagnostic = Diagnostic::new(NonAugmentedAssignment { operator }, assign.range());
diagnostic.set_fix(Fix::unsafe_edit(augmented_assignment(
checker.generator(),
target,
Expand All @@ -104,14 +101,11 @@ pub(crate) fn non_augmented_assignment(checker: &mut Checker, assign: &ast::Stmt
return;
}

// Match, e.g., `x = 1 + x`.
if ComparableExpr::from(target) == ComparableExpr::from(&value.right) {
let mut diagnostic = Diagnostic::new(
NonAugmentedAssignment {
operator: AugmentedOperator::from(value.op),
},
assign.range(),
);
// If the operator is commutative, match, e.g., `x = 1 + x`.
if operator.is_commutative()
&& ComparableExpr::from(target) == ComparableExpr::from(&value.right)
{
let mut diagnostic = Diagnostic::new(NonAugmentedAssignment { operator }, assign.range());
diagnostic.set_fix(Fix::unsafe_edit(augmented_assignment(
checker.generator(),
target,
Expand Down Expand Up @@ -161,6 +155,16 @@ enum AugmentedOperator {
Sub,
}

impl AugmentedOperator {
/// Returns `true` if the operator is commutative.
fn is_commutative(self) -> bool {
matches!(
self,
Self::Add | Self::BitAnd | Self::BitOr | Self::BitXor | Self::Mult
)
}
}

impl From<Operator> for AugmentedOperator {
fn from(value: Operator) -> Self {
match value {
Expand Down

0 comments on commit e9870fe

Please sign in to comment.