This commit is contained in:
Dan Parizher 2025-11-16 18:29:33 +00:00 committed by GitHub
commit 3359b36e3d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 54 additions and 7 deletions

View file

@ -94,12 +94,22 @@ pub(crate) fn check_os_pathlib_single_arg_calls(
let fix = match applicability {
Some(Applicability::Unsafe) => Fix::unsafe_edits(edit, [import_edit]),
_ => {
let applicability = if checker.comment_ranges().intersects(range) {
let determined_applicability = if checker.comment_ranges().intersects(range) {
Applicability::Unsafe
} else if applicability.is_none() {
// When applicability is None, we need to determine it based on return type changes
if is_top_level_expression_call(checker, call) {
// Safe when the call is a top-level expression (return value not used)
Applicability::Safe
} else {
// Unsafe because the return type changes (str/bytes -> Path or None -> Path)
Applicability::Unsafe
}
} else {
// applicability is Some(Applicability::Safe), use it
Applicability::Safe
};
Fix::applicable_edits(edit, [import_edit], applicability)
Fix::applicable_edits(edit, [import_edit], determined_applicability)
}
};
@ -176,8 +186,12 @@ pub(crate) fn check_os_pathlib_two_arg_calls(
let applicability = if checker.comment_ranges().intersects(range) {
Applicability::Unsafe
} else {
} else if is_top_level_expression_call(checker, call) {
// Safe when the call is a top-level expression (return value not used)
Applicability::Safe
} else {
// Unsafe because the return type changes (None -> Path)
Applicability::Unsafe
};
Ok(Fix::applicable_edits(
@ -209,3 +223,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, _call: &ExprCall) -> bool {
checker.semantic().current_expression_parent().is_none()
}

View file

@ -1,6 +1,7 @@
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};
use ruff_diagnostics::{Applicability, Edit, Fix};
use ruff_macros::{ViolationMetadata, derive_message_formats};
@ -37,6 +38,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 +86,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, call)
{
Applicability::Unsafe
} else {
Applicability::Safe

View file

@ -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 a `str`, 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)

View file

@ -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 a `str`, 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,

View file

@ -5,6 +5,7 @@ use crate::checkers::ast::Checker;
use crate::preview::is_fix_os_path_exists_enabled;
use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls;
use crate::{FixAvailability, Violation};
use ruff_diagnostics::Applicability;
/// ## What it does
/// Checks for uses of `os.path.exists`.
@ -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,
Some(Applicability::Safe),
);
}

View file

@ -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 a `str`, 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)

View file

@ -5,6 +5,7 @@ use crate::checkers::ast::Checker;
use crate::preview::is_fix_os_path_isfile_enabled;
use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls;
use crate::{FixAvailability, Violation};
use ruff_diagnostics::Applicability;
/// ## What it does
/// Checks for uses of `os.path.isfile`.
@ -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,
Some(Applicability::Safe),
);
}

View file

@ -5,6 +5,7 @@ use crate::checkers::ast::Checker;
use crate::preview::is_fix_os_path_islink_enabled;
use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls;
use crate::{FixAvailability, Violation};
use ruff_diagnostics::Applicability;
/// ## What it does
/// Checks for uses of `os.path.islink`.
@ -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,
Some(Applicability::Safe),
);
}

View file

@ -38,6 +38,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` to a `Path` object.
///
/// ## References
/// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline)

View file

@ -38,6 +38,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)

View file

@ -41,6 +41,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)