[refurb] Auto-fix annotated assignments (FURB101)

[danparizher]

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.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -0 violations, +14 -0 fixes in 2 projects; 53 projects unchanged)

apache/airflow (+0 -0 violations, +2 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py:1005:18: FURB101 [*] `open` and `read` should be replaced by `Path(self.waiter_path).read_text()`
- providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py:1005:18: FURB101 `open` and `read` should be replaced by `Path(self.waiter_path).read_text()`

zulip/zulip (+0 -0 violations, +12 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ zerver/lib/export.py:1732:10: FURB101 [*] `open` and `read` should be replaced by `Path(input_path).read_bytes()`
- zerver/lib/export.py:1732:10: FURB101 `open` and `read` should be replaced by `Path(input_path).read_bytes()`
+ zerver/lib/import_realm.py:1205:10: FURB101 [*] `open` and `read` should be replaced by `Path(migration_status_filename).read_text()`
- zerver/lib/import_realm.py:1205:10: FURB101 `open` and `read` should be replaced by `Path(migration_status_filename).read_text()`
+ zerver/lib/import_realm.py:996:10: FURB101 [*] `open` and `read` should be replaced by `Path(records_filename).read_bytes()`
- zerver/lib/import_realm.py:996:10: FURB101 `open` and `read` should be replaced by `Path(records_filename).read_bytes()`
+ zerver/lib/management.py:282:18: FURB101 [*] `open` and `read` should be replaced by `Path(options["password_file"]).read_text()`
- zerver/lib/management.py:282:18: FURB101 `open` and `read` should be replaced by `Path(options["password_file"]).read_text()`
+ zerver/management/commands/send_custom_email.py:118:22: FURB101 [*] `open` and `read` should be replaced by `Path(options["json_file"]).read_text()`
- zerver/management/commands/send_custom_email.py:118:22: FURB101 `open` and `read` should be replaced by `Path(options["json_file"]).read_text()`
+ zerver/management/commands/send_custom_email.py:233:18: FURB101 [*] `open` and `read` should be replaced by `Path(options["json_file"]).read_text()`
- zerver/management/commands/send_custom_email.py:233:18: FURB101 `open` and `read` should be replaced by `Path(options["json_file"]).read_text()`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
FURB101	14	0	0	14	0

[ntBre]

Thanks, the results look good here, including the ecosystem check. I just had a couple of suggestions for simplification.

[ntBre]

I should have noticed this in the earlier PR, but this actually looks quite repetitive with the check inside of generate_fix. I think we can combine this check and this check:

matches!(with_stmt.body.as_slice(), [Stmt::Assign(_)]))

generate_fix seems like the right place to do that to me, which should allow us to decrease the number of arguments since we're already passing the with_stmt itself.

[ntBre]

Do we need to use the generator here, or can we just slice the &str out of the input?

Suggested change

	let annotation_code = self
	.checker
	.generator()
	.expr(ann_assign.annotation.as_ref());
	let annotation_code = self
	.checker
	.locator()
	.slice(ann_assign.annotation.range());

(untested but that's what I had in mind)

[ntBre]

I merged the conflicting test changes I made in another PR, simplified the implementation a bit further, and then fixed a couple of preexisting bugs I noticed. The ecosystem comment doesn't seem to be updating, but I downloaded the artifact and checked that instead. It's showing -188 fixes, and from the ones I've clicked on this seems correct. The two bugs I fixed were that the assign.targets.first() check wasn't exactly right because it could drop other elements in an assignment like:

content, x = f.read(), 2

and the contains_range checks also weren't correct. contains_range is true for an expression like:

content = process_contents(f.read())

but we were then replacing the entire assignment statement and discarding the process_contents call. All of the ecosystem changes I've seen so far fit into this latter pattern.

The fix is now fairly restricted. We may want to consider expanding it in the future by doing a more targeted replacement of the read call expression itself instead of re-building the whole assignment as a string. But I think the current state of the PR is a substantial enough improvement to merge as-is.