refactor: make filename optional

Signed-off-by: 11happy <[email protected]>

Author: 11happy

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

@@ -1,4 +1,3 @@
-from pathlib import Path
 def foo():
     ...
 
@@ -155,20 +154,4 @@ def bar(x):
 with open("test.json", "wb") as f:
     f.write(json.dumps(data, indent=4).encode("utf-8"))
 
-with Path("file.txt").open("w") as f:
-    f.write("test")
-
-with Path("file.txt").open("wb") as f:
-    f.write(b"test")
 
-with Path("file.txt").open(mode="w") as f:
-    f.write("test")
-
-with Path("file.txt").open("w", encoding="utf8") as f:
-    f.write("test")
-
-with Path("file.txt").open("w", errors="ignore") as f:
-    f.write("test")
-
-with Path(foo()).open("w") as f:
-    f.write("test")

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

@@ -0,0 +1,23 @@
+from pathlib import Path
+
+with Path("file.txt").open("w") as f:
+    f.write("test")
+
+with Path("file.txt").open("wb") as f:
+    f.write(b"test")
+
+with Path("file.txt").open(mode="w") as f:
+    f.write("test")
+
+with Path("file.txt").open("w", encoding="utf8") as f:
+    f.write("test")
+
+with Path("file.txt").open("w", errors="ignore") as f:
+    f.write("test")
+
+with Path(foo()).open("w") as f:
+    f.write("test")
+
+p = Path("file.txt")
+with p.open("w") as f:
+    f.write("test")

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

@@ -1,14 +1,15 @@
 use std::borrow::Cow;
 
-use crate::checkers::ast::Checker;
-use crate::rules::flake8_async::rules::blocking_open_call::is_open_call_from_pathlib;
-use crate::{Applicability, Edit, Fix};
 use ruff_python_ast::PythonVersion;
 use ruff_python_ast::{self as ast, Expr, name::Name, parenthesize::parenthesized_range};
 use ruff_python_codegen::Generator;
-use ruff_python_semantic::{BindingId, ResolvedReference, SemanticModel};
+use ruff_python_semantic::{ResolvedReference, SemanticModel};
 use ruff_text_size::{Ranged, TextRange};
 
+use crate::checkers::ast::Checker;
+use crate::rules::flake8_async::rules::blocking_open_call::is_open_call_from_pathlib;
+use crate::{Applicability, Edit, Fix};
+
 /// Format a code snippet to call `name.method()`.
 pub(super) fn generate_method_call(name: Name, method: &str, generator: Generator) -> String {
     // Construct `name`.
@@ -120,14 +121,14 @@ pub(super) struct FileOpen<'a> {
     /// With item where the open happens, we use it for the reporting range.
     pub(super) item: &'a ast::WithItem,
     /// Filename expression used as the first argument in `open`, we use it in the diagnostic message.
-    pub(super) filename: &'a Expr,
+    pub(super) filename: Option<&'a Expr>,
     /// The file open mode.
     pub(super) mode: OpenMode,
     /// The file open keywords.
     pub(super) keywords: Vec<&'a ast::Keyword>,
     /// We only check `open` operations whose file handles are used exactly once.
     pub(super) reference: &'a ResolvedReference,
-    /// `Path().open()` call
+    /// Pathlib Path object used to open the file, if any.
     pub(super) path_obj: Option<&'a Expr>,
 }
 
@@ -155,12 +156,13 @@ pub(super) fn find_file_opens<'a>(
         .collect()
 }
 
+#[expect(clippy::too_many_arguments)]
 fn resolve_file_open<'a>(
     item: &'a ast::WithItem,
     with: &'a ast::StmtWith,
     semantic: &'a SemanticModel<'a>,
     read_mode: bool,
-    filename: &'a Expr,
+    filename: Option<&'a Expr>,
     mode: OpenMode,
     keywords: Vec<&'a ast::Keyword>,
     path_obj: Option<&'a Expr>,
@@ -181,15 +183,13 @@ 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 references: Vec<&ResolvedReference> = binding
         .references
         .iter()
@@ -247,7 +247,14 @@ fn find_file_open<'a>(
 
     let mode = kw_mode.unwrap_or(pos_mode);
     resolve_file_open(
-        item, with, semantic, read_mode, filename, mode, keywords, None,
+        item,
+        with,
+        semantic,
+        read_mode,
+        Some(filename),
+        mode,
+        keywords,
+        None,
     )
 }
 
@@ -263,17 +270,26 @@ fn find_path_open<'a>(
         arguments: ast::Arguments { args, keywords, .. },
         ..
     } = item.context_expr.as_call_expr()?;
+    if args.iter().any(Expr::is_starred_expr)
+        || keywords.iter().any(|keyword| keyword.arg.is_none())
+    {
+        return None;
+    }
     if !is_open_call_from_pathlib(func, semantic) {
         return None;
     }
     let attr = func.as_attribute_expr()?;
-    let path_call = attr.value.as_call_expr()?;
-    let filename = path_call.arguments.args.first()?;
+    let filename = if let Expr::Call(path_call) = attr.value.as_ref() {
+        path_call.arguments.args.first()
+    } else {
+        None
+    };
     let mode = if args.is_empty() {
         OpenMode::ReadText
     } else {
         match_open_mode(args.first()?)?
     };
