[refurb] Fix FURB101 and FURB103 false positives when I/O variable is used later

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

@@ -0,0 +1,21 @@
+# FURB101
+with open("file.txt", encoding="utf-8") as f:
+    _ = f.read()
+f = object()
+print(f)
+
+# See: https://github.com/astral-sh/ruff/issues/21483
+with open("file.txt", encoding="utf-8") as f:
+    _ = f.read()
+print(f.mode)
+
+# Rebinding in a later `with ... as config_file` should not suppress this one.
+with open("config.yaml", encoding="utf-8") as config_file:
+    config_raw = config_file.read()
+
+if "tts:" in config_raw:
+    try:
+        with open("config.yaml", "w", encoding="utf-8") as config_file:
+            config_file.write(config_raw.replace("tts:", "google_translate:"))
+    except OSError:
+        pass

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

@@ -0,0 +1,26 @@
+# FURB103
+# should trigger
+with open("file.txt", "w", encoding="utf-8") as f:
+    f.write("\n")
+f = object()
+print(f)
+
+# See: https://github.com/astral-sh/ruff/issues/21483
+with open("file.txt", "w") as f:
+    f.write("\n")
+print(f.encoding)
+
+
+def _():
+    # should trigger
+    with open("file.txt", "w") as f:
+        f.write("\n")
+    return (f.name for _ in [0])
+
+
+def _set():
+    # should trigger
+    with open("file.txt", "w") as f:
+        f.write("\n")
+    g = {f.name for _ in [0]}
+    return g

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

@@ -0,0 +1,23 @@
+use ruff_python_ast::Stmt;
+
+use crate::{checkers::ast::Checker, codes::Rule, rules::refurb};
+
+/// Run lint rules over all deferred with-statements in the [`SemanticModel`].
+pub(crate) fn deferred_with_statements(checker: &mut Checker) {
+    while !checker.analyze.with_statements.is_empty() {
+        let with_statements = std::mem::take(&mut checker.analyze.with_statements);
+        for snapshot in with_statements {
+            checker.semantic.restore(snapshot);
+
+            let Stmt::With(stmt_with) = checker.semantic.current_statement() else {
+                unreachable!("Expected Stmt::With");
+            };
+            if checker.is_rule_enabled(Rule::ReadWholeFile) {
+                refurb::rules::read_whole_file(checker, stmt_with);
+            }
+            if checker.is_rule_enabled(Rule::WriteWholeFile) {
+                refurb::rules::write_whole_file(checker, stmt_with);
+            }
+        }
+    }
+}

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

@@ -4,6 +4,7 @@ pub(super) use deferred_comprehensions::deferred_comprehensions;
 pub(super) use deferred_for_loops::deferred_for_loops;
 pub(super) use deferred_lambdas::deferred_lambdas;
 pub(super) use deferred_scopes::deferred_scopes;
+pub(super) use deferred_with_statements::deferred_with_statements;
 pub(super) use definitions::definitions;
 pub(super) use except_handler::except_handler;
 pub(super) use expression::expression;
@@ -21,6 +22,7 @@ mod deferred_comprehensions;
 mod deferred_for_loops;
 mod deferred_lambdas;
 mod deferred_scopes;
+mod deferred_with_statements;
 mod definitions;
 mod except_handler;
 mod expression;

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

@@ -1181,11 +1181,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.is_rule_enabled(Rule::RedefinedLoopName) {
                 pylint::rules::redefined_loop_name(checker, stmt);
             }
