mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-18 03:36:18 +00:00
[refurb] Auto-fix annotated assignments (FURB101) (#21278)
Some checks are pending
CI / cargo fmt (push) Waiting to run
CI / Determine changes (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (${{ github.repository == 'astral-sh/ruff' && 'depot-windows-2022-16' || 'windows-latest' }}) (push) Blocked by required conditions
CI / cargo test (macos-latest) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / ty completion evaluation (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks instrumented (ruff) (push) Blocked by required conditions
CI / benchmarks instrumented (ty) (push) Blocked by required conditions
CI / benchmarks walltime (medium|multithreaded) (push) Blocked by required conditions
CI / benchmarks walltime (small|large) (push) Blocked by required conditions
Some checks are pending
CI / cargo fmt (push) Waiting to run
CI / Determine changes (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (${{ github.repository == 'astral-sh/ruff' && 'depot-windows-2022-16' || 'windows-latest' }}) (push) Blocked by required conditions
CI / cargo test (macos-latest) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / ty completion evaluation (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks instrumented (ruff) (push) Blocked by required conditions
CI / benchmarks instrumented (ty) (push) Blocked by required conditions
CI / benchmarks walltime (medium|multithreaded) (push) Blocked by required conditions
CI / benchmarks walltime (small|large) (push) Blocked by required conditions
## 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>
This commit is contained in:
parent
e06e108095
commit
16de4aa3cc
3 changed files with 107 additions and 18 deletions
|
|
@ -125,3 +125,18 @@ with open(*filename, mode="r") as f:
|
|||
# `buffering`.
|
||||
with open(*filename, file="file.txt", mode="r") as f:
|
||||
x = f.read()
|
||||
|
||||
# FURB101
|
||||
with open("file.txt", encoding="utf-8") as f:
|
||||
contents: str = f.read()
|
||||
|
||||
# FURB101 but no fix because it would remove the assignment to `x`
|
||||
with open("file.txt", encoding="utf-8") as f:
|
||||
contents, x = f.read(), 2
|
||||
|
||||
# FURB101 but no fix because it would remove the `process_contents` call
|
||||
with open("file.txt", encoding="utf-8") as f:
|
||||
contents = process_contents(f.read())
|
||||
|
||||
with open("file.txt", encoding="utf-8") as f:
|
||||
contents: str = process_contents(f.read())
|
||||
|
|
|
|||
|
|
@ -125,20 +125,8 @@ impl<'a> Visitor<'a> for ReadMatcher<'a, '_> {
|
|||
open.item.range(),
|
||||
);
|
||||
|
||||
let target = match self.with_stmt.body.first() {
|
||||
Some(Stmt::Assign(assign))
|
||||
if assign.value.range().contains_range(expr.range()) =>
|
||||
{
|
||||
match assign.targets.first() {
|
||||
Some(Expr::Name(name)) => Some(name.id.as_str()),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
|
||||
if let Some(fix) =
|
||||
generate_fix(self.checker, &open, target, self.with_stmt, &suggestion)
|
||||
generate_fix(self.checker, &open, expr, self.with_stmt, &suggestion)
|
||||
{
|
||||
diagnostic.set_fix(fix);
|
||||
}
|
||||
|
|
@ -190,15 +178,16 @@ fn make_suggestion(open: &FileOpen<'_>, generator: Generator) -> String {
|
|||
fn generate_fix(
|
||||
checker: &Checker,
|
||||
open: &FileOpen,
|
||||
target: Option<&str>,
|
||||
expr: &Expr,
|
||||
with_stmt: &ast::StmtWith,
|
||||
suggestion: &str,
|
||||
) -> Option<Fix> {
|
||||
if !(with_stmt.items.len() == 1 && matches!(with_stmt.body.as_slice(), [Stmt::Assign(_)])) {
|
||||
if with_stmt.items.len() != 1 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let locator = checker.locator();
|
||||
|
||||
let filename_code = locator.slice(open.filename.range());
|
||||
|
||||
let (import_edit, binding) = checker
|
||||
|
|
@ -210,9 +199,39 @@ fn generate_fix(
|
|||
)
|
||||
.ok()?;
|
||||
|
||||
let replacement = match target {
|
||||
Some(var) => format!("{var} = {binding}({filename_code}).{suggestion}"),
|
||||
None => format!("{binding}({filename_code}).{suggestion}"),
|
||||
// Only replace context managers with a single assignment or annotated assignment in the body.
|
||||
// The assignment's RHS must also be the same as the `read` call in `expr`, otherwise this fix
|
||||
// would remove the rest of the expression.
|
||||
let replacement = match with_stmt.body.as_slice() {
|
||||
[Stmt::Assign(ast::StmtAssign { targets, value, .. })] if value.range() == expr.range() => {
|
||||
match targets.as_slice() {
|
||||
[Expr::Name(name)] => {
|
||||
format!(
|
||||
"{name} = {binding}({filename_code}).{suggestion}",
|
||||
name = name.id
|
||||
)
|
||||
}
|
||||
_ => return None,
|
||||
}
|
||||
}
|
||||
[
|
||||
Stmt::AnnAssign(ast::StmtAnnAssign {
|
||||
target,
|
||||
annotation,
|
||||
value: Some(value),
|
||||
..
|
||||
}),
|
||||
] if value.range() == expr.range() => match target.as_ref() {
|
||||
Expr::Name(name) => {
|
||||
format!(
|
||||
"{var}: {ann} = {binding}({filename_code}).{suggestion}",
|
||||
var = name.id,
|
||||
ann = locator.slice(annotation.range())
|
||||
)
|
||||
}
|
||||
_ => return None,
|
||||
},
|
||||
_ => return None,
|
||||
};
|
||||
|
||||
let applicability = if checker.comment_ranges().intersects(with_stmt.range()) {
|
||||
|
|
|
|||
|
|
@ -189,3 +189,58 @@ FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()`
|
|||
51 | # the user reads the whole file and that bit they can replace.
|
||||
|
|
||||
help: Replace with `Path("file.txt").read_text()`
|
||||
|
||||
FURB101 [*] `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
|
||||
--> FURB101.py:130:6
|
||||
|
|
||||
129 | # FURB101
|
||||
130 | with open("file.txt", encoding="utf-8") as f:
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
131 | contents: str = f.read()
|
||||
|
|
||||
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
|
||||
1 + import pathlib
|
||||
2 | def foo():
|
||||
3 | ...
|
||||
4 |
|
||||
--------------------------------------------------------------------------------
|
||||
128 | x = f.read()
|
||||
129 |
|
||||
130 | # FURB101
|
||||
- with open("file.txt", encoding="utf-8") as f:
|
||||
- contents: str = f.read()
|
||||
131 + contents: str = pathlib.Path("file.txt").read_text(encoding="utf-8")
|
||||
132 |
|
||||
133 | # FURB101 but no fix because it would remove the assignment to `x`
|
||||
134 | with open("file.txt", encoding="utf-8") as f:
|
||||
|
||||
FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
|
||||
--> FURB101.py:134:6
|
||||
|
|
||||
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
|
||||
|
|
||||
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
|
||||
|
||||
FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
|
||||
--> FURB101.py:138:6
|
||||
|
|
||||
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())
|
||||
|
|
||||
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
|
||||
|
||||
FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
|
||||
--> FURB101.py:141:6
|
||||
|
|
||||
139 | contents = process_contents(f.read())
|
||||
140 |
|
||||
141 | with open("file.txt", encoding="utf-8") as f:
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
142 | contents: str = process_contents(f.read())
|
||||
|
|
||||
help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue