Skip to content

Commit 16de4aa

Browse files
danparizherntBre
andauthored
[refurb] Auto-fix annotated assignments (FURB101) (#21278)
## Summary Fixed FURB101 (`read-whole-file`) to handle annotated assignments. Previously, the rule would detect violations in code like `contents: str = f.read()` but fail to generate a fix. Now it correctly generates fixes that preserve type annotations (e.g., `contents: str = Path("file.txt").read_text(encoding="utf-8")`). Fixes #21274 ## Problem Analysis The FURB101 rule was only checking for `Stmt::Assign` statements when determining whether a fix could be applied. When encountering annotated assignments (`Stmt::AnnAssign`) like `contents: str = f.read()`, the rule would: 1. Correctly detect the violation (the diagnostic was reported) 2. Fail to generate a fix because: - The `visit_expr` method only matched `Stmt::Assign`, not `Stmt::AnnAssign` - The `generate_fix` function only accepted `Stmt::Assign` in its body validation - The replacement code generation didn't account for type annotations This occurred because Python's AST represents annotated assignments as a different node type (`StmtAnnAssign`) with separate fields for the target, annotation, and value, unlike regular assignments which use a list of targets. ## Approach The fix extends the rule to handle both assignment types: 1. **Updated `visit_expr` method**: Now matches both `Stmt::Assign` and `Stmt::AnnAssign`, extracting: - Variable name from the target expression - Type annotation code (when present) using the code generator 2. **Updated `generate_fix` function**: - Added `annotation: Option<String>` parameter to accept annotation code - Updated body validation to accept both `Stmt::Assign` and `Stmt::AnnAssign` - Modified replacement code generation to preserve annotations: `{var}: {annotation} = {binding}({filename_code}).{suggestion}` 3. **Added test case**: Added an annotated assignment test case to verify the fix works correctly. The implementation maintains backward compatibility with regular assignments while adding support for annotated assignments, ensuring type annotations are preserved in the generated fixes. --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent e06e108 commit 16de4aa

File tree

3 files changed

+107
-18
lines changed

3 files changed

+107
-18
lines changed

crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,18 @@ def bar(x):
125125
# `buffering`.
126126
with open(*filename, file="file.txt", mode="r") as f:
127127
x = f.read()
128+
129+
# FURB101
130+
with open("file.txt", encoding="utf-8") as f:
131+
contents: str = f.read()
132+
133+
# FURB101 but no fix because it would remove the assignment to `x`
134+
with open("file.txt", encoding="utf-8") as f:
135+
contents, x = f.read(), 2
136+
137+
# FURB101 but no fix because it would remove the `process_contents` call
138+
with open("file.txt", encoding="utf-8") as f:
139+
contents = process_contents(f.read())
140+
141+
with open("file.txt", encoding="utf-8") as f:
142+
contents: str = process_contents(f.read())

crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -125,20 +125,8 @@ impl<'a> Visitor<'a> for ReadMatcher<'a, '_> {
125125
open.item.range(),
126126
);
127127

128-
let target = match self.with_stmt.body.first() {
129-
Some(Stmt::Assign(assign))
130-
if assign.value.range().contains_range(expr.range()) =>
131-
{
132-
match assign.targets.first() {
133-
Some(Expr::Name(name)) => Some(name.id.as_str()),
134-
_ => None,
135-
}
136-
}
137-
_ => None,
138-
};
139-
140128
if let Some(fix) =
141-
generate_fix(self.checker, &open, target, self.with_stmt, &suggestion)
129+
generate_fix(self.checker, &open, expr, self.with_stmt, &suggestion)
142130
{
143131
diagnostic.set_fix(fix);
144132
}
@@ -190,15 +178,16 @@ fn make_suggestion(open: &FileOpen<'_>, generator: Generator) -> String {
190178
fn generate_fix(
191179
checker: &Checker,
192180
open: &FileOpen,
193-
target: Option<&str>,
181+
expr: &Expr,
194182
with_stmt: &ast::StmtWith,
195183
suggestion: &str,
196184
) -> Option<Fix> {
197-
if !(with_stmt.items.len() == 1 && matches!(with_stmt.body.as_slice(), [Stmt::Assign(_)])) {
185+
if with_stmt.items.len() != 1 {
198186
return None;
199187
}
200188

201189
let locator = checker.locator();
190+
202191
let filename_code = locator.slice(open.filename.range());
203192

204193
let (import_edit, binding) = checker
@@ -210,9 +199,39 @@ fn generate_fix(
210199
)
211200
.ok()?;
212201

213-
let replacement = match target {
214-
Some(var) => format!("{var} = {binding}({filename_code}).{suggestion}"),
215-
None => format!("{binding}({filename_code}).{suggestion}"),
202+
// Only replace context managers with a single assignment or annotated assignment in the body.
203+
// The assignment's RHS must also be the same as the `read` call in `expr`, otherwise this fix
204+
// would remove the rest of the expression.
205+
let replacement = match with_stmt.body.as_slice() {
206+
[Stmt::Assign(ast::StmtAssign { targets, value, .. })] if value.range() == expr.range() => {
207+
match targets.as_slice() {
208+
[Expr::Name(name)] => {
209+
format!(
210+
"{name} = {binding}({filename_code}).{suggestion}",
211+
name = name.id
212+
)
213+
}
214+
_ => return None,
215+
}
216+
}
217+
[
218+
Stmt::AnnAssign(ast::StmtAnnAssign {
219+
target,
220+
annotation,
221+
value: Some(value),
222+
..
223+
}),
224+
] if value.range() == expr.range() => match target.as_ref() {
225+
Expr::Name(name) => {
226+
format!(
227+
"{var}: {ann} = {binding}({filename_code}).{suggestion}",
228+
var = name.id,
229+
ann = locator.slice(annotation.range())
230+
)
231+
}
232+
_ => return None,
233+
},
234+
_ => return None,
216235
};
217236

218237
let applicability = if checker.comment_ranges().intersects(with_stmt.range()) {

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,3 +189,58 @@ FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()`
189189
51 | # the user reads the whole file and that bit they can replace.
190190
|
191191
help: Replace with `Path("file.txt").read_text()`
192+
193+
FURB101 [*] `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
194+
--> FURB101.py:130:6
195+
|
196+
129 | # FURB101
197+
130 | with open("file.txt", encoding="utf-8") as f:
198+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
199+
131 | contents: str = f.read()
200+
|
201+
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
202+
1 + import pathlib
203+
2 | def foo():
204+
3 | ...
205+
4 |
206+
--------------------------------------------------------------------------------
207+
128 | x = f.read()
208+
129 |
209+
130 | # FURB101
210+
- with open("file.txt", encoding="utf-8") as f:
211+
- contents: str = f.read()
212+
131 + contents: str = pathlib.Path("file.txt").read_text(encoding="utf-8")
213+
132 |
214+
133 | # FURB101 but no fix because it would remove the assignment to `x`
215+
134 | with open("file.txt", encoding="utf-8") as f:
216+
217+
FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
218+
--> FURB101.py:134:6
219+
|
220+
133 | # FURB101 but no fix because it would remove the assignment to `x`
221+
134 | with open("file.txt", encoding="utf-8") as f:
222+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
223+
135 | contents, x = f.read(), 2
224+
|
225+
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
226+
227+
FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
228+
--> FURB101.py:138:6
229+
|
230+
137 | # FURB101 but no fix because it would remove the `process_contents` call
231+
138 | with open("file.txt", encoding="utf-8") as f:
232+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
233+
139 | contents = process_contents(f.read())
234+
|
235+
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
236+
237+
FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
238+
--> FURB101.py:141:6
239+
|
240+
139 | contents = process_contents(f.read())
241+
140 |
242+
141 | with open("file.txt", encoding="utf-8") as f:
243+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
244+
142 | contents: str = process_contents(f.read())
245+
|
246+
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`

0 commit comments

Comments
 (0)