Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,6 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
flake8_simplify::rules::zip_dict_keys_and_values(checker, call);
}
if checker.any_rule_enabled(&[
Rule::OsStat,
Rule::OsPathJoin,
Rule::OsPathSplitext,
Rule::PyPath,
Expand Down Expand Up @@ -1192,6 +1191,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
if checker.is_rule_enabled(Rule::OsMakedirs) {
flake8_use_pathlib::rules::os_makedirs(checker, call, segments);
}
if checker.is_rule_enabled(Rule::OsStat) {
flake8_use_pathlib::rules::os_stat(checker, call, segments);
}
if checker.is_rule_enabled(Rule::OsSymlink) {
flake8_use_pathlib::rules::os_symlink(checker, call, segments);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8UsePathlib, "113") => rules::flake8_use_pathlib::rules::OsPathIsfile,
(Flake8UsePathlib, "114") => rules::flake8_use_pathlib::rules::OsPathIslink,
(Flake8UsePathlib, "115") => rules::flake8_use_pathlib::rules::OsReadlink,
(Flake8UsePathlib, "116") => rules::flake8_use_pathlib::violations::OsStat,
(Flake8UsePathlib, "116") => rules::flake8_use_pathlib::rules::OsStat,
(Flake8UsePathlib, "117") => rules::flake8_use_pathlib::rules::OsPathIsabs,
(Flake8UsePathlib, "118") => rules::flake8_use_pathlib::violations::OsPathJoin,
(Flake8UsePathlib, "119") => rules::flake8_use_pathlib::rules::OsPathBasename,
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ pub(crate) const fn is_fix_os_makedirs_enabled(settings: &LinterSettings) -> boo
settings.preview.is_enabled()
}

// https://github.com/astral-sh/ruff/pull/26460
pub(crate) const fn is_fix_os_stat_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
}

// https://github.com/astral-sh/ruff/pull/20009
pub(crate) const fn is_fix_os_symlink_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
Expand Down
11 changes: 11 additions & 0 deletions crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,14 @@ pub(crate) fn is_top_level_expression_in_statement(checker: &Checker) -> bool {
checker.semantic().current_expression_parent().is_none()
&& checker.semantic().current_statement().is_expr_stmt()
}

pub(crate) fn is_keyword_true_or_default(argument: &Arguments, name: &str) -> Option<bool> {
let Some(kw) = argument.find_keyword(name) else {
return Some(true);
};

match &kw.value {
Expr::BooleanLiteral(ast::ExprBooleanLiteral { value, .. }) => Some(*value),
_ => None,
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_use_pathlib/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ 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_stat::*;
pub(crate) use os_symlink::*;
pub(crate) use os_unlink::*;
pub(crate) use path_constructor_current_directory::*;
Expand Down Expand Up @@ -57,6 +58,7 @@ mod os_rename;
mod os_replace;
mod os_rmdir;
mod os_sep_split;
mod os_stat;
mod os_symlink;
mod os_unlink;
mod path_constructor_current_directory;
Expand Down
166 changes: 166 additions & 0 deletions crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_stat.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
use ruff_diagnostics::{Applicability, Edit, Fix};
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::{ArgOrKeyword, ExprCall, PythonVersion};
use ruff_text_size::Ranged;

use crate::{
FixAvailability, Violation,
checkers::ast::Checker,
importer::ImportRequest,
preview::is_fix_os_stat_enabled,
rules::flake8_use_pathlib::helpers::{
has_unknown_keywords_or_starred_expr, is_keyword_only_argument_non_default,
is_keyword_true_or_default, is_pathlib_path_call,
},
};

/// ## What it does
/// Checks for uses of `os.stat`.
///
/// ## 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.stat()` can improve readability over the `os`
/// module's counterparts (e.g., `os.path.stat()`).
///
/// ## Examples
/// ```python
/// import os
/// from pwd import getpwuid
/// from grp import getgrgid
///
/// stat = os.stat(file_name)
/// owner_name = getpwuid(stat.st_uid).pw_name
/// group_name = getgrgid(stat.st_gid).gr_name
/// ```
///
/// Use instead:
/// ```python
/// from pathlib import Path
///
/// file_path = Path(file_name)
/// stat = file_path.stat()
/// owner_name = file_path.owner()
/// group_name = file_path.group()
/// ```
///
/// ## 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.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
/// - [Python documentation: `Path.group`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.group)
/// - [Python documentation: `Path.owner`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.owner)
/// - [Python documentation: `os.stat`](https://docs.python.org/3/library/os.html#os.stat)
/// - [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#corresponding-tools)
/// - [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)]
#[violation_metadata(stable_since = "v0.0.231")]
pub(crate) struct OsStat;

impl Violation for OsStat {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
"`os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path.group()`"
.to_string()
}

fn fix_title(&self) -> Option<String> {
Some("Replace with `Path(...).stat()`".to_string())
}
}

// PTH116
pub(crate) fn os_stat(checker: &Checker, call: &ExprCall, segment: &[&str]) {
if segment != ["os", "stat"] {
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.stat)
// ```text
// 0 1 2
// os.stat(path, *, dir_fd=None, follow_symlinks=True)
// ```
if is_keyword_only_argument_non_default(&call.arguments, "dir_fd") {
return;
}

if has_unknown_keywords_or_starred_expr(&call.arguments, &["path", "dir_fd", "follow_symlinks"])
{
return;
}

let Some(path_args) = call.arguments.find_argument_value("path", 0) else {
return;
};

let method = if checker.target_version() >= PythonVersion::PY310 {
"stat"
} else {
match is_keyword_true_or_default(&call.arguments, "follow_symlinks") {
Some(true) => "stat",
Some(false) => "lstat",
None => return,
}
};

let range = call.range();
let mut diagnostic = checker.report_diagnostic(OsStat, call.func.range());

if !is_fix_os_stat_enabled(checker.settings()) {
return;
}

diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import("pathlib", "Path"),
call.start(),
checker.semantic(),
)?;

let locator = checker.locator();
let path_code = locator.slice(path_args.range());

let args = |arg: ArgOrKeyword| match arg {
ArgOrKeyword::Arg(expr) if expr.range() != path_args.range() => {
Some(locator.slice(expr.range()))
}
ArgOrKeyword::Keyword(kw)
if matches!(kw.arg.as_deref(), Some("follow_symlinks")) && method == "stat" =>
{
Some(locator.slice(kw.range()))
}
_ => None,
};

let stat_args = itertools::join(call.arguments.iter_source_order().filter_map(args), ", ");

let replacement = if is_pathlib_path_call(checker, path_args) {
format!("{path_code}.{method}({stat_args})")
} else {
format!("{binding}({path_code}).{method}({stat_args})")
};

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,
))
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::rules::flake8_use_pathlib::helpers::{
};
use crate::rules::flake8_use_pathlib::{
rules::Glob,
violations::{Joiner, OsListdir, OsPathJoin, OsPathSplitext, OsStat, PyPath},
violations::{Joiner, OsListdir, OsPathJoin, OsPathSplitext, PyPath},
};

pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
Expand All @@ -17,24 +17,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {

let range = call.func.range();
match qualified_name.segments() {
// PTH116
["os", "stat"] => {
// `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.stat)
// ```text
// 0 1 2
// os.stat(path, *, 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(OsStat, range)
}
// PTH118
["os", "path", "join"] => checker.report_diagnostic_if_enabled(
OsPathJoin {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path
24 | os.path.isabs(p)
25 | os.path.join(p, q)
|
help: Replace with `Path(...).stat()`

PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()`
--> full_name.py:24:1
Expand Down Expand Up @@ -411,6 +412,47 @@ PTH123 `open()` should be replaced by `Path.open()`
|
help: Replace with `Path.open()`

PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path.group()`
--> full_name.py:74:1
|
73 | # https://github.com/astral-sh/ruff/issues/17693
74 | os.stat(1)
| ^^^^^^^
75 | os.stat(x)
|
help: Replace with `Path(...).stat()`

PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path.group()`
--> full_name.py:75:1
|
73 | # https://github.com/astral-sh/ruff/issues/17693
74 | os.stat(1)
75 | os.stat(x)
| ^^^^^^^
|
help: Replace with `Path(...).stat()`

PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path.group()`
--> full_name.py:80:1
|
78 | def func() -> int:
79 | return 2
80 | os.stat(func())
| ^^^^^^^
|
help: Replace with `Path(...).stat()`

PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path.group()`
--> full_name.py:84:5
|
83 | def bar(x: int):
84 | os.stat(x)
| ^^^^^^^
85 |
86 | # https://github.com/astral-sh/ruff/issues/17694
|
help: Replace with `Path(...).stat()`

PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
--> full_name.py:108:1
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path
24 | foo_p.isabs(p)
25 | foo_p.join(p, q)
|
help: Replace with `Path(...).stat()`

PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()`
--> import_as.py:24:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path
26 | isabs(p)
27 | join(p, q)
|
help: Replace with `Path(...).stat()`

PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()`
--> import_from.py:26:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ PTH116 `os.stat()` should be replaced by `Path.stat()`, `Path.owner()`, or `Path
31 | xisabs(p)
32 | xjoin(p, q)
|
help: Replace with `Path(...).stat()`

PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()`
--> import_from_as.py:31:1
Expand Down
Loading