Skip to content

Commit a9e4393

Browse files
authored
[pylint] Implement rule to prefer augmented assignment (PLR6104) (#9932)
## Summary Implement new rule: Prefer augmented assignment (#8877). It checks for the assignment statement with the form of `<expr> = <expr> <binary-operator> …` with a unsafe fix to use augmented assignment instead. ## Test Plan 1. Snapshot test is included in the PR. 2. Manually test with playground.
1 parent 312f434 commit a9e4393

File tree

8 files changed

+785
-0
lines changed

8 files changed

+785
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Errors
2+
some_string = "some string"
3+
index, a_number, to_multiply, to_divide, to_cube, timeDiffSeconds, flags = (
4+
0,
5+
1,
6+
2,
7+
3,
8+
4,
9+
5,
10+
0x3,
11+
)
12+
a_list = [1, 2]
13+
some_set = {"elem"}
14+
mat1, mat2 = None, None
15+
16+
some_string = some_string + "a very long end of string"
17+
index = index - 1
18+
a_list = a_list + ["to concat"]
19+
some_set = some_set | {"to concat"}
20+
to_multiply = to_multiply * 5
21+
to_divide = to_divide / 5
22+
to_divide = to_divide // 5
23+
to_cube = to_cube**3
24+
to_cube = 3**to_cube
25+
to_cube = to_cube**to_cube
26+
timeDiffSeconds = timeDiffSeconds % 60
27+
flags = flags & 0x1
28+
flags = flags | 0x1
29+
flags = flags ^ 0x1
30+
flags = flags << 1
31+
flags = flags >> 1
32+
mat1 = mat1 @ mat2
33+
a_list[1] = a_list[1] + 1
34+
35+
a_list[0:2] = a_list[0:2] * 3
36+
a_list[:2] = a_list[:2] * 3
37+
a_list[1:] = a_list[1:] * 3
38+
a_list[:] = a_list[:] * 3
39+
40+
index = index * (index + 10)
41+
42+
43+
class T:
44+
def t(self):
45+
self.a = self.a + 1
46+
47+
48+
obj = T()
49+
obj.a = obj.a + 1
50+
51+
# OK
52+
a_list[0] = a_list[:] * 3
53+
index = a_number = a_number + 1
54+
a_number = index = a_number + 1
55+
index = index * index + 10

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
14791479
if checker.settings.rules.enabled(Rule::TypeBivariance) {
14801480
pylint::rules::type_bivariance(checker, value);
14811481
}
1482+
if checker.enabled(Rule::NonAugmentedAssignment) {
1483+
pylint::rules::non_augmented_assignment(checker, assign);
1484+
}
14821485
if checker.settings.rules.enabled(Rule::UnsortedDunderAll) {
14831486
ruff::rules::sort_dunder_all_assign(checker, assign);
14841487
}

crates/ruff_linter/src/codes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
293293
(Pylint, "R2004") => (RuleGroup::Stable, rules::pylint::rules::MagicValueComparison),
294294
(Pylint, "R2044") => (RuleGroup::Preview, rules::pylint::rules::EmptyComment),
295295
(Pylint, "R5501") => (RuleGroup::Stable, rules::pylint::rules::CollapsibleElseIf),
296+
(Pylint, "R6104") => (RuleGroup::Preview, rules::pylint::rules::NonAugmentedAssignment),
296297
(Pylint, "R6201") => (RuleGroup::Preview, rules::pylint::rules::LiteralMembership),
297298
#[allow(deprecated)]
298299
(Pylint, "R6301") => (RuleGroup::Nursery, rules::pylint::rules::NoSelfUse),

crates/ruff_linter/src/rules/pylint/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ mod tests {
186186
Rule::UnnecessaryDictIndexLookup,
187187
Path::new("unnecessary_dict_index_lookup.py")
188188
)]
189+
#[test_case(Rule::NonAugmentedAssignment, Path::new("non_augmented_assignment.py"))]
189190
#[test_case(
190191
Rule::UselessExceptionStatement,
191192
Path::new("useless_exception_statement.py")

