Skip to content

Commit

Permalink
[flake8-use-pathlib] Implement path-constructor-default-argument
Browse files Browse the repository at this point in the history
…(`PTH201`) (#5833)

Reviving #2348 step by step

Pt 2. PTH201: Path Constructor Default Argument

- rule originates from `refurb`:
#1348
- Using PTH201 rather than FURBXXX to keep all pathlib logic together
  • Loading branch information
sbrugman authored Jul 20, 2023
1 parent a37d915 commit d35cb69
Show file tree
Hide file tree
Showing 8 changed files with 175 additions and 0 deletions.
14 changes: 14 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH201.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from pathlib import Path, PurePath
from pathlib import Path as pth

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

# no match
_ = Path()
print(".")
Path("file.txt")
Path(".", "folder")
PurePath(".", "folder")
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3027,6 +3027,9 @@ where
]) {
flake8_use_pathlib::rules::replaceable_by_pathlib(self, func);
}
if self.enabled(Rule::PathConstructorCurrentDirectory) {
flake8_use_pathlib::rules::path_constructor_current_directory(self, expr, func);
}
if self.enabled(Rule::NumpyLegacyRandom) {
numpy::rules::legacy_random(self, func);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8UsePathlib, "122") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::violations::OsPathSplitext),
(Flake8UsePathlib, "123") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::violations::BuiltinOpen),
(Flake8UsePathlib, "124") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::violations::PyPath),
(Flake8UsePathlib, "201") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::PathConstructorCurrentDirectory),

// flake8-logging-format
(Flake8LoggingFormat, "001") => (RuleGroup::Unspecified, rules::flake8_logging_format::violations::LoggingStringFormat),
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_use_pathlib/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ mod tests {

#[test_case(Rule::PyPath, Path::new("py_path_1.py"))]
#[test_case(Rule::PyPath, Path::new("py_path_2.py"))]
#[test_case(Rule::PathConstructorCurrentDirectory, Path::new("PTH201.py"))]

fn rules_pypath(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_use_pathlib/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub(crate) use path_constructor_current_directory::*;
pub(crate) use replaceable_by_pathlib::*;

mod path_constructor_current_directory;
mod replaceable_by_pathlib;
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
use rustpython_parser::ast::{Constant, Expr, ExprCall, ExprConstant};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;
use crate::registry::AsRule;

/// ## What it does
/// Checks for `pathlib.Path` objects that are initialized with the current
/// directory.
///
/// ## Why is this bad?
/// The `Path()` constructor defaults to the current directory, so passing it
/// in explicitly (as `"."`) is unnecessary.
///
/// ## Example
/// ```python
/// from pathlib import Path
///
/// _ = Path(".")
/// ```
///
/// Use instead:
/// ```python
/// from pathlib import Path
///
/// _ = Path()
/// ```
///
/// ## References
/// - [Python documentation: `Path`](https://docs.python.org/3/library/pathlib.html#pathlib.Path)
#[violation]
pub struct PathConstructorCurrentDirectory;

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

fn autofix_title(&self) -> String {
"Remove the current directory argument".to_string()
}
}

/// PTH201
pub(crate) fn path_constructor_current_directory(checker: &mut Checker, expr: &Expr, func: &Expr) {
if !checker
.semantic()
.resolve_call_path(func)
.map_or(false, |call_path| {
matches!(call_path.as_slice(), ["pathlib", "Path" | "PurePath"])
})
{
return;
}

let Expr::Call(ExprCall { args, keywords, .. }) = expr else {
return;
};

if !keywords.is_empty() {
return;
}

let [Expr::Constant(ExprConstant {
value: Constant::Str(value),
kind: _,
range
})] = args.as_slice()
else {
return;
};

if value != "." {
return;
}

let mut diagnostic = Diagnostic::new(PathConstructorCurrentDirectory, *range);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_deletion(*range)));
}
checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
---
source: crates/ruff/src/rules/flake8_use_pathlib/mod.rs
---
PTH201.py:5:10: PTH201 [*] Do not pass the current directory explicitly to `Path`
|
4 | # match
5 | _ = Path(".")
| ^^^ PTH201
6 | _ = pth(".")
7 | _ = PurePath(".")
|
= help: Remove the current directory argument

Fix
2 2 | from pathlib import Path as pth
3 3 |
4 4 | # match
5 |-_ = Path(".")
5 |+_ = Path()
6 6 | _ = pth(".")
7 7 | _ = PurePath(".")
8 8 |

PTH201.py:6:9: PTH201 [*] Do not pass the current directory explicitly to `Path`
|
4 | # match
5 | _ = Path(".")
6 | _ = pth(".")
| ^^^ PTH201
7 | _ = PurePath(".")
|
= help: Remove the current directory argument

Fix
3 3 |
4 4 | # match
5 5 | _ = Path(".")
6 |-_ = pth(".")
6 |+_ = pth()
7 7 | _ = PurePath(".")
8 8 |
9 9 | # no match

PTH201.py:7:14: PTH201 [*] Do not pass the current directory explicitly to `Path`
|
5 | _ = Path(".")
6 | _ = pth(".")
7 | _ = PurePath(".")
| ^^^ PTH201
8 |
9 | # no match
|
= help: Remove the current directory argument

Fix
4 4 | # match
5 5 | _ = Path(".")
6 6 | _ = pth(".")
7 |-_ = PurePath(".")
7 |+_ = PurePath()
8 8 |
9 9 | # no match
10 10 | _ = Path()


3 changes: 3 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d35cb69

Please sign in to comment.