[refurb] Add fixes for FURB101, FURB103

crates/ruff_linter/src/preview.rs

@@ -259,3 +259,13 @@ pub(crate) const fn is_b006_unsafe_fix_preserve_assignment_expr_enabled(
 ) -> bool {
     settings.preview.is_enabled()
 }
+
+// https://github.com/astral-sh/ruff/pull/20520
+pub(crate) const fn is_fix_read_whole_file_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}
+
+// https://github.com/astral-sh/ruff/pull/20520
+pub(crate) const fn is_fix_write_whole_file_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}

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

@@ -1,14 +1,13 @@
 use std::borrow::Cow;
 
-use ruff_python_ast::name::Name;
-use ruff_python_ast::{self as ast, Expr, parenthesize::parenthesized_range};
+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_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
 use crate::{Applicability, Edit, Fix};
-use ruff_python_ast::PythonVersion;
 
 /// Format a code snippet to call `name.method()`.
 pub(super) fn generate_method_call(name: Name, method: &str, generator: Generator) -> String {
@@ -345,12 +344,8 @@ pub(super) fn parenthesize_loop_iter_if_necessary<'a>(
     let iter_in_source = locator.slice(iter);
 
     match iter {
-        ast::Expr::Tuple(tuple) if !tuple.parenthesized => {
-            Cow::Owned(format!("({iter_in_source})"))
-        }
-        ast::Expr::Lambda(_) | ast::Expr::If(_)
-            if matches!(location, IterLocation::Comprehension) =>
-        {
+        Expr::Tuple(tuple) if !tuple.parenthesized => Cow::Owned(format!("({iter_in_source})")),
+        Expr::Lambda(_) | Expr::If(_) if matches!(location, IterLocation::Comprehension) => {
             Cow::Owned(format!("({iter_in_source})"))
         }
         _ => Cow::Borrowed(iter_in_source),

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

@@ -12,6 +12,7 @@ mod tests {
     use test_case::test_case;
 
     use crate::registry::Rule;
+    use crate::settings::types::PreviewMode;
     use crate::test::test_path;
     use crate::{assert_diagnostics, settings};
 
@@ -62,6 +63,25 @@ mod tests {
         Ok(())
     }
 
+    #[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))]
+    #[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))]
+    fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
+        let snapshot = format!(
+            "preview_{}_{}",
+            rule_code.noqa_code(),
+            path.to_string_lossy()
+        );
+        let diagnostics = test_path(
+            Path::new("refurb").join(path).as_path(),
+            &settings::LinterSettings {
+                preview: PreviewMode::Enabled,
+                ..settings::LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_diagnostics!(snapshot, diagnostics);
+        Ok(())
+    }
+
     #[test]
     fn write_whole_file_python_39() -> Result<()> {
         let diagnostics = test_path(

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

@@ -1,14 +1,17 @@
+use ruff_diagnostics::{Applicability, Edit, Fix};
 use ruff_macros::{ViolationMetadata, derive_message_formats};
-use ruff_python_ast::visitor::{self, Visitor};
-use ruff_python_ast::{self as ast, Expr};
+use ruff_python_ast::{
+    self as ast, Expr, Stmt,
+    visitor::{self, Visitor},
+};
 use ruff_python_codegen::Generator;
 use ruff_text_size::{Ranged, TextRange};
 
-use crate::Violation;
 use crate::checkers::ast::Checker;
 use crate::fix::snippet::SourceCodeSnippet;
-
+use crate::importer::ImportRequest;
 use crate::rules::refurb::helpers::{FileOpen, find_file_opens};
+use crate::{FixAvailability, Violation};
 
 /// ## What it does
 /// Checks for uses of `open` and `read` that can be replaced by `pathlib`
@@ -31,6 +34,8 @@ use crate::rules::refurb::helpers::{FileOpen, find_file_opens};
 ///
 /// contents = Path(filename).read_text()
 /// ```
+/// ## Fix Safety
+/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression.
 ///
 /// ## References
 /// - [Python documentation: `Path.read_bytes`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.read_bytes)
@@ -42,12 +47,22 @@ pub(crate) struct ReadWholeFile {
 }
 
 impl Violation for ReadWholeFile {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         let filename = self.filename.truncated_display();
         let suggestion = self.suggestion.truncated_display();
         format!("`open` and `read` 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(),
+        ))
+    }
 }
 
 /// FURB101
@@ -64,21 +79,27 @@ pub(crate) fn read_whole_file(checker: &Checker, with: &ast::StmtWith) {
     }
 
     // Then we need to match each `open` operation with exactly one `read` call.
-    let mut matcher = ReadMatcher::new(checker, candidates);
+    let mut matcher = ReadMatcher::new(checker, candidates, with);
     visitor::walk_body(&mut matcher, &with.body);
 }
 
 /// AST visitor that matches `open` operations with the corresponding `read` calls.
 struct ReadMatcher<'a, 'b> {
     checker: &'a Checker<'b>,
     candidates: Vec<FileOpen<'a>>,
+    with_stmt: &'a ast::StmtWith,
 }
 
 impl<'a, 'b> ReadMatcher<'a, 'b> {
-    fn new(checker: &'a Checker<'b>, candidates: Vec<FileOpen<'a>>) -> Self {
+    fn new(
+        checker: &'a Checker<'b>,
+        candidates: Vec<FileOpen<'a>>,
+        with_stmt: &'a ast::StmtWith,
+    ) -> Self {
         Self {
             checker,
             candidates,
+            with_stmt,
         }
     }
 }
@@ -92,15 +113,38 @@ impl<'a> Visitor<'a> for ReadMatcher<'a, '_> {
                 .position(|open| open.is_ref(read_from))
             {
                 let open = self.candidates.remove(open);
-                self.checker.report_diagnostic(
+                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),
                         ),
-                        suggestion: make_suggestion(&open, self.checker.generator()),
+                        suggestion: SourceCodeSnippet::from_str(&suggestion),
                     },
                     open.item.range(),
                 );
+
+                if !crate::preview::is_fix_read_whole_file_enabled(self.checker.settings()) {
+                    return;
+                }
+
+                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)
+                {
+                    diagnostic.set_fix(fix);
+                }
             }
             return;
         }
@@ -125,7 +169,7 @@ fn match_read_call(expr: &Expr) -> Option<&Expr> {
     Some(&*attr.value)
 }
 
-fn make_suggestion(open: &FileOpen<'_>, generator: Generator) -> SourceCodeSnippet {
+fn make_suggestion(open: &FileOpen<'_>, generator: Generator) -> String {
     let name = ast::ExprName {
         id: open.mode.pathlib_method(),
         ctx: ast::ExprContext::Load,
@@ -143,5 +187,46 @@ fn make_suggestion(open: &FileOpen<'_>, generator: Generator) -> SourceCodeSnipp
         range: TextRange::default(),
         node_index: ruff_python_ast::AtomicNodeIndex::NONE,
     };
-    SourceCodeSnippet::from_str(&generator.expr(&call.into()))
+    generator.expr(&call.into())
+}
+
+fn generate_fix(
+    checker: &Checker,
+    open: &FileOpen,
+    target: Option<&str>,
+    with_stmt: &ast::StmtWith,
+    suggestion: &str,
+) -> Option<Fix> {
+    if !(with_stmt.items.len() == 1 && matches!(with_stmt.body.as_slice(), [Stmt::Assign(_)])) {
+        return None;
+    }
+
+    let locator = checker.locator();
+    let filename_code = locator.slice(open.filename.range());
+
+    let (import_edit, binding) = checker
+        .importer()
+        .get_or_import_symbol(
+            &ImportRequest::import("pathlib", "Path"),
+            with_stmt.start(),
+            checker.semantic(),
+        )
+        .ok()?;
+
+    let replacement = match target {
+        Some(var) => format!("{var} = {binding}({filename_code}).{suggestion}"),
+        None => format!("{binding}({filename_code}).{suggestion}"),
+    };
+
+    let applicability = if checker.comment_ranges().intersects(with_stmt.range()) {
+        Applicability::Unsafe
+    } else {
+        Applicability::Safe
+    };
+
+    Some(Fix::applicable_edits(
+        Edit::range_replacement(replacement, with_stmt.range()),
+        [import_edit],
+        applicability,
+    ))
 }