+
     let (keywords, kw_mode) = match_open_keywords(keywords, read_mode, python_version)?;
     let mode = kw_mode.unwrap_or(mode);
     resolve_file_open(

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

@@ -48,6 +48,7 @@ mod tests {
     #[test_case(Rule::HashlibDigestHex, Path::new("FURB181.py"))]
     #[test_case(Rule::ListReverseCopy, Path::new("FURB187.py"))]
     #[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))]
+    #[test_case(Rule::WriteWholeFile, Path::new("FURB103_EXT.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/rules/read_whole_file.rs

@@ -114,12 +114,17 @@ impl<'a> Visitor<'a> for ReadMatcher<'a, '_> {
                 .position(|open| open.is_ref(read_from))
             {
                 let open = self.candidates.remove(open);
+                let filename_display = if let Some(filename) = open.filename {
+                    self.checker.generator().expr(filename)
+                } else if let Some(path_obj) = open.path_obj {
+                    self.checker.locator().slice(path_obj.range()).to_string()
+                } else {
+                    return;
+                };
                 let suggestion = make_suggestion(&open, self.checker.generator());
                 let mut diagnostic = self.checker.report_diagnostic(
                     ReadWholeFile {
-                        filename: SourceCodeSnippet::from_str(
-                            &self.checker.generator().expr(open.filename),
-                        ),
+                        filename: SourceCodeSnippet::from_str(&filename_display),
                         suggestion: SourceCodeSnippet::from_str(&suggestion),
                     },
                     open.item.range(),
@@ -201,9 +206,9 @@ fn generate_fix(
     if !(with_stmt.items.len() == 1 && matches!(with_stmt.body.as_slice(), [Stmt::Assign(_)])) {
         return None;
     }
-
+    let filename = open.filename?;
     let locator = checker.locator();
-    let filename_code = locator.slice(open.filename.range());
+    let filename_code = locator.slice(filename.range());
 
     let (import_edit, binding) = checker
         .importer()

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

@@ -48,6 +48,7 @@ pub(crate) struct WriteWholeFile {
     filename: SourceCodeSnippet,
     suggestion: SourceCodeSnippet,
     is_path_open: bool,
+    has_filename: bool,
 }
 
 impl Violation for WriteWholeFile {
@@ -58,19 +59,35 @@ impl Violation for WriteWholeFile {
         let filename = self.filename.truncated_display();
         let suggestion = self.suggestion.truncated_display();
         if self.is_path_open {
-            format!(
-                "`Path.open()` followed by `write()` can be replaced by `Path({filename}).{suggestion}`"
-            )
+            if self.has_filename {
+                format!(
+                    "`Path().open()` followed by `write()` can be replaced by `Path({filename}).{suggestion}`"
+                )
+            } else {
+                format!(
+                    "`Path.open()` followed by `write()` can be replaced by `{filename}.{suggestion}`"
+                )
+            }
         } else {
             format!("`open` and `write` should be replaced by `Path({filename}).{suggestion}`")
         }
     }
     fn fix_title(&self) -> Option<String> {
-        Some(format!(
-            "Replace with `Path({}).{}`",
-            self.filename.truncated_display(),
-            self.suggestion.truncated_display(),
-        ))
+        let formatted = if self.has_filename {
+            format!(
+                "Replace with `Path({}).{}`",
+                self.filename.truncated_display(),
+                self.suggestion.truncated_display(),
+            )
+        } else {
+            format!(
+                "Replace with `{}.{}`",
+                self.filename.truncated_display(),
+                self.suggestion.truncated_display(),
+            )
+        };
+
+        Some(formatted)
     }
 }
 
@@ -134,17 +151,27 @@ impl<'a> Visitor<'a> for WriteMatcher<'a, '_> {
                 .position(|open| open.is_ref(write_to))
             {
                 let open = self.candidates.remove(open);
-
+                let has_filename: bool;
+                let filename_display: String;
                 if self.loop_counter == 0 {
+                    if let Some(filename) = open.filename {
+                        filename_display = self.checker.generator().expr(filename);
+                        has_filename = true;
+                    } else if let Some(path_obj) = open.path_obj {
+                        filename_display =
+                            self.checker.locator().slice(path_obj.range()).to_string();
+                        has_filename = false;
+                    } else {
+                        return;
+                    }
                     let suggestion = make_suggestion(&open, content, self.checker.generator());
 
                     let mut diagnostic = self.checker.report_diagnostic(
                         WriteWholeFile {
-                            filename: SourceCodeSnippet::from_str(
-                                &self.checker.generator().expr(open.filename),
-                            ),
+                            filename: SourceCodeSnippet::from_str(&filename_display),
                             suggestion: SourceCodeSnippet::from_str(&suggestion),
                             is_path_open: open.path_obj.is_some(),
+                            has_filename,
                         },
                         open.item.range(),
                     );
@@ -218,7 +245,8 @@ fn generate_fix(
     }
 
     let locator = checker.locator();
-    let filename_code = locator.slice(open.filename.range());
+    let filename = open.filename?;
+    let filename_code = locator.slice(filename.range());
 
     let (import_edit, binding) = checker
         .importer()