mirror of https://github.com/astral-sh/ruff
[`flake8-use-pathlib`] Add autofix for `PTH109` (#19245)
## Summary Part of #2331 ## Test Plan `cargo nextest run flake8_use_pathlib`
This commit is contained in:
parent
c2a05b4825
commit
5d78b3117a
|
|
@ -104,3 +104,6 @@ os.chmod(x)
|
|||
os.replace("src", "dst", src_dir_fd=1, dst_dir_fd=2)
|
||||
os.replace("src", "dst", src_dir_fd=1)
|
||||
os.replace("src", "dst", dst_dir_fd=2)
|
||||
|
||||
os.getcwd()
|
||||
os.getcwdb()
|
||||
|
|
@ -1044,7 +1044,6 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
|
|||
Rule::OsMakedirs,
|
||||
Rule::OsRename,
|
||||
Rule::OsReplace,
|
||||
Rule::OsGetcwd,
|
||||
Rule::OsStat,
|
||||
Rule::OsPathJoin,
|
||||
Rule::OsPathSamefile,
|
||||
|
|
@ -1110,6 +1109,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
|
|||
if checker.is_rule_enabled(Rule::OsReadlink) {
|
||||
flake8_use_pathlib::rules::os_readlink(checker, call, segments);
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::OsGetcwd) {
|
||||
flake8_use_pathlib::rules::os_getcwd(checker, call, segments);
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::PathConstructorCurrentDirectory) {
|
||||
flake8_use_pathlib::rules::path_constructor_current_directory(
|
||||
checker, call, segments,
|
||||
|
|
|
|||
|
|
@ -928,7 +928,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Flake8UsePathlib, "106") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsRmdir),
|
||||
(Flake8UsePathlib, "107") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsRemove),
|
||||
(Flake8UsePathlib, "108") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsUnlink),
|
||||
(Flake8UsePathlib, "109") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsGetcwd),
|
||||
(Flake8UsePathlib, "109") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsGetcwd),
|
||||
(Flake8UsePathlib, "110") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathExists),
|
||||
(Flake8UsePathlib, "111") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathExpanduser),
|
||||
(Flake8UsePathlib, "112") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathIsdir),
|
||||
|
|
|
|||
|
|
@ -134,6 +134,11 @@ pub(crate) const fn is_fix_os_path_dirname_enabled(settings: &LinterSettings) ->
|
|||
settings.preview.is_enabled()
|
||||
}
|
||||
|
||||
// https://github.com/astral-sh/ruff/pull/19245
|
||||
pub(crate) const fn is_fix_os_getcwd_enabled(settings: &LinterSettings) -> bool {
|
||||
settings.preview.is_enabled()
|
||||
}
|
||||
|
||||
// https://github.com/astral-sh/ruff/pull/11436
|
||||
// https://github.com/astral-sh/ruff/pull/11168
|
||||
pub(crate) const fn is_dunder_init_fix_unused_import_enabled(settings: &LinterSettings) -> bool {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
pub(crate) use glob_rule::*;
|
||||
pub(crate) use invalid_pathlib_with_suffix::*;
|
||||
pub(crate) use os_getcwd::*;
|
||||
pub(crate) use os_path_abspath::*;
|
||||
pub(crate) use os_path_basename::*;
|
||||
pub(crate) use os_path_dirname::*;
|
||||
|
|
@ -23,6 +24,7 @@ pub(crate) use replaceable_by_pathlib::*;
|
|||
|
||||
mod glob_rule;
|
||||
mod invalid_pathlib_with_suffix;
|
||||
mod os_getcwd;
|
||||
mod os_path_abspath;
|
||||
mod os_path_basename;
|
||||
mod os_path_dirname;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,100 @@
|
|||
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;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for uses of `os.getcwd` and `os.getcwdb`.
|
||||
///
|
||||
/// ## 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.cwd()` can improve readability over the `os`
|
||||
/// module's counterparts (e.g., `os.getcwd()`).
|
||||
///
|
||||
/// ## Examples
|
||||
/// ```python
|
||||
/// import os
|
||||
///
|
||||
/// cwd = os.getcwd()
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// from pathlib import Path
|
||||
///
|
||||
/// cwd = Path.cwd()
|
||||
/// ```
|
||||
///
|
||||
/// ## 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.cwd`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd)
|
||||
/// - [Python documentation: `os.getcwd`](https://docs.python.org/3/library/os.html#os.getcwd)
|
||||
/// - [Python documentation: `os.getcwdb`](https://docs.python.org/3/library/os.html#os.getcwdb)
|
||||
/// - [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#correspondence-to-tools-in-the-os-module)
|
||||
/// - [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)]
|
||||
pub(crate) struct OsGetcwd;
|
||||
|
||||
impl Violation for OsGetcwd {
|
||||
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
"`os.getcwd()` should be replaced by `Path.cwd()`".to_string()
|
||||
}
|
||||
|
||||
fn fix_title(&self) -> Option<String> {
|
||||
Some("Replace with `Path.cwd()`".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
/// PTH109
|
||||
pub(crate) fn os_getcwd(checker: &Checker, call: &ExprCall, segments: &[&str]) {
|
||||
if !matches!(segments, ["os", "getcwd" | "getcwdb"]) {
|
||||
return;
|
||||
}
|
||||
|
||||
let range = call.range();
|
||||
let mut diagnostic = checker.report_diagnostic(OsGetcwd, call.func.range());
|
||||
|
||||
if !call.arguments.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
if is_fix_os_getcwd_enabled(checker.settings()) {
|
||||
diagnostic.try_set_fix(|| {
|
||||
let (import_edit, binding) = checker.importer().get_or_import_symbol(
|
||||
&ImportRequest::import("pathlib", "Path"),
|
||||
call.start(),
|
||||
checker.semantic(),
|
||||
)?;
|
||||
|
||||
let applicability = if checker.comment_ranges().intersects(range) {
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
Applicability::Safe
|
||||
};
|
||||
|
||||
let replacement = format!("{binding}.cwd()");
|
||||
|
||||
Ok(Fix::applicable_edits(
|
||||
Edit::range_replacement(replacement, range),
|
||||
[import_edit],
|
||||
applicability,
|
||||
))
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -7,8 +7,8 @@ use crate::checkers::ast::Checker;
|
|||
use crate::rules::flake8_use_pathlib::helpers::is_keyword_only_argument_non_default;
|
||||
use crate::rules::flake8_use_pathlib::rules::Glob;
|
||||
use crate::rules::flake8_use_pathlib::violations::{
|
||||
BuiltinOpen, Joiner, OsChmod, OsGetcwd, OsListdir, OsMakedirs, OsMkdir, OsPathJoin,
|
||||
OsPathSamefile, OsPathSplitext, OsRename, OsReplace, OsStat, OsSymlink, PyPath,
|
||||
BuiltinOpen, Joiner, OsChmod, OsListdir, OsMakedirs, OsMkdir, OsPathJoin, OsPathSamefile,
|
||||
OsPathSplitext, OsRename, OsReplace, OsStat, OsSymlink, PyPath,
|
||||
};
|
||||
|
||||
pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
|
||||
|
|
@ -83,10 +83,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
|
|||
}
|
||||
checker.report_diagnostic_if_enabled(OsReplace, range)
|
||||
}
|
||||
// PTH109
|
||||
["os", "getcwd"] => checker.report_diagnostic_if_enabled(OsGetcwd, range),
|
||||
["os", "getcwdb"] => checker.report_diagnostic_if_enabled(OsGetcwd, range),
|
||||
|
||||
// PTH116
|
||||
["os", "stat"] => {
|
||||
// `dir_fd` is not supported by pathlib, so check if it's set to non-default values.
|
||||
|
|
|
|||
|
|
@ -103,6 +103,7 @@ full_name.py:16:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
17 | b = os.path.exists(p)
|
||||
18 | bb = os.path.expanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
full_name.py:17:5: PTH110 `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
@ -292,6 +293,7 @@ full_name.py:35:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
36 | os.path.join(p, *q)
|
||||
37 | os.sep.join(p, *q)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
full_name.py:36:1: PTH118 `os.path.join()` should be replaced by `Path.joinpath()`
|
||||
|
|
||||
|
|
@ -360,3 +362,21 @@ full_name.py:71:1: PTH123 `open()` should be replaced by `Path.open()`
|
|||
72 |
|
||||
73 | # https://github.com/astral-sh/ruff/issues/17693
|
||||
|
|
||||
|
||||
full_name.py:108:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
|
|
||||
106 | os.replace("src", "dst", dst_dir_fd=2)
|
||||
107 |
|
||||
108 | os.getcwd()
|
||||
| ^^^^^^^^^ PTH109
|
||||
109 | os.getcwdb()
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
full_name.py:109:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
|
|
||||
108 | os.getcwd()
|
||||
109 | os.getcwdb()
|
||||
| ^^^^^^^^^^ PTH109
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
|
|
|||
|
|
@ -103,6 +103,7 @@ import_as.py:16:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
17 | b = foo_p.exists(p)
|
||||
18 | bb = foo_p.expanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
import_as.py:17:5: PTH110 `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
|
|||
|
|
@ -103,6 +103,7 @@ import_from.py:18:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
19 | b = exists(p)
|
||||
20 | bb = expanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
import_from.py:19:5: PTH110 `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
|
|||
|
|
@ -103,6 +103,7 @@ import_from_as.py:23:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
24 | b = xexists(p)
|
||||
25 | bb = xexpanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
import_from_as.py:24:5: PTH110 `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
|
|||
|
|
@ -168,6 +168,7 @@ full_name.py:16:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
17 | b = os.path.exists(p)
|
||||
18 | bb = os.path.expanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
full_name.py:17:5: PTH110 [*] `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
@ -510,6 +511,7 @@ full_name.py:35:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
36 | os.path.join(p, *q)
|
||||
37 | os.sep.join(p, *q)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
full_name.py:36:1: PTH118 `os.path.join()` should be replaced by `Path.joinpath()`
|
||||
|
|
||||
|
|
@ -578,3 +580,50 @@ full_name.py:71:1: PTH123 `open()` should be replaced by `Path.open()`
|
|||
72 |
|
||||
73 | # https://github.com/astral-sh/ruff/issues/17693
|
||||
|
|
||||
|
||||
full_name.py:108:1: PTH109 [*] `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
|
|
||||
106 | os.replace("src", "dst", dst_dir_fd=2)
|
||||
107 |
|
||||
108 | os.getcwd()
|
||||
| ^^^^^^^^^ PTH109
|
||||
109 | os.getcwdb()
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
ℹ Safe fix
|
||||
1 1 | import os
|
||||
2 2 | import os.path
|
||||
3 |+import pathlib
|
||||
3 4 |
|
||||
4 5 | p = "/foo"
|
||||
5 6 | q = "bar"
|
||||
--------------------------------------------------------------------------------
|
||||
105 106 | os.replace("src", "dst", src_dir_fd=1)
|
||||
106 107 | os.replace("src", "dst", dst_dir_fd=2)
|
||||
107 108 |
|
||||
108 |-os.getcwd()
|
||||
109 |+pathlib.Path.cwd()
|
||||
109 110 | os.getcwdb()
|
||||
|
||||
full_name.py:109:1: PTH109 [*] `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
|
|
||||
108 | os.getcwd()
|
||||
109 | os.getcwdb()
|
||||
| ^^^^^^^^^^ PTH109
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
ℹ Safe fix
|
||||
1 1 | import os
|
||||
2 2 | import os.path
|
||||
3 |+import pathlib
|
||||
3 4 |
|
||||
4 5 | p = "/foo"
|
||||
5 6 | q = "bar"
|
||||
--------------------------------------------------------------------------------
|
||||
106 107 | os.replace("src", "dst", dst_dir_fd=2)
|
||||
107 108 |
|
||||
108 109 | os.getcwd()
|
||||
109 |-os.getcwdb()
|
||||
110 |+pathlib.Path.cwd()
|
||||
|
|
|
|||
|
|
@ -168,6 +168,7 @@ import_as.py:16:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
17 | b = foo_p.exists(p)
|
||||
18 | bb = foo_p.expanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
import_as.py:17:5: PTH110 [*] `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
|
|||
|
|
@ -172,6 +172,7 @@ import_from.py:18:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
19 | b = exists(p)
|
||||
20 | bb = expanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
import_from.py:19:5: PTH110 [*] `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
|
|||
|
|
@ -172,6 +172,7 @@ import_from_as.py:23:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
|||
24 | b = xexists(p)
|
||||
25 | bb = xexpanduser(p)
|
||||
|
|
||||
= help: Replace with `Path.cwd()`
|
||||
|
||||
import_from_as.py:24:5: PTH110 [*] `os.path.exists()` should be replaced by `Path.exists()`
|
||||
|
|
||||
|
|
|
|||
|
|
@ -230,52 +230,6 @@ impl Violation for OsReplace {
|
|||
}
|
||||
}
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for uses of `os.getcwd` and `os.getcwdb`.
|
||||
///
|
||||
/// ## 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.cwd()` can improve readability over the `os`
|
||||
/// module's counterparts (e.g., `os.getcwd()`).
|
||||
///
|
||||
/// ## Examples
|
||||
/// ```python
|
||||
/// import os
|
||||
///
|
||||
/// cwd = os.getcwd()
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// from pathlib import Path
|
||||
///
|
||||
/// cwd = Path.cwd()
|
||||
/// ```
|
||||
///
|
||||
/// ## 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.
|
||||
///
|
||||
/// ## References
|
||||
/// - [Python documentation: `Path.cwd`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd)
|
||||
/// - [Python documentation: `os.getcwd`](https://docs.python.org/3/library/os.html#os.getcwd)
|
||||
/// - [Python documentation: `os.getcwdb`](https://docs.python.org/3/library/os.html#os.getcwdb)
|
||||
/// - [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#correspondence-to-tools-in-the-os-module)
|
||||
/// - [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)]
|
||||
pub(crate) struct OsGetcwd;
|
||||
|
||||
impl Violation for OsGetcwd {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
"`os.getcwd()` should be replaced by `Path.cwd()`".to_string()
|
||||
}
|
||||
}
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for uses of `os.stat`.
|
||||
///
|
||||
|
|
|
|||
Loading…
Reference in New Issue