Skip to content

Commit

Permalink
flake8-use-pathlib autofix and new rules
Browse files Browse the repository at this point in the history
PTH200-204:
- Simplify path constructor `Path(".")` to `Path()`
- Replace `os.path.getsize` with `Path.stat()`

Autofix for:
- PTH200: `Path(".")` to `Path()`
- PTH109: `os.path.getcwd(x)` to `Path(x).cwd()`
  • Loading branch information
sbrugman committed Jan 30, 2023
1 parent 7a83b65 commit 0a47e2c
Show file tree
Hide file tree
Showing 22 changed files with 634 additions and 115 deletions.
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,7 @@ For more, see [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/)
| 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()` | |
| 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()` | |
Expand All @@ -1232,6 +1232,11 @@ For more, see [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/)
| 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 | simplify-path-constructor | 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
@@ -0,0 +1,10 @@
from pathlib import Path

# match
_ = Path(".")

# no match
_ = Path()
print(".")
Path("file.txt")
Path(".", "folder")
19 changes: 19 additions & 0 deletions 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__)
15 changes: 15 additions & 0 deletions resources/test/fixtures/flake8_use_pathlib/use_pathlib.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
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')
7 changes: 7 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1719,6 +1719,13 @@
"PTH122",
"PTH123",
"PTH124",
"PTH2",
"PTH20",
"PTH200",
"PTH201",
"PTH202",
"PTH203",
"PTH204",
"Q",
"Q0",
"Q00",
Expand Down
10 changes: 9 additions & 1 deletion src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2661,7 +2661,15 @@ 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,
self.current_expr_parent().map(Into::into),
);
}

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

// flake8-logging-format
Expand Down
5 changes: 5 additions & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,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::SimplifyPathConstructor,
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
68 changes: 68 additions & 0 deletions src/rules/flake8_use_pathlib/fixes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use rustpython_ast::{Expr, ExprKind};

use crate::ast::helpers;
use crate::ast::types::Range;
use crate::autofix::apply_fix;
use crate::checkers::ast::Checker;
use crate::fix::Fix;
use crate::registry::DiagnosticKind;
use crate::source_code::Locator;

pub fn pathlib_fix(
checker: &mut Checker,
diagnostic: &DiagnosticKind,
expr: &Expr,
parent: Option<&Expr>,
) -> Option<Fix> {
// TODO: if pathlib not imported, then add to imports. Remove os import if not referenced
if let ExprKind::Call { func, .. } = &expr.node {
let mut fix = match diagnostic {
DiagnosticKind::PathlibGetcwd(_) => Some(Fix::replacement(
"Path.cwd".to_string(),
func.location,
func.end_location.unwrap(),
)),
_ => None,
};

// Wrapped in a `Path()` call
if let Some(fixme) = fix.clone() {
if let Some(parent) = parent {
if checker
.resolve_call_path(parent)
.map_or(false, |call_path| {
call_path.as_slice() == ["pathlib", "Path"]
})
{
if let ExprKind::Call { args, keywords, .. } = &parent.node {
if args.len() == 1 && keywords.is_empty() {
// Reset the line index.
let fixme = Fix::replacement(
fixme.content.to_string(),
helpers::to_relative(fixme.location, expr.location),
helpers::to_relative(fixme.end_location, expr.location),
);

// Apply the fix to
let arg = args.first().unwrap();
let contents = checker.locator.slice_source_code_range(&Range::new(
arg.location,
arg.end_location.unwrap(),
));
let output = apply_fix(&fixme, &Locator::new(contents));

fix = Some(Fix::replacement(
output,
parent.location,
parent.end_location.unwrap(),
));
}
}
}
}
}
fix
} else {
None
}
}
24 changes: 18 additions & 6 deletions src/rules/flake8_use_pathlib/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ use crate::checkers::ast::Checker;
use crate::registry::{Diagnostic, DiagnosticKind};
use crate::rules::flake8_use_pathlib::violations::{
PathlibAbspath, PathlibBasename, PathlibChmod, PathlibDirname, PathlibExists,
PathlibExpanduser, PathlibGetcwd, PathlibIsAbs, PathlibIsDir, PathlibIsFile, PathlibIsLink,
PathlibJoin, PathlibMakedirs, PathlibMkdir, PathlibOpen, PathlibPyPath, PathlibReadlink,
PathlibRemove, PathlibRename, PathlibReplace, PathlibRmdir, PathlibSamefile, PathlibSplitext,
PathlibStat, PathlibUnlink,
PathlibExpanduser, PathlibGetatime, PathlibGetctime, PathlibGetcwd, PathlibGetmtime,
PathlibGetsize, PathlibIsAbs, PathlibIsDir, PathlibIsFile, PathlibIsLink, PathlibJoin,
PathlibMakedirs, PathlibMkdir, PathlibOpen, PathlibPyPath, PathlibReadlink, PathlibRemove,
PathlibRename, PathlibReplace, PathlibRmdir, PathlibSamefile, PathlibSplitext, PathlibStat,
PathlibUnlink,
};

