From bc44dc2afb52b8e2da527e07c6d499173ec4f210 Mon Sep 17 00:00:00 2001 From: Dan Parizher <105245560+danparizher@users.noreply.github.com> Date: Mon, 1 Dec 2025 15:26:55 -0500 Subject: [PATCH] [`flake8-use-pathlib`] Mark fixes unsafe for return type changes (`PTH104`, `PTH105`, `PTH109`, `PTH115`) (#21440) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Marks fixes as unsafe when they change return types (`None` → `Path`, `str`/`bytes` → `Path`, `str` → `Path`), except when the call is a top-level expression. Fixes #21431. ## Problem Fixes for `os.rename`, `os.replace`, `os.getcwd`/`os.getcwdb`, and `os.readlink` were marked safe despite changing return types, which can break code that uses the return value. ## Approach Added `is_top_level_expression_call` helper to detect when a call is a top-level expression (return value unused). Updated `check_os_pathlib_two_arg_calls` and `check_os_pathlib_single_arg_calls` to mark fixes as unsafe unless the call is a top-level expression. Updated PTH109 to use the helper for applicability determination. ## Test Plan Updated snapshots for `preview_full_name.py`, `preview_import_as.py`, `preview_import_from.py`, and `preview_import_from_as.py` to reflect unsafe markers. --------- Co-authored-by: Brent Westbrook --- .../src/rules/flake8_use_pathlib/helpers.rs | 34 +++++++++++-------- .../flake8_use_pathlib/rules/os_getcwd.rs | 17 +++++++--- .../rules/os_path_abspath.rs | 6 +++- .../rules/os_path_basename.rs | 2 +- .../rules/os_path_dirname.rs | 6 +++- .../rules/os_path_exists.rs | 3 +- .../rules/os_path_expanduser.rs | 6 +++- .../rules/os_path_getatime.rs | 3 +- .../rules/os_path_getctime.rs | 3 +- .../rules/os_path_getmtime.rs | 3 +- .../rules/os_path_getsize.rs | 3 +- .../flake8_use_pathlib/rules/os_path_isabs.rs | 3 +- .../flake8_use_pathlib/rules/os_path_isdir.rs | 3 +- .../rules/os_path_isfile.rs | 3 +- .../rules/os_path_islink.rs | 3 +- .../rules/os_path_samefile.rs | 7 ++-- .../flake8_use_pathlib/rules/os_readlink.rs | 13 ++++++- .../flake8_use_pathlib/rules/os_remove.rs | 3 +- .../flake8_use_pathlib/rules/os_rename.rs | 29 +++++++++++++--- .../flake8_use_pathlib/rules/os_replace.rs | 19 +++++++++-- .../flake8_use_pathlib/rules/os_rmdir.rs | 3 +- .../flake8_use_pathlib/rules/os_unlink.rs | 3 +- 22 files changed, 129 insertions(+), 46 deletions(-) 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 3ba017ecb7..24d9daee25 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -57,7 +57,7 @@ pub(crate) fn check_os_pathlib_single_arg_calls( fn_argument: &str, fix_enabled: bool, violation: impl Violation, - applicability: Option, + applicability: Applicability, ) { if call.arguments.len() != 1 { return; @@ -91,18 +91,14 @@ pub(crate) fn check_os_pathlib_single_arg_calls( let edit = Edit::range_replacement(replacement, range); - let fix = match applicability { - Some(Applicability::Unsafe) => Fix::unsafe_edits(edit, [import_edit]), - _ => { - let applicability = if checker.comment_ranges().intersects(range) { - Applicability::Unsafe - } else { - Applicability::Safe - }; - Fix::applicable_edits(edit, [import_edit], applicability) - } + let applicability = match applicability { + Applicability::DisplayOnly => Applicability::DisplayOnly, + _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe, + _ => applicability, }; + let fix = Fix::applicable_edits(edit, [import_edit], applicability); + Ok(fix) }); } @@ -138,6 +134,7 @@ pub(crate) fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool typing::is_int(binding, semantic) } +#[expect(clippy::too_many_arguments)] pub(crate) fn check_os_pathlib_two_arg_calls( checker: &Checker, call: &ExprCall, @@ -146,6 +143,7 @@ pub(crate) fn check_os_pathlib_two_arg_calls( second_arg: &str, fix_enabled: bool, violation: impl Violation, + applicability: Applicability, ) { let range = call.range(); let mut diagnostic = checker.report_diagnostic(violation, call.func.range()); @@ -174,10 +172,10 @@ pub(crate) fn check_os_pathlib_two_arg_calls( format!("{binding}({path_code}).{attr}({second_code})") }; - let applicability = if checker.comment_ranges().intersects(range) { - Applicability::Unsafe - } else { - Applicability::Safe + let applicability = match applicability { + Applicability::DisplayOnly => Applicability::DisplayOnly, + _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe, + _ => applicability, }; Ok(Fix::applicable_edits( @@ -209,3 +207,9 @@ pub(crate) fn is_argument_non_default(arguments: &Arguments, name: &str, positio .find_argument_value(name, position) .is_some_and(|expr| !expr.is_none_literal_expr()) } + +/// Returns `true` if the given call is a top-level expression in its statement. +/// This means the call's return value is not used, so return type changes don't matter. +pub(crate) fn is_top_level_expression_call(checker: &Checker) -> bool { + checker.semantic().current_expression_parent().is_none() +} diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs index 7bb533246d..4174b5825a 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs @@ -1,12 +1,14 @@ -use crate::checkers::ast::Checker; -use crate::importer::ImportRequest; -use crate::preview::is_fix_os_getcwd_enabled; -use crate::{FixAvailability, Violation}; use ruff_diagnostics::{Applicability, Edit, Fix}; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; use ruff_text_size::Ranged; +use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; +use crate::preview::is_fix_os_getcwd_enabled; +use crate::rules::flake8_use_pathlib::helpers::is_top_level_expression_call; +use crate::{FixAvailability, Violation}; + /// ## What it does /// Checks for uses of `os.getcwd` and `os.getcwdb`. /// @@ -37,6 +39,8 @@ use ruff_text_size::Ranged; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `str` or `bytes` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.cwd`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd) @@ -83,7 +87,10 @@ pub(crate) fn os_getcwd(checker: &Checker, call: &ExprCall, segments: &[&str]) { checker.semantic(), )?; - let applicability = if checker.comment_ranges().intersects(range) { + // Unsafe when the fix would delete comments or change a used return value + let applicability = if checker.comment_ranges().intersects(range) + || !is_top_level_expression_call(checker) + { Applicability::Unsafe } else { Applicability::Safe diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs index 9b58561e88..b51ce5cc6d 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs @@ -45,6 +45,10 @@ use crate::{FixAvailability, Violation}; /// behaviors is required, there's no existing `pathlib` alternative. See CPython issue /// [#69200](https://github.com/python/cpython/issues/69200). /// +/// Additionally, the fix is marked as unsafe because `os.path.abspath()` returns `str` or `bytes` (`AnyStr`), +/// while `Path.resolve()` returns a `Path` object. This change in return type can break code that uses +/// the return value. +/// /// ## References /// - [Python documentation: `Path.resolve`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.resolve) /// - [Python documentation: `os.path.abspath`](https://docs.python.org/3/library/os.path.html#os.path.abspath) @@ -85,6 +89,6 @@ pub(crate) fn os_path_abspath(checker: &Checker, call: &ExprCall, segments: &[&s "path", is_fix_os_path_abspath_enabled(checker.settings()), OsPathAbspath, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs index ca69d07ce3..c11c0ac114 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs @@ -82,6 +82,6 @@ pub(crate) fn os_path_basename(checker: &Checker, call: &ExprCall, segments: &[& "p", is_fix_os_path_basename_enabled(checker.settings()), OsPathBasename, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs index d3175c2035..69b44738f4 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs @@ -42,6 +42,10 @@ use crate::{FixAvailability, Violation}; /// As a result, code relying on the exact string returned by `os.path.dirname` /// may behave differently after the fix. /// +/// Additionally, the fix is marked as unsafe because `os.path.dirname()` returns `str` or `bytes` (`AnyStr`), +/// while `Path.parent` returns a `Path` object. This change in return type can break code that uses +/// the return value. +/// /// ## 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, @@ -82,6 +86,6 @@ pub(crate) fn os_path_dirname(checker: &Checker, call: &ExprCall, segments: &[&s "p", is_fix_os_path_dirname_enabled(checker.settings()), OsPathDirname, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs index f3fe32a641..2b130c72d0 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -72,6 +73,6 @@ pub(crate) fn os_path_exists(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_exists_enabled(checker.settings()), OsPathExists, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs index d544acde39..2b1fdb8980 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs @@ -41,6 +41,10 @@ use crate::{FixAvailability, Violation}; /// directory can't be resolved: `os.path.expanduser` returns the /// input unchanged, while `Path.expanduser` raises `RuntimeError`. /// +/// Additionally, the fix is marked as unsafe because `os.path.expanduser()` returns `str` or `bytes` (`AnyStr`), +/// while `Path.expanduser()` returns a `Path` object. This change in return type can break code that uses +/// the return value. +/// /// ## References /// - [Python documentation: `Path.expanduser`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.expanduser) /// - [Python documentation: `os.path.expanduser`](https://docs.python.org/3/library/os.path.html#os.path.expanduser) @@ -76,6 +80,6 @@ pub(crate) fn os_path_expanduser(checker: &Checker, call: &ExprCall, segments: & "path", is_fix_os_path_expanduser_enabled(checker.settings()), OsPathExpanduser, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs index 0f148f4033..eb8fd1f989 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -75,6 +76,6 @@ pub(crate) fn os_path_getatime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getatime_enabled(checker.settings()), OsPathGetatime, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs index 86bce28aed..3739391711 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -76,6 +77,6 @@ pub(crate) fn os_path_getctime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getctime_enabled(checker.settings()), OsPathGetctime, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs index 42e77e3fe9..2853a83986 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -76,6 +77,6 @@ pub(crate) fn os_path_getmtime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getmtime_enabled(checker.settings()), OsPathGetmtime, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs index a945b2224c..7c17e687df 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -76,6 +77,6 @@ pub(crate) fn os_path_getsize(checker: &Checker, call: &ExprCall, segments: &[&s "filename", is_fix_os_path_getsize_enabled(checker.settings()), OsPathGetsize, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs index b1c8cb33c3..0fcbdf3f06 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -71,6 +72,6 @@ pub(crate) fn os_path_isabs(checker: &Checker, call: &ExprCall, segments: &[&str "s", is_fix_os_path_isabs_enabled(checker.settings()), OsPathIsabs, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs index a2c1b8620f..9f0de09476 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -73,6 +74,6 @@ pub(crate) fn os_path_isdir(checker: &Checker, call: &ExprCall, segments: &[&str "s", is_fix_os_path_isdir_enabled(checker.settings()), OsPathIsdir, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs index d31e39eef7..fc723cbd2f 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -73,6 +74,6 @@ pub(crate) fn os_path_isfile(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_isfile_enabled(checker.settings()), OsPathIsfile, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs index d958a2c19c..f64aa7713b 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -73,6 +74,6 @@ pub(crate) fn os_path_islink(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_islink_enabled(checker.settings()), OsPathIslink, - None, + Applicability::Safe, ); } 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 index cbf6d7a034..af4ee0b605 100644 --- 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 @@ -1,11 +1,13 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + 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, has_unknown_keywords_or_starred_expr, }; 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`. @@ -79,5 +81,6 @@ pub(crate) fn os_path_samefile(checker: &Checker, call: &ExprCall, segments: &[& "f2", fix_enabled, OsPathSamefile, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs index d1df572ed5..1505e62a77 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{ExprCall, PythonVersion}; @@ -5,6 +6,7 @@ use crate::checkers::ast::Checker; use crate::preview::is_fix_os_readlink_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_single_arg_calls, is_keyword_only_argument_non_default, + is_top_level_expression_call, }; use crate::{FixAvailability, Violation}; @@ -38,6 +40,8 @@ use crate::{FixAvailability, Violation}; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `str` or `bytes` (`AnyStr`) to a `Path` object. /// /// ## References /// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline) @@ -82,6 +86,13 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str]) return; } + let applicability = if !is_top_level_expression_call(checker) { + // Unsafe because the return type changes (str/bytes -> Path) + Applicability::Unsafe + } else { + Applicability::Safe + }; + check_os_pathlib_single_arg_calls( checker, call, @@ -89,6 +100,6 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str]) "path", is_fix_os_readlink_enabled(checker.settings()), OsReadlink, - None, + applicability, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs index 43852e11e2..c25d52de21 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -84,6 +85,6 @@ pub(crate) fn os_remove(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_remove_enabled(checker.settings()), OsRemove, - None, + Applicability::Safe, ); } 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 index c5f2293ee9..523eada663 100644 --- 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 @@ -1,12 +1,14 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + 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, has_unknown_keywords_or_starred_expr, - is_keyword_only_argument_non_default, + is_keyword_only_argument_non_default, is_top_level_expression_call, }; 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`. @@ -38,6 +40,8 @@ use ruff_python_ast::ExprCall; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `None` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.rename`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.rename) @@ -87,5 +91,22 @@ pub(crate) fn os_rename(checker: &Checker, call: &ExprCall, segments: &[&str]) { &["src", "dst", "src_dir_fd", "dst_dir_fd"], ); - check_os_pathlib_two_arg_calls(checker, call, "rename", "src", "dst", fix_enabled, OsRename); + // Unsafe when the fix would delete comments or change a used return value + let applicability = if !is_top_level_expression_call(checker) { + // Unsafe because the return type changes (None -> Path) + Applicability::Unsafe + } else { + Applicability::Safe + }; + + check_os_pathlib_two_arg_calls( + checker, + call, + "rename", + "src", + "dst", + fix_enabled, + OsRename, + applicability, + ); } 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 index ef60099467..c1211a24a5 100644 --- 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 @@ -1,12 +1,14 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + 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, has_unknown_keywords_or_starred_expr, - is_keyword_only_argument_non_default, + is_keyword_only_argument_non_default, is_top_level_expression_call, }; 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`. @@ -41,6 +43,8 @@ use ruff_python_ast::ExprCall; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `None` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.replace`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.replace) @@ -90,6 +94,14 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str]) &["src", "dst", "src_dir_fd", "dst_dir_fd"], ); + // Unsafe when the fix would delete comments or change a used return value + let applicability = if !is_top_level_expression_call(checker) { + // Unsafe because the return type changes (None -> Path) + Applicability::Unsafe + } else { + Applicability::Safe + }; + check_os_pathlib_two_arg_calls( checker, call, @@ -98,5 +110,6 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str]) "dst", fix_enabled, OsReplace, + applicability, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs index a044e541b9..7d7a72812d 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -84,6 +85,6 @@ pub(crate) fn os_rmdir(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_rmdir_enabled(checker.settings()), OsRmdir, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs index 9f49025465..28568cf479 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -84,6 +85,6 @@ pub(crate) fn os_unlink(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_unlink_enabled(checker.settings()), OsUnlink, - None, + Applicability::Safe, ); }