[flake8-use-pathlib] Add autofix for PTH101, PTH104, PTH105, PTH121

crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/import_from.py

@@ -47,3 +47,19 @@ def _():
     from builtin import open
 
     with open(p) as _: ...  # No error
+
+file = "file_1.py"
+
+rename(file, "file_2.py")
+
+rename(
+    # commment 1
+    file, # comment 2
+    "file_2.py"
+    ,
+    # comment 3
+)
+
+rename(file, "file_2.py", src_dir_fd=None, dst_dir_fd=None)
+
+rename(file, "file_2.py", src_dir_fd=1)

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

@@ -1039,14 +1039,10 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
                 flake8_simplify::rules::zip_dict_keys_and_values(checker, call);
             }
             if checker.any_rule_enabled(&[
-                Rule::OsChmod,
                 Rule::OsMkdir,
                 Rule::OsMakedirs,
-                Rule::OsRename,
-                Rule::OsReplace,
                 Rule::OsStat,
                 Rule::OsPathJoin,
-                Rule::OsPathSamefile,
                 Rule::OsPathSplitext,
                 Rule::BuiltinOpen,
                 Rule::PyPath,
@@ -1112,6 +1108,18 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
                 if checker.is_rule_enabled(Rule::OsGetcwd) {
                     flake8_use_pathlib::rules::os_getcwd(checker, call, segments);
                 }
+                if checker.is_rule_enabled(Rule::OsChmod) {
+                    flake8_use_pathlib::rules::os_chmod(checker, call, segments);
+                }
+                if checker.is_rule_enabled(Rule::OsRename) {
+                    flake8_use_pathlib::rules::os_rename(checker, call, segments);
+                }
+                if checker.is_rule_enabled(Rule::OsReplace) {
+                    flake8_use_pathlib::rules::os_replace(checker, call, segments);
+                }
+                if checker.is_rule_enabled(Rule::OsPathSamefile) {
+                    flake8_use_pathlib::rules::os_path_samefile(checker, call, segments);
+                }
                 if checker.is_rule_enabled(Rule::PathConstructorCurrentDirectory) {
                     flake8_use_pathlib::rules::path_constructor_current_directory(
                         checker, call, segments,

crates/ruff_linter/src/codes.rs

@@ -920,11 +920,11 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
 
         // flake8-use-pathlib
         (Flake8UsePathlib, "100") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathAbspath),
-        (Flake8UsePathlib, "101") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsChmod),
+        (Flake8UsePathlib, "101") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsChmod),
         (Flake8UsePathlib, "102") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsMkdir),
         (Flake8UsePathlib, "103") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsMakedirs),
-        (Flake8UsePathlib, "104") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsRename),
-        (Flake8UsePathlib, "105") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsReplace),
+        (Flake8UsePathlib, "104") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsRename),
+        (Flake8UsePathlib, "105") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsReplace),
         (Flake8UsePathlib, "106") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsRmdir),
         (Flake8UsePathlib, "107") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsRemove),
         (Flake8UsePathlib, "108") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsUnlink),
@@ -940,7 +940,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8UsePathlib, "118") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsPathJoin),
         (Flake8UsePathlib, "119") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathBasename),
         (Flake8UsePathlib, "120") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathDirname),
-        (Flake8UsePathlib, "121") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsPathSamefile),
+        (Flake8UsePathlib, "121") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathSamefile),
         (Flake8UsePathlib, "122") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsPathSplitext),
         (Flake8UsePathlib, "123") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::BuiltinOpen),
         (Flake8UsePathlib, "124") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::PyPath),

crates/ruff_linter/src/preview.rs

@@ -134,6 +134,26 @@ pub(crate) const fn is_fix_os_path_dirname_enabled(settings: &LinterSettings) ->
     settings.preview.is_enabled()
 }
 
