Skip to content

Commit 5d78b31

Browse files
authored
[flake8-use-pathlib] Add autofix for PTH109 (#19245)
## Summary Part of #2331 ## Test Plan `cargo nextest run flake8_use_pathlib`
1 parent c2a05b4 commit 5d78b31

16 files changed

+191
-54
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,6 @@ def bar(x: int):
104104
os.replace("src", "dst", src_dir_fd=1, dst_dir_fd=2)
105105
os.replace("src", "dst", src_dir_fd=1)
106106
os.replace("src", "dst", dst_dir_fd=2)
107+
108+
os.getcwd()
109+
os.getcwdb()

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,6 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
10441044
Rule::OsMakedirs,
10451045
Rule::OsRename,
10461046
Rule::OsReplace,
1047-
Rule::OsGetcwd,
10481047
Rule::OsStat,
10491048
Rule::OsPathJoin,
10501049
Rule::OsPathSamefile,
@@ -1110,6 +1109,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
11101109
if checker.is_rule_enabled(Rule::OsReadlink) {
11111110
flake8_use_pathlib::rules::os_readlink(checker, call, segments);
11121111
}
1112+
if checker.is_rule_enabled(Rule::OsGetcwd) {
1113+
flake8_use_pathlib::rules::os_getcwd(checker, call, segments);
1114+
}
11131115
if checker.is_rule_enabled(Rule::PathConstructorCurrentDirectory) {
11141116
flake8_use_pathlib::rules::path_constructor_current_directory(
11151117
checker, call, segments,

crates/ruff_linter/src/codes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
928928
(Flake8UsePathlib, "106") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsRmdir),
929929
(Flake8UsePathlib, "107") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsRemove),
930930
(Flake8UsePathlib, "108") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsUnlink),
931-
(Flake8UsePathlib, "109") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsGetcwd),
931+
(Flake8UsePathlib, "109") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsGetcwd),
932932
(Flake8UsePathlib, "110") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathExists),
933933
(Flake8UsePathlib, "111") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathExpanduser),
934934
(Flake8UsePathlib, "112") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathIsdir),

crates/ruff_linter/src/preview.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ pub(crate) const fn is_fix_os_path_dirname_enabled(settings: &LinterSettings) ->
134134
settings.preview.is_enabled()
135135
}
136136

