diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py index 31b1ccd341..77306cfe18 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py @@ -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()) diff --git a/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs b/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs index 279ecb66aa..2b43af89a8 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs @@ -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 { - 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()) { diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap index 4131499c0c..3fea418d76 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap @@ -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")`