Restrict fowarding newline argument in open() calls to Python versions >= 3.10

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

@@ -4,6 +4,8 @@ use ruff_python_codegen::Generator;
 use ruff_python_semantic::{BindingId, ResolvedReference, SemanticModel};
 use ruff_text_size::{Ranged, TextRange};
 
+use crate::settings::types::PythonVersion;
+
 /// Format a code snippet to call `name.method()`.
 pub(super) fn generate_method_call(name: Name, method: &str, generator: Generator) -> String {
     // Construct `name`.
@@ -117,10 +119,11 @@ pub(super) fn find_file_opens<'a>(
     with: &'a ast::StmtWith,
     semantic: &'a SemanticModel<'a>,
     read_mode: bool,
+    python_version: PythonVersion,
 ) -> Vec<FileOpen<'a>> {
     with.items
         .iter()
-        .filter_map(|item| find_file_open(item, with, semantic, read_mode))
+        .filter_map(|item| find_file_open(item, with, semantic, read_mode, python_version))
         .collect()
 }
 
@@ -130,6 +133,7 @@ fn find_file_open<'a>(
     with: &'a ast::StmtWith,
     semantic: &'a SemanticModel<'a>,
     read_mode: bool,
+    python_version: PythonVersion,
 ) -> Option<FileOpen<'a>> {
     // We want to match `open(...) as var`.
     let ast::ExprCall {
@@ -157,7 +161,7 @@ fn find_file_open<'a>(
     let (filename, pos_mode) = match_open_args(args)?;
 
     // Match keyword arguments, get keyword arguments to forward and possibly mode.
-    let (keywords, kw_mode) = match_open_keywords(keywords, read_mode)?;
+    let (keywords, kw_mode) = match_open_keywords(keywords, read_mode, python_version)?;
 
     let mode = kw_mode.unwrap_or(pos_mode);
 
@@ -185,7 +189,7 @@ fn find_file_open<'a>(
 
     let binding = bindings
         .iter()
-        .map(|x| semantic.binding(*x))
+        .map(|id| semantic.binding(*id))
         // We might have many bindings with the same name, but we only care
         // for the one we are looking at right now.
         .find(|binding| binding.range() == var.range())?;
@@ -228,18 +232,23 @@ fn match_open_args(args: &[Expr]) -> Option<(&Expr, OpenMode)> {
 fn match_open_keywords(
     keywords: &[ast::Keyword],
     read_mode: bool,
+    target_version: PythonVersion,
 ) -> Option<(Vec<&ast::Keyword>, Option<OpenMode>)> {
     let mut result: Vec<&ast::Keyword> = vec![];
     let mut mode: Option<OpenMode> = None;
 
     for keyword in keywords {
         match keyword.arg.as_ref()?.as_str() {
             "encoding" | "errors" => result.push(keyword),
-            // newline is only valid for write_text
             "newline" => {
                 if read_mode {
+                    // newline is only valid for write_text
+                    return None;
+                } else if target_version < PythonVersion::Py310 {
+                    // `pathlib` doesn't support `newline` until Python 3.10.
                     return None;
                 }
+
                 result.push(keyword);
             }
 

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

@@ -11,6 +11,7 @@ mod tests {
     use test_case::test_case;
 
     use crate::registry::Rule;
+    use crate::settings::types::PythonVersion;
     use crate::test::test_path;
     use crate::{assert_messages, settings};
 
@@ -54,4 +55,15 @@ mod tests {
         assert_messages!(snapshot, diagnostics);
         Ok(())
     }
+
+    #[test]
+    fn write_whole_file_python_39() -> Result<()> {
+        let diagnostics = test_path(
+            Path::new("refurb/FURB103.py"),
+            &settings::LinterSettings::for_rule(Rule::WriteWholeFile)
+                .with_target_version(PythonVersion::Py39),
+        )?;
+        assert_messages!(diagnostics);
+        Ok(())
+    }
 }

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

@@ -58,7 +58,12 @@ pub(crate) fn read_whole_file(checker: &mut Checker, with: &ast::StmtWith) {
     }
 
     // First we go through all the items in the statement and find all `open` operations.
-    let candidates = find_file_opens(with, checker.semantic(), true);
+    let candidates = find_file_opens(
+        with,
+        checker.semantic(),
+        true,
+        checker.settings.target_version,
+    );
     if candidates.is_empty() {
         return;
     }

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

@@ -59,7 +59,12 @@ pub(crate) fn write_whole_file(checker: &mut Checker, with: &ast::StmtWith) {
     }
 
     // First we go through all the items in the statement and find all `open` operations.
-    let candidates = find_file_opens(with, checker.semantic(), false);
+    let candidates = find_file_opens(
+        with,
+        checker.semantic(),
+        false,
+        checker.settings.target_version,
+    );
     if candidates.is_empty() {
         return;
     }

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__write_whole_file_python_39.snap

@@ -0,0 +1,86 @@
+---
+source: crates/ruff_linter/src/rules/refurb/mod.rs
+---
+FURB103.py:12:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_text("test")`
+   |
+11 | # FURB103
+12 | with open("file.txt", "w") as f:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+13 |     f.write("test")
+   |
+
+FURB103.py:16:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_bytes(foobar)`
+   |
+15 | # FURB103
+16 | with open("file.txt", "wb") as f:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+17 |     f.write(foobar)
+   |
+
+FURB103.py:20:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_bytes(b"abc")`
+   |
+19 | # FURB103
+20 | with open("file.txt", mode="wb") as f:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+21 |     f.write(b"abc")
+   |
+
+FURB103.py:24:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_text(foobar, encoding="utf8")`
+   |
+23 | # FURB103
+24 | with open("file.txt", "w", encoding="utf8") as f:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+25 |     f.write(foobar)
+   |
+
+FURB103.py:28:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_text(foobar, errors="ignore")`
+   |
+27 | # FURB103
+28 | with open("file.txt", "w", errors="ignore") as f:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+29 |     f.write(foobar)
+   |
+
+FURB103.py:32:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_text(foobar)`
+   |
+31 | # FURB103
+32 | with open("file.txt", mode="w") as f:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+33 |     f.write(foobar)
+   |
+
+FURB103.py:36:6: FURB103 `open` and `write` should be replaced by `Path(foo()).write_bytes(bar())`
+   |
+35 | # FURB103
+36 | with open(foo(), "wb") as f:
+   |      ^^^^^^^^^^^^^^^^^^^^^^ FURB103
+37 |     # The body of `with` is non-trivial, but the recommendation holds.
+38 |     bar("pre")
+   |
+
+FURB103.py:44:6: FURB103 `open` and `write` should be replaced by `Path("a.txt").write_text(x)`
+   |
+43 | # FURB103
+44 | with open("a.txt", "w") as a, open("b.txt", "wb") as b:
+   |      ^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+45 |     a.write(x)
+46 |     b.write(y)
+   |
+
+FURB103.py:44:31: FURB103 `open` and `write` should be replaced by `Path("b.txt").write_bytes(y)`
+   |
+43 | # FURB103
+44 | with open("a.txt", "w") as a, open("b.txt", "wb") as b:
+   |                               ^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+45 |     a.write(x)
+46 |     b.write(y)
+   |
+
+FURB103.py:49:18: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_text(bar(bar(a + x)))`
+   |
+48 | # FURB103
+49 | with foo() as a, open("file.txt", "w") as b, foo() as c:
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103
+50 |     # We have other things in here, multiple with items, but the user
+51 |     # writes a single time to file and that bit they can replace.
+   |