-            if checker.is_rule_enabled(Rule::ReadWholeFile) {
-                refurb::rules::read_whole_file(checker, with_stmt);
-            }
-            if checker.is_rule_enabled(Rule::WriteWholeFile) {
-                refurb::rules::write_whole_file(checker, with_stmt);
+            if checker.any_rule_enabled(&[Rule::ReadWholeFile, Rule::WriteWholeFile]) {
+                checker
+                    .analyze
+                    .with_statements
+                    .push(checker.semantic.snapshot());
             }
             if checker.is_rule_enabled(Rule::UselessWithLock) {
                 pylint::rules::useless_with_lock(checker, with_stmt);

crates/ruff_linter/src/checkers/ast/deferred.rs

@@ -34,5 +34,6 @@ pub(crate) struct Analyze {
     pub(crate) scopes: Vec<ScopeId>,
     pub(crate) lambdas: Vec<Snapshot>,
     pub(crate) for_loops: Vec<Snapshot>,
+    pub(crate) with_statements: Vec<Snapshot>,
     pub(crate) comprehensions: Vec<Snapshot>,
 }

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -3321,6 +3321,7 @@ pub(crate) fn check_ast(
     analyze::definitions(&mut checker);
     analyze::bindings(&checker);
     analyze::unresolved_references(&checker);
+    analyze::deferred_with_statements(&mut checker);
 
     // Reset the scope to module-level, and check all consumed scopes.
     checker.semantic.scope_id = ScopeId::global();

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

@@ -217,23 +217,29 @@ fn resolve_file_open<'a>(
     if matches!(mode, OpenMode::ReadBytes | OpenMode::WriteBytes) && !keywords.is_empty() {
         return None;
     }
+
     let var = item.optional_vars.as_deref()?.as_name_expr()?;
     let scope = semantic.current_scope();
-
     let binding = scope.get_all(var.id.as_str()).find_map(|id| {
-        let b = semantic.binding(id);
-        (b.range() == var.range()).then_some(b)
+        let binding = semantic.binding(id);
+        (binding.range() == var.range()).then_some(binding)
     })?;
-    let references: Vec<&ResolvedReference> = binding
-        .references
-        .iter()
-        .map(|id| semantic.reference(*id))
-        .filter(|reference| with.range().contains_range(reference.range()))
-        .collect();
-
-    let [reference] = references.as_slice() else {
+    let mut binding_references = binding
+        .references()
+        .map(|id| semantic.reference(id))
+        // Reassignments in the same scope can carry forward older references. Ignore anything
+        // that appears before this `with` statement and only consider references from this point.
+        .filter(|reference| {
+            reference.scope_id() == binding.scope && reference.start() >= with.start()
+        });
+
+    let reference = binding_references.next()?;
+    if binding_references.next().is_some() {
         return None;
-    };
+    }
+    if !with.range().contains_range(reference.range()) {
+        return None;
+    }
 
     Some(FileOpen {
         item,
@@ -279,6 +285,7 @@ fn find_file_open<'a>(
     let (keywords, kw_mode) = match_open_keywords(keywords, read_mode, python_version)?;
 
     let mode = kw_mode.unwrap_or(pos_mode);
+
     resolve_file_open(
         item,
         with,
@@ -307,9 +314,11 @@ fn find_path_open<'a>(
     {
         return None;
     }
+
     if !is_open_call_from_pathlib(func, semantic) {
         return None;
     }
+
     let attr = func.as_attribute_expr()?;
     let mode = if args.is_empty() {
         OpenMode::ReadText
@@ -319,6 +328,7 @@ fn find_path_open<'a>(
 
     let (keywords, kw_mode) = match_open_keywords(keywords, read_mode, python_version)?;
     let mode = kw_mode.unwrap_or(mode);
+
     resolve_file_open(
         item,
         with,

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

@@ -17,6 +17,7 @@ mod tests {
 
     #[test_case(Rule::ReadWholeFile, Path::new("FURB101_0.py"))]
     #[test_case(Rule::ReadWholeFile, Path::new("FURB101_1.py"))]
+    #[test_case(Rule::ReadWholeFile, Path::new("FURB101_2.py"))]
     #[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
     #[test_case(Rule::IfExpInsteadOfOrOperator, Path::new("FURB110.py"))]
     #[test_case(Rule::ReimplementedOperator, Path::new("FURB118.py"))]
@@ -49,6 +50,7 @@ mod tests {
     #[test_case(Rule::ListReverseCopy, Path::new("FURB187.py"))]
     #[test_case(Rule::WriteWholeFile, Path::new("FURB103_0.py"))]
     #[test_case(Rule::WriteWholeFile, Path::new("FURB103_1.py"))]
+    #[test_case(Rule::WriteWholeFile, Path::new("FURB103_2.py"))]
     #[test_case(Rule::FStringNumberFormat, Path::new("FURB116.py"))]
     #[test_case(Rule::SortedMinMax, Path::new("FURB192.py"))]
     #[test_case(Rule::SliceToRemovePrefixOrSuffix, Path::new("FURB188.py"))]

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

@@ -0,0 +1,47 @@
+---
+source: crates/ruff_linter/src/rules/refurb/mod.rs
+assertion_line: 65
+---
+FURB101 [*] `open` and `read` should be replaced by `Path("file.txt").read_text(encoding="utf-8")`
+ --> FURB101_2.py:2:6
+  |
+1 | # FURB101
+2 | with open("file.txt", encoding="utf-8") as f:
+  |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+3 |     _ = f.read()
+4 | f = object()
+  |
+help: Replace with `Path("file.txt").read_text(encoding="utf-8")`
+1 | # FURB101
+  - with open("file.txt", encoding="utf-8") as f:
+  -     _ = f.read()
+2 + import pathlib
+3 + _ = pathlib.Path("file.txt").read_text(encoding="utf-8")
+4 | f = object()
+5 | print(f)
+6 | 
+
+FURB101 [*] `open` and `read` should be replaced by `Path("config.yaml").read_text(encoding="utf-8")`
+  --> FURB101_2.py:13:6
+   |
+12 | # Rebinding in a later `with ... as config_file` should not suppress this one.
+13 | with open("config.yaml", encoding="utf-8") as config_file:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+14 |     config_raw = config_file.read()
+   |
+help: Replace with `Path("config.yaml").read_text(encoding="utf-8")`
+1  | # FURB101
+2  + import pathlib
+3  | with open("file.txt", encoding="utf-8") as f:
+4  |     _ = f.read()
+5  | f = object()
+--------------------------------------------------------------------------------
+11 | print(f.mode)
+12 | 
+13 | # Rebinding in a later `with ... as config_file` should not suppress this one.
+   - with open("config.yaml", encoding="utf-8") as config_file:
+   -     config_raw = config_file.read()
+14 + config_raw = pathlib.Path("config.yaml").read_text(encoding="utf-8")
+15 | 
+16 | if "tts:" in config_raw:
+17 |     try:

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

@@ -0,0 +1,78 @@
+---
+source: crates/ruff_linter/src/rules/refurb/mod.rs
+---
+FURB103 [*] `open` and `write` should be replaced by `Path("file.txt").write_text("\n", encoding="utf-8")`
+ --> FURB103_2.py:3:6
+  |
+1 | # FURB103
+2 | # should trigger
+3 | with open("file.txt", "w", encoding="utf-8") as f:
+  |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+4 |     f.write("\n")
+5 | f = object()
+  |
+help: Replace with `Path("file.txt").write_text("\n", encoding="utf-8")`
+1 | # FURB103
+2 | # should trigger
+  - with open("file.txt", "w", encoding="utf-8") as f:
+  -     f.write("\n")
+3 + import pathlib
+4 + pathlib.Path("file.txt").write_text("\n", encoding="utf-8")
+5 | f = object()
+6 | print(f)
+7 | 
+
+FURB103 [*] `open` and `write` should be replaced by `Path("file.txt").write_text("\n")`
+  --> FURB103_2.py:16:10
+   |
+14 | def _():
+15 |     # should trigger
+16 |     with open("file.txt", "w") as f:
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^
+17 |         f.write("\n")
+18 |     return (f.name for _ in [0])
+   |
+help: Replace with `Path("file.txt").write_text("\n")`
+1  | # FURB103
+2  | # should trigger
+3  + import pathlib
+4  | with open("file.txt", "w", encoding="utf-8") as f:
+5  |     f.write("\n")
+6  | f = object()
+--------------------------------------------------------------------------------
+14 | 
+15 | def _():
+16 |     # should trigger
+   -     with open("file.txt", "w") as f:
+   -         f.write("\n")
+17 +     pathlib.Path("file.txt").write_text("\n")
+18 |     return (f.name for _ in [0])
+19 | 
+20 | 
+
+FURB103 [*] `open` and `write` should be replaced by `Path("file.txt").write_text("\n")`
+  --> FURB103_2.py:23:10
+   |
+21 | def _set():
+22 |     # should trigger
+23 |     with open("file.txt", "w") as f:
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^
+24 |         f.write("\n")
+25 |     g = {f.name for _ in [0]}
+   |
+help: Replace with `Path("file.txt").write_text("\n")`
+1  | # FURB103
+2  | # should trigger
+3  + import pathlib
+4  | with open("file.txt", "w", encoding="utf-8") as f:
+5  |     f.write("\n")
+6  | f = object()
+--------------------------------------------------------------------------------
+21 | 
+22 | def _set():
+23 |     # should trigger
+   -     with open("file.txt", "w") as f:
+   -         f.write("\n")
+24 +     pathlib.Path("file.txt").write_text("\n")
+25 |     g = {f.name for _ in [0]}
+26 |     return g