+// https://github.com/astral-sh/ruff/pull/19404
+pub(crate) const fn is_fix_os_chmod_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}
+
+// https://github.com/astral-sh/ruff/pull/19404
+pub(crate) const fn is_fix_os_rename_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}
+
+// https://github.com/astral-sh/ruff/pull/19404
+pub(crate) const fn is_fix_os_replace_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}
+
+// https://github.com/astral-sh/ruff/pull/19404
+pub(crate) const fn is_fix_os_path_samefile_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}
+
 // https://github.com/astral-sh/ruff/pull/19245
 pub(crate) const fn is_fix_os_getcwd_enabled(settings: &LinterSettings) -> bool {
     settings.preview.is_enabled()

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

@@ -1,8 +1,8 @@
 use crate::checkers::ast::Checker;
 use crate::importer::ImportRequest;
 use crate::{Applicability, Edit, Fix, Violation};
-use ruff_python_ast::{self as ast};
-use ruff_python_ast::{Expr, ExprCall};
+use ruff_python_ast::{self as ast, Expr, ExprCall};
+use ruff_python_semantic::{SemanticModel, analyze::typing};
 use ruff_text_size::Ranged;
 
 pub(crate) fn is_keyword_only_argument_non_default(arguments: &ast::Arguments, name: &str) -> bool {
@@ -72,3 +72,85 @@ pub(crate) fn check_os_pathlib_single_arg_calls(
         });
     }
 }
+
+pub(crate) fn get_name_expr(expr: &Expr) -> Option<&ast::ExprName> {
+    match expr {
+        Expr::Name(name) => Some(name),
+        Expr::Call(ExprCall { func, .. }) => get_name_expr(func),
+        _ => None,
+    }
+}
+
+/// Returns `true` if the given expression looks like a file descriptor, i.e., if it is an integer.
+pub(crate) fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool {
+    if matches!(
+        expr,
+        Expr::NumberLiteral(ast::ExprNumberLiteral {
+            value: ast::Number::Int(_),
+            ..
+        })
+    ) {
+        return true;
+    }
+
+    let Some(name) = get_name_expr(expr) else {
+        return false;
+    };
+
+    let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
+        return false;
+    };
+
+    typing::is_int(binding, semantic)
+}
+
+pub(crate) fn check_os_pathlib_two_arg_calls(
+    checker: &Checker,
+    call: &ExprCall,
+    attr: &str,
+    path_arg: &str,
+    second_arg: &str,
+    fix_enabled: bool,
+    violation: impl Violation,
+) {
+    let range = call.range();
+    let mut diagnostic = checker.report_diagnostic(violation, call.func.range());
+
+    let (Some(path_expr), Some(second_expr)) = (
+        call.arguments.find_argument_value(path_arg, 0),
+        call.arguments.find_argument_value(second_arg, 1),
+    ) else {
+        return;
+    };
+
+    let path_code = checker.locator().slice(path_expr.range());
+    let second_code = checker.locator().slice(second_expr.range());
+
+    if fix_enabled {
+        diagnostic.try_set_fix(|| {
+            let (import_edit, binding) = checker.importer().get_or_import_symbol(
+                &ImportRequest::import("pathlib", "Path"),
+                call.start(),
+                checker.semantic(),
+            )?;
+
+            let replacement = if is_pathlib_path_call(checker, path_expr) {
+                format!("{path_code}.{attr}({second_code})")
+            } else {
+                format!("{binding}({path_code}).{attr}({second_code})")
+            };
+
+            let applicability = if checker.comment_ranges().intersects(range) {
+                Applicability::Unsafe
+            } else {
+                Applicability::Safe
+            };
+
+            Ok(Fix::applicable_edits(
+                Edit::range_replacement(replacement, range),
+                [import_edit],
+                applicability,
+            ))
+        });
+    }
+}

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

@@ -1,5 +1,6 @@
 pub(crate) use glob_rule::*;
 pub(crate) use invalid_pathlib_with_suffix::*;
+pub(crate) use os_chmod::*;
 pub(crate) use os_getcwd::*;
 pub(crate) use os_path_abspath::*;
 pub(crate) use os_path_basename::*;
@@ -14,8 +15,11 @@ pub(crate) use os_path_isabs::*;
 pub(crate) use os_path_isdir::*;
 pub(crate) use os_path_isfile::*;
 pub(crate) use os_path_islink::*;
+pub(crate) use os_path_samefile::*;
 pub(crate) use os_readlink::*;
 pub(crate) use os_remove::*;
