From 615b8b753efe920c566987cb718fd10241e1a9a5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 12 Apr 2024 14:45:23 -0400 Subject: [PATCH] Limit commutative non-augmented-assignments to trivial data types --- .../fixtures/pylint/non_augmented_assignment.py | 1 + .../pylint/rules/non_augmented_assignment.rs | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/non_augmented_assignment.py b/crates/ruff_linter/resources/test/fixtures/pylint/non_augmented_assignment.py index 3c3a6778eb4d35..af6dbfaaa030a8 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/non_augmented_assignment.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/non_augmented_assignment.py @@ -55,3 +55,4 @@ def t(self): index = a_number = a_number + 1 a_number = index = a_number + 1 index = index * index + 10 +some_string = "a very long start to the string" + some_string diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_augmented_assignment.rs b/crates/ruff_linter/src/rules/pylint/rules/non_augmented_assignment.rs index f00af88cd78a22..fa73252872ef77 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/non_augmented_assignment.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/non_augmented_assignment.rs @@ -22,6 +22,18 @@ use crate::checkers::ast::Checker; /// When performing such an operation, augmented assignments are more concise /// and idiomatic. /// +/// ## Known problems +/// In some cases, this rule will not detect assignments in which the target +/// is on the right-hand side of a binary operation (e.g., `x = y + x`, as +/// opposed to `x = x + y`), as such operations are not commutative for +/// certain data types, like strings. +/// +/// For example, `x = "prefix-" + x` is not equivalent to `x += "prefix-"`, +/// while `x = 1 + x` is equivalent to `x += 1`. +/// +/// If the type of the left-hand side cannot be inferred trivially, the rule +/// will ignore the assignment. +/// /// ## Example /// ```python /// x = x + 1 @@ -101,8 +113,10 @@ pub(crate) fn non_augmented_assignment(checker: &mut Checker, assign: &ast::Stmt return; } - // If the operator is commutative, match, e.g., `x = 1 + x`. + // If the operator is commutative, match, e.g., `x = 1 + x`, but limit such matches to primitive + // types. if operator.is_commutative() + && (value.left.is_number_literal_expr() || value.left.is_boolean_literal_expr()) && ComparableExpr::from(target) == ComparableExpr::from(&value.right) { let mut diagnostic = Diagnostic::new(NonAugmentedAssignment { operator }, assign.range());