137+
// https://github.com/astral-sh/ruff/pull/19245
138+
pub(crate) const fn is_fix_os_getcwd_enabled(settings: &LinterSettings) -> bool {
139+
settings.preview.is_enabled()
140+
}
141+
137142
// https://github.com/astral-sh/ruff/pull/11436
138143
// https://github.com/astral-sh/ruff/pull/11168
139144
pub(crate) const fn is_dunder_init_fix_unused_import_enabled(settings: &LinterSettings) -> bool {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
pub(crate) use glob_rule::*;
22
pub(crate) use invalid_pathlib_with_suffix::*;
3+
pub(crate) use os_getcwd::*;
34
pub(crate) use os_path_abspath::*;
45
pub(crate) use os_path_basename::*;
56
pub(crate) use os_path_dirname::*;
@@ -23,6 +24,7 @@ pub(crate) use replaceable_by_pathlib::*;
2324

2425
mod glob_rule;
2526
mod invalid_pathlib_with_suffix;
27+
mod os_getcwd;
2628
mod os_path_abspath;
2729
mod os_path_basename;
2830
mod os_path_dirname;
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
use crate::checkers::ast::Checker;
2+
use crate::importer::ImportRequest;
3+
use crate::preview::is_fix_os_getcwd_enabled;
4+
use crate::{FixAvailability, Violation};
5+
use ruff_diagnostics::{Applicability, Edit, Fix};
6+
use ruff_macros::{ViolationMetadata, derive_message_formats};
7+
use ruff_python_ast::ExprCall;
8+
use ruff_text_size::Ranged;
9+
10+
/// ## What it does
11+
/// Checks for uses of `os.getcwd` and `os.getcwdb`.
12+
///
13+
/// ## Why is this bad?
14+
/// `pathlib` offers a high-level API for path manipulation, as compared to
15+
/// the lower-level API offered by `os`. When possible, using `Path` object
16+
/// methods such as `Path.cwd()` can improve readability over the `os`
17+
/// module's counterparts (e.g., `os.getcwd()`).
18+
///
19+
/// ## Examples
20+
/// ```python
21+
/// import os
22+
///
23+
/// cwd = os.getcwd()
24+
/// ```
25+
///
26+
/// Use instead:
27+
/// ```python
28+
/// from pathlib import Path
29+
///
30+
/// cwd = Path.cwd()
31+
/// ```
32+
///
33+
/// ## Known issues
34+
/// While using `pathlib` can improve the readability and type safety of your code,
35+
/// it can be less performant than the lower-level alternatives that work directly with strings,
36+
/// especially on older versions of Python.
37+
///
38+
/// ## Fix Safety
39+
/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression.
40+
///
41+
/// ## References
42+
/// - [Python documentation: `Path.cwd`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd)
43+
/// - [Python documentation: `os.getcwd`](https://docs.python.org/3/library/os.html#os.getcwd)
44+
/// - [Python documentation: `os.getcwdb`](https://docs.python.org/3/library/os.html#os.getcwdb)
45+
/// - [PEP 428 – The pathlib module – object-oriented filesystem paths](https://peps.python.org/pep-0428/)
46+
/// - [Correspondence between `os` and `pathlib`](https://docs.python.org/3/library/pathlib.html#correspondence-to-tools-in-the-os-module)
47+
/// - [Why you should be using pathlib](https://treyhunner.com/2018/12/why-you-should-be-using-pathlib/)
48+
/// - [No really, pathlib is great](https://treyhunner.com/2019/01/no-really-pathlib-is-great/)
49+
#[derive(ViolationMetadata)]
50+
pub(crate) struct OsGetcwd;
51+
52+
impl Violation for OsGetcwd {
53+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
54+
#[derive_message_formats]
55+
fn message(&self) -> String {
56+
"`os.getcwd()` should be replaced by `Path.cwd()`".to_string()
57+
}
58+
59+
fn fix_title(&self) -> Option<String> {
60+
Some("Replace with `Path.cwd()`".to_string())
61+
}
62+
}
63+
64+
/// PTH109
65+
pub(crate) fn os_getcwd(checker: &Checker, call: &ExprCall, segments: &[&str]) {
66+
if !matches!(segments, ["os", "getcwd" | "getcwdb"]) {
67+
return;
68+
}
69+
70+
let range = call.range();
71+
let mut diagnostic = checker.report_diagnostic(OsGetcwd, call.func.range());
72+
73+
if !call.arguments.is_empty() {
74+
return;
75+
}
76+
77+
if is_fix_os_getcwd_enabled(checker.settings()) {
78+
diagnostic.try_set_fix(|| {
79+
let (import_edit, binding) = checker.importer().get_or_import_symbol(
80+
&ImportRequest::import("pathlib", "Path"),
81+
call.start(),
82+
checker.semantic(),
83+
)?;
84+
85+
let applicability = if checker.comment_ranges().intersects(range) {
86+
Applicability::Unsafe
87+
} else {
88+
Applicability::Safe
89+
};
90+
91+
let replacement = format!("{binding}.cwd()");
92+
93+
Ok(Fix::applicable_edits(
94+
Edit::range_replacement(replacement, range),
95+
[import_edit],
96+
applicability,
97+
))
98+
});
99+
}
100+
}

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ use crate::checkers::ast::Checker;
77
use crate::rules::flake8_use_pathlib::helpers::is_keyword_only_argument_non_default;
88
use crate::rules::flake8_use_pathlib::rules::Glob;
99
use crate::rules::flake8_use_pathlib::violations::{
10-
BuiltinOpen, Joiner, OsChmod, OsGetcwd, OsListdir, OsMakedirs, OsMkdir, OsPathJoin,
11-
OsPathSamefile, OsPathSplitext, OsRename, OsReplace, OsStat, OsSymlink, PyPath,
10+
BuiltinOpen, Joiner, OsChmod, OsListdir, OsMakedirs, OsMkdir, OsPathJoin, OsPathSamefile,
11+
OsPathSplitext, OsRename, OsReplace, OsStat, OsSymlink, PyPath,
1212
};
1313

1414
pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
@@ -83,10 +83,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
8383
}
8484
checker.report_diagnostic_if_enabled(OsReplace, range)
8585
}
86-
// PTH109
87-
["os", "getcwd"] => checker.report_diagnostic_if_enabled(OsGetcwd, range),
88-
["os", "getcwdb"] => checker.report_diagnostic_if_enabled(OsGetcwd, range),
89-
9086
// PTH116
9187
["os", "stat"] => {
9288
// `dir_fd` is not supported by pathlib, so check if it's set to non-default values.

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ full_name.py:16:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
103103
17 | b = os.path.exists(p)
104104
18 | bb = os.path.expanduser(p)
105105
|
106+
= help: Replace with `Path.cwd()`
106107

107108
full_name.py:17:5: PTH110 `os.path.exists()` should be replaced by `Path.exists()`
108109
|
@@ -292,6 +293,7 @@ full_name.py:35:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
292293
36 | os.path.join(p, *q)
293294
37 | os.sep.join(p, *q)
294295
|
296+
= help: Replace with `Path.cwd()`
295297

296298
full_name.py:36:1: PTH118 `os.path.join()` should be replaced by `Path.joinpath()`
297299
|
@@ -360,3 +362,21 @@ full_name.py:71:1: PTH123 `open()` should be replaced by `Path.open()`
360362
72 |
361363
73 | # https://github.com/astral-sh/ruff/issues/17693
362364
|
365+
366+
full_name.py:108:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
367+
|
368+
106 | os.replace("src", "dst", dst_dir_fd=2)
369+
107 |
370+
108 | os.getcwd()
371+
| ^^^^^^^^^ PTH109
372+
109 | os.getcwdb()
373+
|
374+
= help: Replace with `Path.cwd()`
375+
376+
full_name.py:109:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
377+
|
378+
108 | os.getcwd()
379+
109 | os.getcwdb()
380+
| ^^^^^^^^^^ PTH109
381+
|
382+
= help: Replace with `Path.cwd()`

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_as.py.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ import_as.py:16:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
103103
17 | b = foo_p.exists(p)
104104
18 | bb = foo_p.expanduser(p)
105105
|
106+
= help: Replace with `Path.cwd()`
106107

107108
import_as.py:17:5: PTH110 `os.path.exists()` should be replaced by `Path.exists()`
108109
|

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__import_from.py.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ import_from.py:18:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
103103
19 | b = exists(p)
104104
20 | bb = expanduser(p)
105105
|
106+
= help: Replace with `Path.cwd()`
106107

107108
import_from.py:19:5: PTH110 `os.path.exists()` should be replaced by `Path.exists()`
108109
|

0 commit comments

Comments
 (0)