Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-use-pathlib] autofix and new rules #2348

Closed
wants to merge 2 commits into from
Closed
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
41 changes: 23 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1281,31 +1281,36 @@ For more, see [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/)

| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| PTH100 | pathlib-abspath | `os.path.abspath` should be replaced by `.resolve()` | |
| PTH101 | pathlib-chmod | `os.chmod` should be replaced by `.chmod()` | |
| PTH102 | pathlib-mkdir | `os.mkdir` should be replaced by `.mkdir()` | |
| PTH100 | pathlib-abspath | `os.path.abspath` should be replaced by `.resolve()` | 🛠 |
| PTH101 | pathlib-chmod | `os.chmod` should be replaced by `.chmod()` | 🛠 |
| PTH102 | pathlib-mkdir | `os.mkdir` should be replaced by `.mkdir()` | 🛠 |
| PTH103 | pathlib-makedirs | `os.makedirs` should be replaced by `.mkdir(parents=True)` | |
| PTH104 | pathlib-rename | `os.rename` should be replaced by `.rename()` | |
| PTH105 | pathlib-replace | `os.replace`should be replaced by `.replace()` | |
| PTH106 | pathlib-rmdir | `os.rmdir` should be replaced by `.rmdir()` | |
| PTH107 | pathlib-remove | `os.remove` should be replaced by `.unlink()` | |
| PTH108 | pathlib-unlink | `os.unlink` should be replaced by `.unlink()` | |
| PTH109 | pathlib-getcwd | `os.getcwd` should be replaced by `Path.cwd()` | |
| PTH110 | pathlib-exists | `os.path.exists` should be replaced by `.exists()` | |
| PTH111 | pathlib-expanduser | `os.path.expanduser` should be replaced by `.expanduser()` | |
| PTH112 | pathlib-is-dir | `os.path.isdir` should be replaced by `.is_dir()` | |
| PTH113 | pathlib-is-file | `os.path.isfile` should be replaced by `.is_file()` | |
| PTH114 | pathlib-is-link | `os.path.islink` should be replaced by `.is_symlink()` | |
| PTH115 | pathlib-readlink | `os.readlink` should be replaced by `.readlink()` | |
| PTH104 | pathlib-rename | `os.rename` should be replaced by `.rename()` | 🛠 |
| PTH105 | pathlib-replace | `os.replace`should be replaced by `.replace()` | 🛠 |
| PTH106 | pathlib-rmdir | `os.rmdir` should be replaced by `.rmdir()` | 🛠 |
| PTH107 | pathlib-remove | `os.remove` should be replaced by `.unlink()` | 🛠 |
| PTH108 | pathlib-unlink | `os.unlink` should be replaced by `.unlink()` | 🛠 |
| PTH109 | [pathlib-getcwd](https://github.com/charliermarsh/ruff/blob/main/docs/rules/pathlib-getcwd.md) | `os.getcwd` should be replaced by `Path.cwd()` | 🛠 |
| PTH110 | pathlib-exists | `os.path.exists` should be replaced by `.exists()` | 🛠 |
| PTH111 | pathlib-expanduser | `os.path.expanduser` should be replaced by `.expanduser()` | 🛠 |
| PTH112 | pathlib-is-dir | `os.path.isdir` should be replaced by `.is_dir()` | 🛠 |
| PTH113 | pathlib-is-file | `os.path.isfile` should be replaced by `.is_file()` | 🛠 |
| PTH114 | pathlib-is-link | `os.path.islink` should be replaced by `.is_symlink()` | 🛠 |
| PTH115 | [pathlib-readlink](https://github.com/charliermarsh/ruff/blob/main/docs/rules/pathlib-readlink.md) | `os.readlink` should be replaced by `.readlink()` | 🛠 |
| PTH116 | pathlib-stat | `os.stat` should be replaced by `.stat()` or `.owner()` or `.group()` | |
| PTH117 | pathlib-is-abs | `os.path.isabs` should be replaced by `.is_absolute()` | |
| PTH118 | pathlib-join | `os.path.join` should be replaced by foo_path / "bar" | |
| PTH117 | pathlib-is-abs | `os.path.isabs` should be replaced by `.is_absolute()` | 🛠 |
| PTH118 | pathlib-join | `os.path.join` should be replaced by `foo_path / "bar"` | |
| PTH119 | pathlib-basename | `os.path.basename` should be replaced by `.name` | |
| PTH120 | pathlib-dirname | `os.path.dirname` should be replaced by `.parent` | |
| PTH121 | pathlib-samefile | `os.path.samefile` should be replaced by `.samefile()` | |
| PTH121 | pathlib-samefile | `os.path.samefile` should be replaced by `.samefile()` | 🛠 |
| PTH122 | pathlib-splitext | `os.path.splitext` should be replaced by `.suffix` | |
| PTH123 | pathlib-open | `open("foo")` should be replaced by `Path("foo").open()` | |
| PTH124 | pathlib-py-path | `py.path` is in maintenance mode, use `pathlib` instead | |
| PTH200 | [path-constructor-current-directory](https://github.com/charliermarsh/ruff/blob/main/docs/rules/path-constructor-current-directory.md) | Do not pass the current directory explicitly to `Path` | 🛠 |
| PTH201 | pathlib-getsize | `os.path.getsize` should be replaced by `stat().st_size` | |
| PTH202 | pathlib-getatime | `os.path.getatime` should be replaced by `stat().st_atime` | |
| PTH203 | pathlib-getmtime | `os.path.getmtime` should be replaced by `stat().st_mtime` | |
| PTH204 | pathlib-getctime | `os.path.getctime` should be replaced by `stat().st_ctime` | |

### eradicate (ERA)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import os
import os.path
import pathlib

p = "/foo"

a = os.path.abspath(p)
aa = os.chmod(p)
aa = os.chmod(p, mode=511)
aaa = os.mkdir(p)
os.makedirs(p)
os.rename(p)
os.replace(p)
os.rename(p, q)
os.replace(p, q)
os.rmdir(p)
os.remove(p)
os.unlink(p)
os.getcwd(p)
os.getcwd()
b = os.path.exists(p)
bb = os.path.expanduser(p)
bbb = os.path.isdir(p)
Expand All @@ -29,4 +30,4 @@
with open(p) as fp:
fp.read()
open(p).close()
os.getcwdb(p)
os.getcwdb()
34 changes: 34 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_use_pathlib/guarded.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# ensure that no fixes are applied when`pathlib` is not imported
# (needed until this item is resolved: https://github.com/charliermarsh/ruff/issues/835)
import os
import os.path

p = "/foo"

a = os.path.abspath(p)
aa = os.chmod(p)
aaa = os.mkdir(p)
os.makedirs(p)
os.rename(p)
os.replace(p)
os.rmdir(p)
os.remove(p)
os.unlink(p)
os.getcwd(p)
b = os.path.exists(p)
bb = os.path.expanduser(p)
bbb = os.path.isdir(p)
bbbb = os.path.isfile(p)
bbbbb = os.path.islink(p)
os.readlink(p)
os.stat(p)
os.path.isabs(p)
os.path.join(p)
os.path.basename(p)
os.path.dirname(p)
os.path.samefile(p)
os.path.splitext(p)
with open(p) as fp:
fp.read()
open(p).close()
os.getcwdb(p)
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os as foo
import os.path as foo_p
import pathlib as pth

p = "/foo"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from os import remove, unlink, getcwd, readlink, stat
from os.path import abspath, exists, expanduser, isdir, isfile, islink
from os.path import isabs, join, basename, dirname, samefile, splitext
from pathlib import Path

p = "/foo"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from os.path import isfile as xisfile, islink as xislink, isabs as xisabs
from os.path import join as xjoin, basename as xbasename, dirname as xdirname
from os.path import samefile as xsamefile, splitext as xsplitext
from pathlib import Path as pth

p = "/foo"

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from pathlib import Path
from pathlib import Path as pth

# match
_ = Path(".")
_ = pth(".")

# no match
_ = Path()
print(".")
Path("file.txt")
Path(".", "folder")
19 changes: 19 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_use_pathlib/stat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import os
from pathlib import Path

os.path.getatime("filename")
os.path.getatime(b"filename")
os.path.getatime(Path("filename"))

os.path.getmtime("filename")
os.path.getmtime(b"filename")
os.path.getmtime(Path("filename"))

os.path.getctime("filename")
os.path.getctime(b"filename")
os.path.getctime(Path("filename"))

os.path.getsize("filename")
os.path.getsize(b"filename")
os.path.getsize(Path("filename"))
os.path.getsize(__file__)
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
import os
from pathlib import Path

(Path("") / "").open()

_ = Path(os.getcwd())

_ = Path(
os.\
getcwd()
)

_ = Path(
os.getcwdb(),
)

# should not be unwrapped
_ = Path(os.getcwd(), hello='world')

# other cases
os.path.abspath("../../hello.py")
60 changes: 60 additions & 0 deletions crates/ruff/src/ast/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,6 +1146,66 @@ pub fn is_logger_candidate(func: &Expr) -> bool {
false
}

/// Return the appropriate `sys.exit` reference based on the current set of
/// imports, or `None` is `sys.exit` hasn't been imported.
pub fn get_member_import_name_alias(
checker: &Checker,
module: &str,
member: &str,
) -> Option<String> {
checker.current_scopes().find_map(|scope| {
scope
.bindings
.values()
.find_map(|index| match &checker.bindings[*index].kind {
// e.g. module=sys object=exit
// `import sys` -> `sys.exit`
// `import sys as sys2` -> `sys2.exit`
BindingKind::Importation(name, full_name) => {
if full_name == &module {
Some(format!("{name}.{member}"))
} else {
None
}
}
// e.g. module=os.path object=join
// `from os.path import join` -> `join`
// `from os.path import join as join2` -> `join2`
BindingKind::FromImportation(name, full_name) => {
let mut parts = full_name.split('.');
if parts.next() == Some(module)
&& parts.next() == Some(member)
&& parts.next().is_none()
{
Some((*name).to_string())
} else {
None
}
}
// e.g. module=os.path object=join
// `from os.path import *` -> `join`
BindingKind::StarImportation(_, name) => {
if name.as_ref().map(|name| name == module).unwrap_or_default() {
Some(member.to_string())
} else {
None
}
}
// e.g. module=os.path object=join
// `import os.path ` -> `os.path.join`
BindingKind::SubmoduleImportation(_, full_name) => {
if full_name == &module {
Some(format!("{full_name}.{member}"))
} else {
None
}
}
// Non-imports.
_ => None,
})
})
}

#[cfg(test)]
mod tests {
use anyhow::Result;
Expand Down
15 changes: 14 additions & 1 deletion crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2756,7 +2756,20 @@ where
|| self.settings.rules.enabled(&Rule::PathlibOpen)
|| self.settings.rules.enabled(&Rule::PathlibPyPath)
{
flake8_use_pathlib::helpers::replaceable_by_pathlib(self, func);
flake8_use_pathlib::helpers::replaceable_by_pathlib(
self,
expr,
func,
self.current_expr_parent().map(Into::into),
);
}

if self
.settings
.rules
.enabled(&Rule::PathConstructorCurrentDirectory)
{
flake8_use_pathlib::rules::simplify_path_constructor(self, expr, func);
}

// flake8-logging-format
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,11 @@ ruff_macros::define_rule_mapping!(
PTH122 => rules::flake8_use_pathlib::violations::PathlibSplitext,
PTH123 => rules::flake8_use_pathlib::violations::PathlibOpen,
PTH124 => rules::flake8_use_pathlib::violations::PathlibPyPath,
PTH200 => rules::flake8_use_pathlib::rules::PathConstructorCurrentDirectory,
PTH201 => rules::flake8_use_pathlib::violations::PathlibGetsize,
PTH202 => rules::flake8_use_pathlib::violations::PathlibGetatime,
PTH203 => rules::flake8_use_pathlib::violations::PathlibGetmtime,
PTH204 => rules::flake8_use_pathlib::violations::PathlibGetctime,
// flake8-logging-format
G001 => rules::flake8_logging_format::violations::LoggingStringFormat,
G002 => rules::flake8_logging_format::violations::LoggingPercentFormat,
Expand Down
Loading