crates/ruff_linter/src/rules/pylint/rules/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ pub(crate) use no_method_decorator::*;
4747
pub(crate) use no_self_use::*;
4848
pub(crate) use non_ascii_module_import::*;
4949
pub(crate) use non_ascii_name::*;
50+
pub(crate) use non_augmented_assignment::*;
5051
pub(crate) use non_slot_assignment::*;
5152
pub(crate) use nonlocal_and_global::*;
5253
pub(crate) use nonlocal_without_binding::*;
@@ -143,6 +144,7 @@ mod no_method_decorator;
143144
mod no_self_use;
144145
mod non_ascii_module_import;
145146
mod non_ascii_name;
147+
mod non_augmented_assignment;
146148
mod non_slot_assignment;
147149
mod nonlocal_and_global;
148150
mod nonlocal_without_binding;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
use ast::{Expr, StmtAugAssign};
2+
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
3+
use ruff_macros::{derive_message_formats, violation};
4+
use ruff_python_ast as ast;
5+
use ruff_python_ast::comparable::ComparableExpr;
6+
use ruff_python_ast::Operator;
7+
use ruff_python_codegen::Generator;
8+
use ruff_text_size::{Ranged, TextRange};
9+
10+
use crate::checkers::ast::Checker;
11+
12+
/// ## What it does
13+
/// Checks for assignments that can be replaced with augmented assignment
14+
/// statements.
15+
///
16+
/// ## Why is this bad?
17+
/// If an assignment statement consists of a binary operation in which one
18+
/// operand is the same as the assignment target, it can be rewritten as an
19+
/// augmented assignment. For example, `x = x + 1` can be rewritten as
20+
/// `x += 1`.
21+
///
22+
/// When performing such an operation, augmented assignments are more concise
23+
/// and idiomatic.
24+
///
25+
/// ## Example
26+
/// ```python
27+
/// x = x + 1
28+
/// ```
29+
///
30+
/// Use instead:
31+
/// ```python
32+
/// x += 1
33+
/// ```
34+
///
35+
/// ## Fix safety
36+
/// This rule's fix is marked as unsafe, as augmented assignments have
37+
/// different semantics when the target is a mutable data type, like a list or
38+
/// dictionary.
39+
///
40+
/// For example, consider the following:
41+
///
42+
/// ```python
43+
/// foo = [1]
44+
/// bar = foo
45+
/// foo = foo + [2]
46+
/// assert (foo, bar) == ([1, 2], [1])
47+
/// ```
48+
///
49+
/// If the assignment is replaced with an augmented assignment, the update
50+
/// operation will apply to both `foo` and `bar`, as they refer to the same
51+
/// object:
52+
///
53+
/// ```python
54+
/// foo = [1]
55+
/// bar = foo
56+
/// foo += [2]
57+
/// assert (foo, bar) == ([1, 2], [1, 2])
58+
/// ```
59+
#[violation]
60+
pub struct NonAugmentedAssignment {
61+
operator: AugmentedOperator,
62+
}
63+
64+
impl AlwaysFixableViolation for NonAugmentedAssignment {
65+
#[derive_message_formats]
66+
fn message(&self) -> String {
67+
let NonAugmentedAssignment { operator } = self;
68+
format!("Use `{operator}` to perform an augmented assignment directly")
69+
}
70+
71+
fn fix_title(&self) -> String {
72+
"Replace with augmented assignment".to_string()
73+
}
74+
}
75+
76+
/// PLR6104
77+
pub(crate) fn non_augmented_assignment(checker: &mut Checker, assign: &ast::StmtAssign) {
78+
// Ignore multiple assignment targets.
79+
let [target] = assign.targets.as_slice() else {
80+
return;
81+
};
82+
83+
// Match, e.g., `x = x + 1`.
84+
let Expr::BinOp(value) = &*assign.value else {
85+
return;
86+
};
87+
88+
// Match, e.g., `x = x + 1`.
89+
if ComparableExpr::from(target) == ComparableExpr::from(&value.left) {
90+
let mut diagnostic = Diagnostic::new(
91+
NonAugmentedAssignment {
92+
operator: AugmentedOperator::from(value.op),
93+
},
94+
assign.range(),
95+
);
96+
diagnostic.set_fix(Fix::unsafe_edit(augmented_assignment(
97+
checker.generator(),
98+
target,
99+
value.op,
100+
&value.right,
101+
assign.range(),
102+
)));
103+
checker.diagnostics.push(diagnostic);
104+
return;
105+
}
106+
107+
// Match, e.g., `x = 1 + x`.
108+
if ComparableExpr::from(target) == ComparableExpr::from(&value.right) {
109+
let mut diagnostic = Diagnostic::new(
110+
NonAugmentedAssignment {
111+
operator: AugmentedOperator::from(value.op),
112+
},
113+
assign.range(),
114+
);
115+
diagnostic.set_fix(Fix::unsafe_edit(augmented_assignment(
116+
checker.generator(),
117+
target,
118+
value.op,
119+
&value.left,
120+
assign.range(),
121+
)));
122+
checker.diagnostics.push(diagnostic);
123+
}
124+
}
125+
126+
/// Generate a fix to convert an assignment statement to an augmented assignment.
127+
///
128+
/// For example, given `x = x + 1`, the fix would be `x += 1`.
129+
fn augmented_assignment(
130+
generator: Generator,
131+
target: &Expr,
132+
operator: Operator,
133+
right_operand: &Expr,
134+
range: TextRange,
135+
) -> Edit {
136+
Edit::range_replacement(
137+
generator.stmt(&ast::Stmt::AugAssign(StmtAugAssign {
138+
range: TextRange::default(),
139+
target: Box::new(target.clone()),
140+
op: operator,
141+
value: Box::new(right_operand.clone()),
142+
})),
143+
range,
144+
)
145+
}
146+
147+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
148+
enum AugmentedOperator {
149+
Add,
150+
BitAnd,
151+
BitOr,
152+
BitXor,
153+
Div,
154+
FloorDiv,
155+
LShift,
156+
MatMult,
157+
Mod,
158+
Mult,
159+
Pow,
160+
RShift,
161+
Sub,
162+
}
163+
164+
impl From<Operator> for AugmentedOperator {
165+
fn from(value: Operator) -> Self {
166+
match value {
167+
Operator::Add => Self::Add,
168+
Operator::BitAnd => Self::BitAnd,
169+
Operator::BitOr => Self::BitOr,
170+
Operator::BitXor => Self::BitXor,
171+
Operator::Div => Self::Div,
172+
Operator::FloorDiv => Self::FloorDiv,
173+
Operator::LShift => Self::LShift,
174+
Operator::MatMult => Self::MatMult,
175+
Operator::Mod => Self::Mod,
176+
Operator::Mult => Self::Mult,
177+
Operator::Pow => Self::Pow,
178+
Operator::RShift => Self::RShift,
179+
Operator::Sub => Self::Sub,
180+
}
181+
}
182+
}
183+
184+
impl std::fmt::Display for AugmentedOperator {
185+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
186+
match self {
187+
Self::Add => f.write_str("+="),
188+
Self::BitAnd => f.write_str("&="),
189+
Self::BitOr => f.write_str("|="),
190+
Self::BitXor => f.write_str("^="),
191+
Self::Div => f.write_str("/="),
192+
Self::FloorDiv => f.write_str("//="),
193+
Self::LShift => f.write_str("<<="),
194+
Self::MatMult => f.write_str("@="),
195+
Self::Mod => f.write_str("%="),
196+
Self::Mult => f.write_str("*="),
197+
Self::Pow => f.write_str("**="),
198+
Self::RShift => f.write_str(">>="),
199+
Self::Sub => f.write_str("-="),
200+
}
201+
}
202+
}

0 commit comments

Comments
 (0)