+pub(crate) use os_rename::*;
+pub(crate) use os_replace::*;
 pub(crate) use os_rmdir::*;
 pub(crate) use os_sep_split::*;
 pub(crate) use os_unlink::*;
@@ -24,6 +28,7 @@ pub(crate) use replaceable_by_pathlib::*;
 
 mod glob_rule;
 mod invalid_pathlib_with_suffix;
+mod os_chmod;
 mod os_getcwd;
 mod os_path_abspath;
 mod os_path_basename;
@@ -38,8 +43,11 @@ mod os_path_isabs;
 mod os_path_isdir;
 mod os_path_isfile;
 mod os_path_islink;
+mod os_path_samefile;
 mod os_readlink;
 mod os_remove;
+mod os_rename;
+mod os_replace;
 mod os_rmdir;
 mod os_sep_split;
 mod os_unlink;

crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_chmod.rs

@@ -0,0 +1,94 @@
+use crate::checkers::ast::Checker;
+use crate::preview::is_fix_os_chmod_enabled;
+use crate::rules::flake8_use_pathlib::helpers::{
+    check_os_pathlib_two_arg_calls, is_file_descriptor, is_keyword_only_argument_non_default,
+};
+use crate::{FixAvailability, Violation};
+use ruff_macros::{ViolationMetadata, derive_message_formats};
+use ruff_python_ast::ExprCall;
+
+/// ## What it does
+/// Checks for uses of `os.chmod`.
+///
+/// ## Why is this bad?
+/// `pathlib` offers a high-level API for path manipulation, as compared to
+/// the lower-level API offered by `os`. When possible, using `Path` object
+/// methods such as `Path.chmod()` can improve readability over the `os`
+/// module's counterparts (e.g., `os.chmod()`).
+///
+/// ## Examples
+/// ```python
+/// import os
+///
+/// os.chmod("file.py", 0o444)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// from pathlib import Path
+///
+/// Path("file.py").chmod(0o444)
+/// ```
+///
+/// ## Known issues
+/// While using `pathlib` can improve the readability and type safety of your code,
+/// it can be less performant than the lower-level alternatives that work directly with strings,
+/// especially on older versions of Python.
+///
+/// ## 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.chmod`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.chmod)
+/// - [Python documentation: `os.chmod`](https://docs.python.org/3/library/os.html#os.chmod)
+/// - [PEP 428 – The pathlib module – object-oriented filesystem paths](https://peps.python.org/pep-0428/)
+/// - [Correspondence between `os` and `pathlib`](https://docs.python.org/3/library/pathlib.html#correspondence-to-tools-in-the-os-module)
+/// - [Why you should be using pathlib](https://treyhunner.com/2018/12/why-you-should-be-using-pathlib/)
+/// - [No really, pathlib is great](https://treyhunner.com/2019/01/no-really-pathlib-is-great/)
+#[derive(ViolationMetadata)]
+pub(crate) struct OsChmod;
+
+impl Violation for OsChmod {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "`os.chmod()` should be replaced by `Path.chmod()`".to_string()
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with `Path(...).chmod(...)`".to_string())
+    }
+}
+
+/// PTH101
+pub(crate) fn os_chmod(checker: &Checker, call: &ExprCall, segments: &[&str]) {
+    if segments != ["os", "chmod"] {
+        return;
+    }
+
+    // `dir_fd` is not supported by pathlib, so check if it's set to non-default values.
+    // Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.chmod)
+    // ```text
+    //           0     1          2               3
+    // os.chmod(path, mode, *, dir_fd=None, follow_symlinks=True)
+    // ```
+    if call
+        .arguments
+        .find_argument_value("path", 0)
+        .is_some_and(|expr| is_file_descriptor(expr, checker.semantic()))
+        || is_keyword_only_argument_non_default(&call.arguments, "dir_fd")
+    {
+        return;
+    }
+
+    check_os_pathlib_two_arg_calls(
+        checker,
+        call,
+        "chmod",
+        "path",
+        "mode",
+        is_fix_os_chmod_enabled(checker.settings()),
+        OsChmod,
+    );
+}