pub fn replaceable_by_pathlib(checker: &mut Checker, expr: &Expr) {
use super::fixes::pathlib_fix;

pub fn replaceable_by_pathlib(checker: &mut Checker, expr: &Expr, parent: Option<&Expr>) {
if let Some(diagnostic_kind) =
checker
.resolve_call_path(expr)
Expand Down Expand Up @@ -42,11 +45,20 @@ pub fn replaceable_by_pathlib(checker: &mut Checker, expr: &Expr) {
["os", "path", "splitext"] => Some(PathlibSplitext.into()),
["", "open"] => Some(PathlibOpen.into()),
["py", "path", "local"] => Some(PathlibPyPath.into()),
["os", "path", "getsize"] => Some(PathlibGetsize.into()),
["os", "path", "getatime"] => Some(PathlibGetatime.into()),
["os", "path", "getmtime"] => Some(PathlibGetmtime.into()),
["os", "path", "getctime"] => Some(PathlibGetctime.into()),
_ => None,
})
{
let diagnostic =
let mut diagnostic =
Diagnostic::new::<DiagnosticKind>(diagnostic_kind, Range::from_located(expr));
if checker.patch(diagnostic.kind.rule()) {
if let Some(fix) = pathlib_fix(checker, &diagnostic.kind, expr, parent) {
diagnostic.amend(fix);
}
}

if checker.settings.rules.enabled(diagnostic.kind.rule()) {
checker.diagnostics.push(diagnostic);
Expand Down
9 changes: 9 additions & 0 deletions src/rules/flake8_use_pathlib/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Rules from [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/).
pub(crate) mod fixes;
pub(crate) mod helpers;
pub(crate) mod rules;
pub(crate) mod violations;

#[cfg(test)]
Expand All @@ -18,6 +20,8 @@ mod tests {
#[test_case(Path::new("import_from_as.py"); "PTH1_3")]
#[test_case(Path::new("import_from.py"); "PTH1_4")]
#[test_case(Path::new("use_pathlib.py"); "PTH1_5")]
#[test_case(Path::new("stat.py"); "PTH1_6")]
#[test_case(Path::new("simplify_pathlib_constructor.py"); "PTH1_7")]
fn rules(path: &Path) -> Result<()> {
let snapshot = format!("{}", path.to_string_lossy());
let diagnostics = test_path(
Expand Down Expand Up @@ -49,6 +53,11 @@ mod tests {
Rule::PathlibSamefile,
Rule::PathlibSplitext,
Rule::PathlibOpen,
Rule::PathlibGetsize,
Rule::PathlibGetatime,
Rule::PathlibGetmtime,
Rule::PathlibGetctime,
Rule::SimplifyPathConstructor,
]),
)?;
insta::assert_yaml_snapshot!(snapshot, diagnostics);
Expand Down
3 changes: 3 additions & 0 deletions src/rules/flake8_use_pathlib/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub use simplify_path_constructor::{simplify_path_constructor, SimplifyPathConstructor};

mod simplify_path_constructor;
51 changes: 51 additions & 0 deletions src/rules/flake8_use_pathlib/rules/simplify_path_constructor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use ruff_macros::derive_message_formats;
use rustpython_ast::{Constant, Expr, ExprKind};

use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::define_violation;
use crate::fix::Fix;
use crate::registry::Diagnostic;
use crate::violation::AlwaysAutofixableViolation;

define_violation!(
pub struct SimplifyPathConstructor;
);
impl AlwaysAutofixableViolation for SimplifyPathConstructor {
#[derive_message_formats]
fn message(&self) -> String {
format!("Do not pass the current directory explicitly to `Path`")
}

fn autofix_title(&self) -> String {
format!("Replace `Path(\".\")` with `Path()`")
}
}

/// PTH200
pub fn simplify_path_constructor(checker: &mut Checker, expr: &Expr) {
if checker.resolve_call_path(expr).map_or(false, |call_path| {
call_path.as_slice() == ["pathlib", "Path"]
}) {
if let ExprKind::Call { args, keywords, .. } = &expr.node {
if keywords.is_empty() && args.len() == 1 {
let arg = &args.first().unwrap();
if let ExprKind::Constant {
value: Constant::Str(value),
..
} = &arg.node
{
if value == "." {
let mut diagnostic =
Diagnostic::new(SimplifyPathConstructor, Range::from_located(expr));
if checker.patch(diagnostic.kind.rule()) {
diagnostic
.amend(Fix::deletion(arg.location, arg.end_location.unwrap()));
}
checker.diagnostics.push(diagnostic);
};
};
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ expression: diagnostics
column: 4
end_location:
row: 3
column: 17
column: 27
fix: ~
parent: ~

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ expression: diagnostics
column: 4
end_location:
row: 3
column: 8
column: 16
fix: ~
parent: ~

Loading

0 comments on commit 0a47e2c

Please sign in to comment.