From 89258f1938f142a3e63aac06c101eb09ff77bc6e Mon Sep 17 00:00:00 2001 From: chiri Date: Wed, 23 Jul 2025 19:13:43 +0300 Subject: [PATCH] [`flake8-use-pathlib`] Add autofix for `PTH101`, `PTH104`, `PTH105`, `PTH121` (#19404) ## Summary Part of https://github.com/astral-sh/ruff/issues/2331 ## Test Plan `cargo nextest run flake8_use_pathlib` --- .../flake8_use_pathlib/import_from.py | 16 ++ .../src/checkers/ast/analyze/expression.rs | 16 +- crates/ruff_linter/src/codes.rs | 8 +- crates/ruff_linter/src/preview.rs | 20 ++ .../src/rules/flake8_use_pathlib/helpers.rs | 86 +++++++- .../src/rules/flake8_use_pathlib/rules/mod.rs | 8 + .../flake8_use_pathlib/rules/os_chmod.rs | 94 +++++++++ .../rules/os_path_samefile.rs | 77 ++++++++ .../flake8_use_pathlib/rules/os_rename.rs | 91 +++++++++ .../flake8_use_pathlib/rules/os_replace.rs | 94 +++++++++ .../rules/replaceable_by_pathlib.rs | 99 +--------- ...ake8_use_pathlib__tests__full_name.py.snap | 4 + ...ake8_use_pathlib__tests__import_as.py.snap | 4 + ...e8_use_pathlib__tests__import_from.py.snap | 37 ++++ ...use_pathlib__tests__import_from_as.py.snap | 4 + ..._pathlib__tests__preview_full_name.py.snap | 4 + ..._pathlib__tests__preview_import_as.py.snap | 4 + ...athlib__tests__preview_import_from.py.snap | 96 +++++++++ ...lib__tests__preview_import_from_as.py.snap | 4 + .../rules/flake8_use_pathlib/violations.rs | 183 ------------------ 20 files changed, 666 insertions(+), 283 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_chmod.rs create mode 100644 crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs create mode 100644 crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs create mode 100644 crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/import_from.py b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/import_from.py index 2a640623ed..6f5b08f811 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/import_from.py +++ b/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) \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 00579dda66..b5270b4f97 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/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, diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 0033254cdb..5addc9592f 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/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), diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index 6951b3727d..23293bdb8d 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/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() diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index d8e1fbe0d7..82f7492beb 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/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, + )) + }); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/mod.rs index e7c0e84bd3..b1412707e9 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/mod.rs +++ b/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; diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_chmod.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_chmod.rs new file mode 100644 index 0000000000..cca89e7278 --- /dev/null +++ b/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 { + 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, + ); +} diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs new file mode 100644 index 0000000000..f761de962c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs @@ -0,0 +1,77 @@ +use crate::checkers::ast::Checker; +use crate::preview::is_fix_os_path_samefile_enabled; +use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_two_arg_calls; +use crate::{FixAvailability, Violation}; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + +/// ## What it does +/// Checks for uses of `os.path.samefile`. +/// +/// ## Why is this bad? +/// `pathlib` offers a high-level API for path manipulation, as compared to +/// the lower-level API offered by `os.path`. When possible, using `Path` object +/// methods such as `Path.samefile()` can improve readability over the `os.path` +/// module's counterparts (e.g., `os.path.samefile()`). +/// +/// ## Examples +/// ```python +/// import os +/// +/// os.path.samefile("f1.py", "f2.py") +/// ``` +/// +/// Use instead: +/// ```python +/// from pathlib import Path +/// +/// Path("f1.py").samefile("f2.py") +/// ``` +/// +/// ## 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.samefile`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.samefile) +/// - [Python documentation: `os.path.samefile`](https://docs.python.org/3/library/os.path.html#os.path.samefile) +/// - [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 OsPathSamefile; + +impl Violation for OsPathSamefile { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + "`os.path.samefile()` should be replaced by `Path.samefile()`".to_string() + } + + fn fix_title(&self) -> Option { + Some("Replace with `Path(...).samefile()`".to_string()) + } +} + +/// PTH121 +pub(crate) fn os_path_samefile(checker: &Checker, call: &ExprCall, segments: &[&str]) { + if segments != ["os", "path", "samefile"] { + return; + } + + check_os_pathlib_two_arg_calls( + checker, + call, + "samefile", + "f1", + "f2", + is_fix_os_path_samefile_enabled(checker.settings()), + OsPathSamefile, + ); +} diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs new file mode 100644 index 0000000000..63e9ac0d60 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs @@ -0,0 +1,91 @@ +use crate::checkers::ast::Checker; +use crate::preview::is_fix_os_rename_enabled; +use crate::rules::flake8_use_pathlib::helpers::{ + check_os_pathlib_two_arg_calls, 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.rename`. +/// +/// ## 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.rename()` can improve readability over the `os` +/// module's counterparts (e.g., `os.rename()`). +/// +/// ## Examples +/// ```python +/// import os +/// +/// os.rename("old.py", "new.py") +/// ``` +/// +/// Use instead: +/// ```python +/// from pathlib import Path +/// +/// Path("old.py").rename("new.py") +/// ``` +/// +/// ## 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.rename`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.rename) +/// - [Python documentation: `os.rename`](https://docs.python.org/3/library/os.html#os.rename) +/// - [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 OsRename; + +impl Violation for OsRename { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + "`os.rename()` should be replaced by `Path.rename()`".to_string() + } + + fn fix_title(&self) -> Option { + Some("Replace with `Path(...).rename(...)`".to_string()) + } +} + +/// PTH104 +pub(crate) fn os_rename(checker: &Checker, call: &ExprCall, segments: &[&str]) { + if segments != ["os", "rename"] { + return; + } + // `src_dir_fd` and `dst_dir_fd` are not supported by pathlib, so check if they are + // set to non-default values. + // Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.rename) + // ```text + // 0 1 2 3 + // os.rename(src, dst, *, src_dir_fd=None, dst_dir_fd=None) + // ``` + if is_keyword_only_argument_non_default(&call.arguments, "src_dir_fd") + || is_keyword_only_argument_non_default(&call.arguments, "dst_dir_fd") + { + return; + } + + check_os_pathlib_two_arg_calls( + checker, + call, + "rename", + "src", + "dst", + is_fix_os_rename_enabled(checker.settings()), + OsRename, + ); +} diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs new file mode 100644 index 0000000000..a63da9dc37 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs @@ -0,0 +1,94 @@ +use crate::checkers::ast::Checker; +use crate::preview::is_fix_os_replace_enabled; +use crate::rules::flake8_use_pathlib::helpers::{ + check_os_pathlib_two_arg_calls, 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.replace`. +/// +/// ## 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.replace()` can improve readability over the `os` +/// module's counterparts (e.g., `os.replace()`). +/// +/// Note that `os` functions may be preferable if performance is a concern, +/// e.g., in hot loops. +/// +/// ## Examples +/// ```python +/// import os +/// +/// os.replace("old.py", "new.py") +/// ``` +/// +/// Use instead: +/// ```python +/// from pathlib import Path +/// +/// Path("old.py").replace("new.py") +/// ``` +/// +/// ## 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.replace`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.replace) +/// - [Python documentation: `os.replace`](https://docs.python.org/3/library/os.html#os.replace) +/// - [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 OsReplace; + +impl Violation for OsReplace { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + "`os.replace()` should be replaced by `Path.replace()`".to_string() + } + + fn fix_title(&self) -> Option { + Some("Replace with `Path(...).replace(...)`".to_string()) + } +} + +/// PTH105 +pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str]) { + if segments != ["os", "replace"] { + return; + } + // `src_dir_fd` and `dst_dir_fd` are not supported by pathlib, so check if they are + // set to non-default values. + // Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.replace) + // ```text + // 0 1 2 3 + // os.replace(src, dst, *, src_dir_fd=None, dst_dir_fd=None) + // ``` + if is_keyword_only_argument_non_default(&call.arguments, "src_dir_fd") + || is_keyword_only_argument_non_default(&call.arguments, "dst_dir_fd") + { + return; + } + + check_os_pathlib_two_arg_calls( + checker, + call, + "replace", + "src", + "dst", + is_fix_os_replace_enabled(checker.settings()), + OsReplace, + ); +} diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs index a076c2ccad..65026a0108 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs @@ -1,14 +1,16 @@ use ruff_python_ast::{self as ast, Expr, ExprBooleanLiteral, ExprCall}; -use ruff_python_semantic::SemanticModel; -use ruff_python_semantic::analyze::typing; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; -use crate::rules::flake8_use_pathlib::helpers::is_keyword_only_argument_non_default; -use crate::rules::flake8_use_pathlib::rules::Glob; -use crate::rules::flake8_use_pathlib::violations::{ - BuiltinOpen, Joiner, OsChmod, OsListdir, OsMakedirs, OsMkdir, OsPathJoin, OsPathSamefile, - OsPathSplitext, OsRename, OsReplace, OsStat, OsSymlink, PyPath, +use crate::rules::flake8_use_pathlib::helpers::{ + is_file_descriptor, is_keyword_only_argument_non_default, +}; +use crate::rules::flake8_use_pathlib::{ + rules::Glob, + violations::{ + BuiltinOpen, Joiner, OsListdir, OsMakedirs, OsMkdir, OsPathJoin, OsPathSplitext, OsStat, + OsSymlink, PyPath, + }, }; pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { @@ -18,24 +20,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { let range = call.func.range(); match qualified_name.segments() { - // PTH101 - ["os", "chmod"] => { - // `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; - } - checker.report_diagnostic_if_enabled(OsChmod, range) - } // PTH102 ["os", "makedirs"] => checker.report_diagnostic_if_enabled(OsMakedirs, range), // PTH103 @@ -51,38 +35,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { } checker.report_diagnostic_if_enabled(OsMkdir, range) } - // PTH104 - ["os", "rename"] => { - // `src_dir_fd` and `dst_dir_fd` are not supported by pathlib, so check if they are - // set to non-default values. - // Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.rename) - // ```text - // 0 1 2 3 - // os.rename(src, dst, *, src_dir_fd=None, dst_dir_fd=None) - // ``` - if is_keyword_only_argument_non_default(&call.arguments, "src_dir_fd") - || is_keyword_only_argument_non_default(&call.arguments, "dst_dir_fd") - { - return; - } - checker.report_diagnostic_if_enabled(OsRename, range) - } - // PTH105 - ["os", "replace"] => { - // `src_dir_fd` and `dst_dir_fd` are not supported by pathlib, so check if they are - // set to non-default values. - // Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.replace) - // ```text - // 0 1 2 3 - // os.replace(src, dst, *, src_dir_fd=None, dst_dir_fd=None) - // ``` - if is_keyword_only_argument_non_default(&call.arguments, "src_dir_fd") - || is_keyword_only_argument_non_default(&call.arguments, "dst_dir_fd") - { - return; - } - checker.report_diagnostic_if_enabled(OsReplace, range) - } // PTH116 ["os", "stat"] => { // `dir_fd` is not supported by pathlib, so check if it's set to non-default values. @@ -124,8 +76,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { }, range, ), - // PTH121 - ["os", "path", "samefile"] => checker.report_diagnostic_if_enabled(OsPathSamefile, range), // PTH122 ["os", "path", "splitext"] => checker.report_diagnostic_if_enabled(OsPathSplitext, range), // PTH211 @@ -234,37 +184,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { }; } -/// Returns `true` if the given expression looks like a file descriptor, i.e., if it is an integer. -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) -} - -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 argument `name` is set to a non-default `None` value. fn is_argument_non_default(arguments: &ast::Arguments, name: &str, position: usize) -> bool { arguments diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap index 0c85f258cc..a1e7f26c6f 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap @@ -20,6 +20,7 @@ full_name.py:8:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 9 | aaa = os.mkdir(p) 10 | os.makedirs(p) | + = help: Replace with `Path(...).chmod(...)` full_name.py:9:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -50,6 +51,7 @@ full_name.py:11:1: PTH104 `os.rename()` should be replaced by `Path.rename()` 12 | os.replace(p) 13 | os.rmdir(p) | + = help: Replace with `Path(...).rename(...)` full_name.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -60,6 +62,7 @@ full_name.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` 13 | os.rmdir(p) 14 | os.remove(p) | + = help: Replace with `Path(...).replace(...)` full_name.py:13:1: PTH106 `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -253,6 +256,7 @@ full_name.py:30:1: PTH121 `os.path.samefile()` should be replaced by `Path.samef 31 | os.path.splitext(p) 32 | with open(p) as fp: | + = help: Replace with `Path(...).samefile()` full_name.py:31:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_as.py.snap index 17d84d50c7..b3571f0799 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_as.py.snap @@ -20,6 +20,7 @@ import_as.py:8:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 9 | aaa = foo.mkdir(p) 10 | foo.makedirs(p) | + = help: Replace with `Path(...).chmod(...)` import_as.py:9:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -50,6 +51,7 @@ import_as.py:11:1: PTH104 `os.rename()` should be replaced by `Path.rename()` 12 | foo.replace(p) 13 | foo.rmdir(p) | + = help: Replace with `Path(...).rename(...)` import_as.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -60,6 +62,7 @@ import_as.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` 13 | foo.rmdir(p) 14 | foo.remove(p) | + = help: Replace with `Path(...).replace(...)` import_as.py:13:1: PTH106 `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -252,6 +255,7 @@ import_as.py:30:1: PTH121 `os.path.samefile()` should be replaced by `Path.samef | ^^^^^^^^^^^^^^ PTH121 31 | foo_p.splitext(p) | + = help: Replace with `Path(...).samefile()` import_as.py:31:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from.py.snap index a145e3ed54..7fdfa8ad30 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from.py.snap @@ -20,6 +20,7 @@ import_from.py:10:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 11 | aaa = mkdir(p) 12 | makedirs(p) | + = help: Replace with `Path(...).chmod(...)` import_from.py:11:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -50,6 +51,7 @@ import_from.py:13:1: PTH104 `os.rename()` should be replaced by `Path.rename()` 14 | replace(p) 15 | rmdir(p) | + = help: Replace with `Path(...).rename(...)` import_from.py:14:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -60,6 +62,7 @@ import_from.py:14:1: PTH105 `os.replace()` should be replaced by `Path.replace() 15 | rmdir(p) 16 | remove(p) | + = help: Replace with `Path(...).replace(...)` import_from.py:15:1: PTH106 `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -253,6 +256,7 @@ import_from.py:32:1: PTH121 `os.path.samefile()` should be replaced by `Path.sam 33 | splitext(p) 34 | with open(p) as fp: | + = help: Replace with `Path(...).samefile()` import_from.py:33:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | @@ -289,3 +293,36 @@ import_from.py:43:10: PTH123 `open()` should be replaced by `Path.open()` 43 | with open(p) as _: ... # Error | ^^^^ PTH123 | + +import_from.py:53:1: PTH104 `os.rename()` should be replaced by `Path.rename()` + | +51 | file = "file_1.py" +52 | +53 | rename(file, "file_2.py") + | ^^^^^^ PTH104 +54 | +55 | rename( + | + = help: Replace with `Path(...).rename(...)` + +import_from.py:55:1: PTH104 `os.rename()` should be replaced by `Path.rename()` + | +53 | rename(file, "file_2.py") +54 | +55 | rename( + | ^^^^^^ PTH104 +56 | # commment 1 +57 | file, # comment 2 + | + = help: Replace with `Path(...).rename(...)` + +import_from.py:63:1: PTH104 `os.rename()` should be replaced by `Path.rename()` + | +61 | ) +62 | +63 | rename(file, "file_2.py", src_dir_fd=None, dst_dir_fd=None) + | ^^^^^^ PTH104 +64 | +65 | rename(file, "file_2.py", src_dir_fd=1) + | + = help: Replace with `Path(...).rename(...)` diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from_as.py.snap index b7136fc442..8bc86717ac 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from_as.py.snap @@ -20,6 +20,7 @@ import_from_as.py:15:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 16 | aaa = xmkdir(p) 17 | xmakedirs(p) | + = help: Replace with `Path(...).chmod(...)` import_from_as.py:16:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -50,6 +51,7 @@ import_from_as.py:18:1: PTH104 `os.rename()` should be replaced by `Path.rename( 19 | xreplace(p) 20 | xrmdir(p) | + = help: Replace with `Path(...).rename(...)` import_from_as.py:19:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -60,6 +62,7 @@ import_from_as.py:19:1: PTH105 `os.replace()` should be replaced by `Path.replac 20 | xrmdir(p) 21 | xremove(p) | + = help: Replace with `Path(...).replace(...)` import_from_as.py:20:1: PTH106 `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -252,6 +255,7 @@ import_from_as.py:37:1: PTH121 `os.path.samefile()` should be replaced by `Path. | ^^^^^^^^^ PTH121 38 | xsplitext(p) | + = help: Replace with `Path(...).samefile()` import_from_as.py:38:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap index 02bc16382f..ca7f6d6967 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap @@ -34,6 +34,7 @@ full_name.py:8:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 9 | aaa = os.mkdir(p) 10 | os.makedirs(p) | + = help: Replace with `Path(...).chmod(...)` full_name.py:9:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -64,6 +65,7 @@ full_name.py:11:1: PTH104 `os.rename()` should be replaced by `Path.rename()` 12 | os.replace(p) 13 | os.rmdir(p) | + = help: Replace with `Path(...).rename(...)` full_name.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -74,6 +76,7 @@ full_name.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` 13 | os.rmdir(p) 14 | os.remove(p) | + = help: Replace with `Path(...).replace(...)` full_name.py:13:1: PTH106 [*] `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -471,6 +474,7 @@ full_name.py:30:1: PTH121 `os.path.samefile()` should be replaced by `Path.samef 31 | os.path.splitext(p) 32 | with open(p) as fp: | + = help: Replace with `Path(...).samefile()` full_name.py:31:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap index 2df02db6e5..bd5e1ca5ba 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap @@ -34,6 +34,7 @@ import_as.py:8:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 9 | aaa = foo.mkdir(p) 10 | foo.makedirs(p) | + = help: Replace with `Path(...).chmod(...)` import_as.py:9:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -64,6 +65,7 @@ import_as.py:11:1: PTH104 `os.rename()` should be replaced by `Path.rename()` 12 | foo.replace(p) 13 | foo.rmdir(p) | + = help: Replace with `Path(...).rename(...)` import_as.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -74,6 +76,7 @@ import_as.py:12:1: PTH105 `os.replace()` should be replaced by `Path.replace()` 13 | foo.rmdir(p) 14 | foo.remove(p) | + = help: Replace with `Path(...).replace(...)` import_as.py:13:1: PTH106 [*] `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -469,6 +472,7 @@ import_as.py:30:1: PTH121 `os.path.samefile()` should be replaced by `Path.samef | ^^^^^^^^^^^^^^ PTH121 31 | foo_p.splitext(p) | + = help: Replace with `Path(...).samefile()` import_as.py:31:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap index 2caa7965ab..d4087514c9 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap @@ -35,6 +35,7 @@ import_from.py:10:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 11 | aaa = mkdir(p) 12 | makedirs(p) | + = help: Replace with `Path(...).chmod(...)` import_from.py:11:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -65,6 +66,7 @@ import_from.py:13:1: PTH104 `os.rename()` should be replaced by `Path.rename()` 14 | replace(p) 15 | rmdir(p) | + = help: Replace with `Path(...).rename(...)` import_from.py:14:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -75,6 +77,7 @@ import_from.py:14:1: PTH105 `os.replace()` should be replaced by `Path.replace() 15 | rmdir(p) 16 | remove(p) | + = help: Replace with `Path(...).replace(...)` import_from.py:15:1: PTH106 [*] `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -484,6 +487,7 @@ import_from.py:32:1: PTH121 `os.path.samefile()` should be replaced by `Path.sam 33 | splitext(p) 34 | with open(p) as fp: | + = help: Replace with `Path(...).samefile()` import_from.py:33:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | @@ -520,3 +524,95 @@ import_from.py:43:10: PTH123 `open()` should be replaced by `Path.open()` 43 | with open(p) as _: ... # Error | ^^^^ PTH123 | + +import_from.py:53:1: PTH104 [*] `os.rename()` should be replaced by `Path.rename()` + | +51 | file = "file_1.py" +52 | +53 | rename(file, "file_2.py") + | ^^^^^^ PTH104 +54 | +55 | rename( + | + = help: Replace with `Path(...).rename(...)` + +ℹ Safe fix +2 2 | from os import remove, unlink, getcwd, readlink, stat +3 3 | from os.path import abspath, exists, expanduser, isdir, isfile, islink +4 4 | from os.path import isabs, join, basename, dirname, samefile, splitext + 5 |+import pathlib +5 6 | +6 7 | p = "/foo" +7 8 | q = "bar" +-------------------------------------------------------------------------------- +50 51 | +51 52 | file = "file_1.py" +52 53 | +53 |-rename(file, "file_2.py") + 54 |+pathlib.Path(file).rename("file_2.py") +54 55 | +55 56 | rename( +56 57 | # commment 1 + +import_from.py:55:1: PTH104 [*] `os.rename()` should be replaced by `Path.rename()` + | +53 | rename(file, "file_2.py") +54 | +55 | rename( + | ^^^^^^ PTH104 +56 | # commment 1 +57 | file, # comment 2 + | + = help: Replace with `Path(...).rename(...)` + +ℹ Unsafe fix +2 2 | from os import remove, unlink, getcwd, readlink, stat +3 3 | from os.path import abspath, exists, expanduser, isdir, isfile, islink +4 4 | from os.path import isabs, join, basename, dirname, samefile, splitext + 5 |+import pathlib +5 6 | +6 7 | p = "/foo" +7 8 | q = "bar" +-------------------------------------------------------------------------------- +52 53 | +53 54 | rename(file, "file_2.py") +54 55 | +55 |-rename( +56 |- # commment 1 +57 |- file, # comment 2 +58 |- "file_2.py" +59 |- , +60 |- # comment 3 +61 |-) + 56 |+pathlib.Path(file).rename("file_2.py") +62 57 | +63 58 | rename(file, "file_2.py", src_dir_fd=None, dst_dir_fd=None) +64 59 | + +import_from.py:63:1: PTH104 [*] `os.rename()` should be replaced by `Path.rename()` + | +61 | ) +62 | +63 | rename(file, "file_2.py", src_dir_fd=None, dst_dir_fd=None) + | ^^^^^^ PTH104 +64 | +65 | rename(file, "file_2.py", src_dir_fd=1) + | + = help: Replace with `Path(...).rename(...)` + +ℹ Safe fix +2 2 | from os import remove, unlink, getcwd, readlink, stat +3 3 | from os.path import abspath, exists, expanduser, isdir, isfile, islink +4 4 | from os.path import isabs, join, basename, dirname, samefile, splitext + 5 |+import pathlib +5 6 | +6 7 | p = "/foo" +7 8 | q = "bar" +-------------------------------------------------------------------------------- +60 61 | # comment 3 +61 62 | ) +62 63 | +63 |-rename(file, "file_2.py", src_dir_fd=None, dst_dir_fd=None) + 64 |+pathlib.Path(file).rename("file_2.py") +64 65 | +65 66 | rename(file, "file_2.py", src_dir_fd=1) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap index 78c341e4d6..7bfd51c194 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap @@ -35,6 +35,7 @@ import_from_as.py:15:6: PTH101 `os.chmod()` should be replaced by `Path.chmod()` 16 | aaa = xmkdir(p) 17 | xmakedirs(p) | + = help: Replace with `Path(...).chmod(...)` import_from_as.py:16:7: PTH102 `os.mkdir()` should be replaced by `Path.mkdir()` | @@ -65,6 +66,7 @@ import_from_as.py:18:1: PTH104 `os.rename()` should be replaced by `Path.rename( 19 | xreplace(p) 20 | xrmdir(p) | + = help: Replace with `Path(...).rename(...)` import_from_as.py:19:1: PTH105 `os.replace()` should be replaced by `Path.replace()` | @@ -75,6 +77,7 @@ import_from_as.py:19:1: PTH105 `os.replace()` should be replaced by `Path.replac 20 | xrmdir(p) 21 | xremove(p) | + = help: Replace with `Path(...).replace(...)` import_from_as.py:20:1: PTH106 [*] `os.rmdir()` should be replaced by `Path.rmdir()` | @@ -482,6 +485,7 @@ import_from_as.py:37:1: PTH121 `os.path.samefile()` should be replaced by `Path. | ^^^^^^^^^ PTH121 38 | xsplitext(p) | + = help: Replace with `Path(...).samefile()` import_from_as.py:38:1: PTH122 `os.path.splitext()` should be replaced by `Path.suffix`, `Path.stem`, and `Path.parent` | diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs index 82d4e6bf70..e9d7419737 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs @@ -2,51 +2,6 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use crate::Violation; -/// ## 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. -/// -/// ## 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 { - #[derive_message_formats] - fn message(&self) -> String { - "`os.chmod()` should be replaced by `Path.chmod()`".to_string() - } -} - /// ## What it does /// Checks for uses of `os.makedirs`. /// @@ -137,99 +92,6 @@ impl Violation for OsMkdir { } } -/// ## What it does -/// Checks for uses of `os.rename`. -/// -/// ## 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.rename()` can improve readability over the `os` -/// module's counterparts (e.g., `os.rename()`). -/// -/// ## Examples -/// ```python -/// import os -/// -/// os.rename("old.py", "new.py") -/// ``` -/// -/// Use instead: -/// ```python -/// from pathlib import Path -/// -/// Path("old.py").rename("new.py") -/// ``` -/// -/// ## 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. -/// -/// ## References -/// - [Python documentation: `Path.rename`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.rename) -/// - [Python documentation: `os.rename`](https://docs.python.org/3/library/os.html#os.rename) -/// - [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 OsRename; - -impl Violation for OsRename { - #[derive_message_formats] - fn message(&self) -> String { - "`os.rename()` should be replaced by `Path.rename()`".to_string() - } -} - -/// ## What it does -/// Checks for uses of `os.replace`. -/// -/// ## 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.replace()` can improve readability over the `os` -/// module's counterparts (e.g., `os.replace()`). -/// -/// Note that `os` functions may be preferable if performance is a concern, -/// e.g., in hot loops. -/// -/// ## Examples -/// ```python -/// import os -/// -/// os.replace("old.py", "new.py") -/// ``` -/// -/// Use instead: -/// ```python -/// from pathlib import Path -/// -/// Path("old.py").replace("new.py") -/// ``` -/// -/// ## 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. -/// -/// ## References -/// - [Python documentation: `Path.replace`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.replace) -/// - [Python documentation: `os.replace`](https://docs.python.org/3/library/os.html#os.replace) -/// - [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 OsReplace; - -impl Violation for OsReplace { - #[derive_message_formats] - fn message(&self) -> String { - "`os.replace()` should be replaced by `Path.replace()`".to_string() - } -} - /// ## What it does /// Checks for uses of `os.stat`. /// @@ -347,51 +209,6 @@ pub(crate) enum Joiner { Joinpath, } -/// ## What it does -/// Checks for uses of `os.path.samefile`. -/// -/// ## Why is this bad? -/// `pathlib` offers a high-level API for path manipulation, as compared to -/// the lower-level API offered by `os.path`. When possible, using `Path` object -/// methods such as `Path.samefile()` can improve readability over the `os.path` -/// module's counterparts (e.g., `os.path.samefile()`). -/// -/// ## Examples -/// ```python -/// import os -/// -/// os.path.samefile("f1.py", "f2.py") -/// ``` -/// -/// Use instead: -/// ```python -/// from pathlib import Path -/// -/// Path("f1.py").samefile("f2.py") -/// ``` -/// -/// ## 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. -/// -/// ## References -/// - [Python documentation: `Path.samefile`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.samefile) -/// - [Python documentation: `os.path.samefile`](https://docs.python.org/3/library/os.path.html#os.path.samefile) -/// - [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 OsPathSamefile; - -impl Violation for OsPathSamefile { - #[derive_message_formats] - fn message(&self) -> String { - "`os.path.samefile()` should be replaced by `Path.samefile()`".to_string() - } -} - /// ## What it does /// Checks for uses of `os.path.splitext`. ///