Skip to content

Commit e23780c

Browse files
authored
[flake8-use-pathlib] Add autofixes for PTH203, PTH204, PTH205 (#18922)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary Part of #2331 | [#18763](#18763 (comment)) <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan update snapshots <!-- How was it tested? -->
1 parent 47f88b3 commit e23780c

20 files changed

Lines changed: 944 additions & 254 deletions

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import os.path
1+
import os.path, pathlib
22
from pathlib import Path
33
from os.path import getatime
44

@@ -10,3 +10,26 @@
1010
getatime("filename")
1111
getatime(b"filename")
1212
getatime(Path("filename"))
13+
14+
15+
file = __file__
16+
17+
os.path.getatime(file)
18+
os.path.getatime(filename="filename")
19+
os.path.getatime(filename=Path("filename"))
20+
21+
os.path.getatime( # comment 1
22+
# comment 2
23+
"filename" # comment 3
24+
# comment 4
25+
, # comment 5
26+
# comment 6
27+
) # comment 7
28+
29+
os.path.getatime("file" + "name")
30+
31+
getatime(Path("filename").resolve())
32+
33+
os.path.getatime(pathlib.Path("filename"))
34+
35+
getatime(Path("dir") / "file.txt")

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,9 +1062,6 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
10621062
Rule::OsPathSplitext,
10631063
Rule::BuiltinOpen,
10641064
Rule::PyPath,
1065-
Rule::OsPathGetatime,
1066-
Rule::OsPathGetmtime,
1067-
Rule::OsPathGetctime,
10681065
Rule::Glob,
10691066
Rule::OsListdir,
10701067
Rule::OsSymlink,
@@ -1074,6 +1071,15 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
10741071
if checker.is_rule_enabled(Rule::OsPathGetsize) {
10751072
flake8_use_pathlib::rules::os_path_getsize(checker, call);
10761073
}
1074+
if checker.is_rule_enabled(Rule::OsPathGetatime) {
1075+
flake8_use_pathlib::rules::os_path_getatime(checker, call);
1076+
}
1077+
if checker.is_rule_enabled(Rule::OsPathGetctime) {
1078+
flake8_use_pathlib::rules::os_path_getctime(checker, call);
1079+
}
1080+
if checker.is_rule_enabled(Rule::OsPathGetmtime) {
1081+
flake8_use_pathlib::rules::os_path_getmtime(checker, call);
1082+
}
10771083
if checker.is_rule_enabled(Rule::PathConstructorCurrentDirectory) {
10781084
flake8_use_pathlib::rules::path_constructor_current_directory(checker, call);
10791085
}

crates/ruff_linter/src/preview.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,20 @@ pub(crate) const fn is_fix_manual_list_comprehension_enabled(settings: &LinterSe
5454
pub(crate) const fn is_fix_os_path_getsize_enabled(settings: &LinterSettings) -> bool {
5555
settings.preview.is_enabled()
5656
}
57+
// https://github.com/astral-sh/ruff/pull/18922
58+
pub(crate) const fn is_fix_os_path_getmtime_enabled(settings: &LinterSettings) -> bool {
59+
settings.preview.is_enabled()
60+
}
61+
62+
// https://github.com/astral-sh/ruff/pull/18922
63+
pub(crate) const fn is_fix_os_path_getatime_enabled(settings: &LinterSettings) -> bool {
64+
settings.preview.is_enabled()
65+
}
66+
67+
// https://github.com/astral-sh/ruff/pull/18922
68+
pub(crate) const fn is_fix_os_path_getctime_enabled(settings: &LinterSettings) -> bool {
69+
settings.preview.is_enabled()
70+
}
5771

5872
// https://github.com/astral-sh/ruff/pull/11436
5973
// https://github.com/astral-sh/ruff/pull/11168
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
use crate::checkers::ast::Checker;
2+
use crate::importer::ImportRequest;
3+
use crate::{Applicability, Edit, Fix, Violation};
4+
use ruff_python_ast::{Expr, ExprCall};
5+
use ruff_text_size::Ranged;
6+
7+
pub(crate) fn is_path_call(checker: &Checker, expr: &Expr) -> bool {
8+
expr.as_call_expr().is_some_and(|expr_call| {
9+
checker
10+
.semantic()
11+
.resolve_qualified_name(&expr_call.func)
12+
.is_some_and(|name| matches!(name.segments(), ["pathlib", "Path"]))
13+
})
14+
}
15+
16+
pub(crate) fn check_os_path_get_calls(
17+
checker: &Checker,
18+
call: &ExprCall,
19+
fn_name: &str,
20+
attr: &str,
21+
fix_enabled: bool,
22+
violation: impl Violation,
23+
) {
24+
if checker
25+
.semantic()
26+
.resolve_qualified_name(&call.func)
27+
.is_none_or(|qualified_name| qualified_name.segments() != ["os", "path", fn_name])
28+
{
29+
return;
30+
}
31+
32+
if call.arguments.len() != 1 {
33+
return;
34+
}
35+
36+
let Some(arg) = call.arguments.find_argument_value("filename", 0) else {
37+
return;
38+
};
39+
40+
let arg_code = checker.locator().slice(arg.range());
41+
let range = call.range();
42+
43+
let mut diagnostic = checker.report_diagnostic(violation, call.func.range());
44+
45+
if fix_enabled {
46+
diagnostic.try_set_fix(|| {
47+
let (import_edit, binding) = checker.importer().get_or_import_symbol(
48+
&ImportRequest::import("pathlib", "Path"),
49+
call.start(),
50+
checker.semantic(),
51+
)?;
52+
53+
let applicability = if checker.comment_ranges().intersects(range) {
54+
Applicability::Unsafe
55+
} else {
56+
Applicability::Safe
57+
};
58+
59+
let replacement = if is_path_call(checker, arg) {
60+
format!("{arg_code}.stat().{attr}")
61+
} else {
62+
format!("{binding}({arg_code}).stat().{attr}")
63+
};
64+
65+
Ok(Fix::applicable_edits(
66+
Edit::range_replacement(replacement, range),
67+
[import_edit],
68+
applicability,
69+
))
70+
});
71+
}
72+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//! Rules from [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/).
2+
mod helpers;
23
pub(crate) mod rules;
34
pub(crate) mod violations;
45

@@ -81,6 +82,9 @@ mod tests {
8182

8283
#[test_case(Rule::OsPathGetsize, Path::new("PTH202.py"))]
8384
#[test_case(Rule::OsPathGetsize, Path::new("PTH202_2.py"))]
85+
#[test_case(Rule::OsPathGetatime, Path::new("PTH203.py"))]
86+
#[test_case(Rule::OsPathGetmtime, Path::new("PTH204.py"))]
87+
#[test_case(Rule::OsPathGetctime, Path::new("PTH205.py"))]
8488
fn preview_flake8_use_pathlib(rule_code: Rule, path: &Path) -> Result<()> {
8589
let snapshot = format!(
8690
"preview__{}_{}",

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
use crate::checkers::ast::Checker;
2+
use crate::preview::is_fix_os_path_getatime_enabled;
3+
use crate::rules::flake8_use_pathlib::helpers::check_os_path_get_calls;
4+
use crate::{FixAvailability, Violation};
15
use ruff_macros::{ViolationMetadata, derive_message_formats};
2-
3-
use crate::Violation;
6+
use ruff_python_ast::ExprCall;
47

58
/// ## What it does
69
/// Checks for uses of `os.path.getatime`.
@@ -32,6 +35,9 @@ use crate::Violation;
3235
/// it can be less performant than the lower-level alternatives that work directly with strings,
3336
/// especially on older versions of Python.
3437
///
38+
/// ## Fix Safety
39+
/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression.
40+
///
3541
/// ## References
3642
/// - [Python documentation: `Path.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
3743
/// - [Python documentation: `os.path.getatime`](https://docs.python.org/3/library/os.path.html#os.path.getatime)
@@ -43,8 +49,25 @@ use crate::Violation;
4349
pub(crate) struct OsPathGetatime;
4450

4551
impl Violation for OsPathGetatime {
52+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
4653
#[derive_message_formats]
4754
fn message(&self) -> String {
4855
"`os.path.getatime` should be replaced by `Path.stat().st_atime`".to_string()
4956
}
57+
58+
fn fix_title(&self) -> Option<String> {
59+
Some("Replace with `Path.stat(...).st_atime`".to_string())
60+
}
61+
}
62+
63+
/// PTH203
64+
pub(crate) fn os_path_getatime(checker: &Checker, call: &ExprCall) {
65+
check_os_path_get_calls(
66+
checker,
67+
call,
68+
"getatime",
69+
"st_atime",
70+
is_fix_os_path_getatime_enabled(checker.settings()),
71+
OsPathGetatime,
72+
);
5073
}

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
use crate::checkers::ast::Checker;
2+
use crate::preview::is_fix_os_path_getctime_enabled;
3+
use crate::rules::flake8_use_pathlib::helpers::check_os_path_get_calls;
4+
use crate::{FixAvailability, Violation};
15
use ruff_macros::{ViolationMetadata, derive_message_formats};
2-
3-
use crate::Violation;
6+
use ruff_python_ast::ExprCall;
47

58
/// ## What it does
69
/// Checks for uses of `os.path.getctime`.
@@ -32,6 +35,9 @@ use crate::Violation;
3235
/// it can be less performant than the lower-level alternatives that work directly with strings,
3336
/// especially on older versions of Python.
3437
///
38+
/// ## Fix Safety
39+
/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression.
40+
///
3541
/// ## References
3642
/// - [Python documentation: `Path.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
3743
/// - [Python documentation: `os.path.getctime`](https://docs.python.org/3/library/os.path.html#os.path.getctime)
@@ -43,8 +49,26 @@ use crate::Violation;
4349
pub(crate) struct OsPathGetctime;
4450

4551
impl Violation for OsPathGetctime {
52+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
53+
4654
#[derive_message_formats]
4755
fn message(&self) -> String {
4856
"`os.path.getctime` should be replaced by `Path.stat().st_ctime`".to_string()
4957
}
58+
59+
fn fix_title(&self) -> Option<String> {
60+
Some("Replace with `Path.stat(...).st_ctime`".to_string())
61+
}
62+
}
63+
64+
/// PTH205
65+
pub(crate) fn os_path_getctime(checker: &Checker, call: &ExprCall) {
66+
check_os_path_get_calls(
67+
checker,
68+
call,
69+
"getctime",
70+
"st_ctime",
71+
is_fix_os_path_getctime_enabled(checker.settings()),
72+
OsPathGetctime,
73+
);
5074
}

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
use crate::checkers::ast::Checker;
2+
use crate::preview::is_fix_os_path_getmtime_enabled;
3+
use crate::rules::flake8_use_pathlib::helpers::check_os_path_get_calls;
4+
use crate::{FixAvailability, Violation};
15
use ruff_macros::{ViolationMetadata, derive_message_formats};
2-
3-
use crate::Violation;
6+
use ruff_python_ast::ExprCall;
47

58
/// ## What it does
69
/// Checks for uses of `os.path.getmtime`.
@@ -32,6 +35,9 @@ use crate::Violation;
3235
/// it can be less performant than the lower-level alternatives that work directly with strings,
3336
/// especially on older versions of Python.
3437
///
38+
/// ## Fix Safety
39+
/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression.
40+
///
3541
/// ## References
3642
/// - [Python documentation: `Path.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
3743
/// - [Python documentation: `os.path.getmtime`](https://docs.python.org/3/library/os.path.html#os.path.getmtime)
@@ -43,8 +49,26 @@ use crate::Violation;
4349
pub(crate) struct OsPathGetmtime;
4450

4551
impl Violation for OsPathGetmtime {
52+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
53+
4654
#[derive_message_formats]
4755
fn message(&self) -> String {
4856
"`os.path.getmtime` should be replaced by `Path.stat().st_mtime`".to_string()
4957
}
58+
59+
fn fix_title(&self) -> Option<String> {
60+
Some("Replace with `Path.stat(...).st_mtime`".to_string())
61+
}
62+
}
63+
64+
/// PTH204
65+
pub(crate) fn os_path_getmtime(checker: &Checker, call: &ExprCall) {
66+
check_os_path_get_calls(
67+
checker,
68+
call,
69+
"getmtime",
70+
"st_mtime",
71+
is_fix_os_path_getmtime_enabled(checker.settings()),
72+
OsPathGetmtime,
73+
);
5074
}

0 commit comments

Comments
 (0)