Skip to content

Commit

Permalink
[flake8-use-pathlib] Implement os-path-getsize and `os-path-get(a…
Browse files Browse the repository at this point in the history
…|m|c)-time` (`PTH202-205`) (#5835)

Reviving #2348 step by step

Pt 3. implement detection for:
- `os.path.getsize`
- `os.path.getmtime`
- `os.path.getctime`
- `os.path.getatime`
  • Loading branch information
sbrugman authored Jul 20, 2023
1 parent d35cb69 commit 4bba0bc
Show file tree
Hide file tree
Showing 19 changed files with 498 additions and 2 deletions.
14 changes: 14 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH202.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import os.path
from pathlib import Path
from os.path import getsize


os.path.getsize("filename")
os.path.getsize(b"filename")
os.path.getsize(Path("filename"))
os.path.getsize(__file__)

getsize("filename")
getsize(b"filename")
getsize(Path("filename"))
getsize(__file__)
12 changes: 12 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH203.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import os.path
from pathlib import Path
from os.path import getatime

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


getatime("filename")
getatime(b"filename")
getatime(Path("filename"))
13 changes: 13 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH204.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import os.path
from pathlib import Path
from os.path import getmtime


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


getmtime("filename")
getmtime(b"filename")
getmtime(Path("filename"))
12 changes: 12 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH205.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import os.path
from pathlib import Path
from os.path import getctime


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

getctime("filename")
getctime(b"filename")
getctime(Path("filename"))
4 changes: 4 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3024,6 +3024,10 @@ where
Rule::OsPathSplitext,
Rule::BuiltinOpen,
Rule::PyPath,
Rule::OsPathGetsize,
Rule::OsPathGetatime,
Rule::OsPathGetmtime,
Rule::OsPathGetctime,
]) {
flake8_use_pathlib::rules::replaceable_by_pathlib(self, func);
}
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,11 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(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),
(Flake8UsePathlib, "202") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetsize),
(Flake8UsePathlib, "202") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetsize),
(Flake8UsePathlib, "203") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetatime),
(Flake8UsePathlib, "204") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetmtime),
(Flake8UsePathlib, "205") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetctime),

// flake8-logging-format
(Flake8LoggingFormat, "001") => (RuleGroup::Unspecified, rules::flake8_logging_format::violations::LoggingStringFormat),
Expand Down
5 changes: 4 additions & 1 deletion crates/ruff/src/rules/flake8_use_pathlib/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ 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"))]

#[test_case(Rule::OsPathGetsize, Path::new("PTH202.py"))]
#[test_case(Rule::OsPathGetatime, Path::new("PTH203.py"))]
#[test_case(Rule::OsPathGetmtime, Path::new("PTH204.py"))]
#[test_case(Rule::OsPathGetctime, Path::new("PTH205.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
8 changes: 8 additions & 0 deletions crates/ruff/src/rules/flake8_use_pathlib/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
pub(crate) use os_path_getatime::*;
pub(crate) use os_path_getctime::*;
pub(crate) use os_path_getmtime::*;
pub(crate) use os_path_getsize::*;
pub(crate) use path_constructor_current_directory::*;
pub(crate) use replaceable_by_pathlib::*;

mod os_path_getatime;
mod os_path_getctime;
mod os_path_getmtime;
mod os_path_getsize;
mod path_constructor_current_directory;
mod replaceable_by_pathlib;
43 changes: 43 additions & 0 deletions crates/ruff/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};

/// ## What it does
/// Checks for uses of `os.path.getatime`.
///
/// ## 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.stat()` can
/// improve readability over the `os` module's counterparts (e.g.,
/// `os.path.getsize()`).
///
/// Note that `os` functions may be preferable if performance is a concern,
/// e.g., in hot loops.
///
/// ## Examples
/// ```python
/// os.path.getsize(__file__)
/// ```
///
/// Use instead:
/// ```python
/// Path(__file__).stat().st_size
/// ```
///
/// ## References
/// - [Python documentation: `Path.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
/// - [Python documentation: `os.path.getsize`](https://docs.python.org/3/library/os.path.html#os.path.getsize)
/// - [PEP 428](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/)
#[violation]
pub struct OsPathGetatime;

impl Violation for OsPathGetatime {
#[derive_message_formats]
fn message(&self) -> String {
format!("`os.path.getatime` should be replaced by `Path.stat().st_atime`")
}
}
43 changes: 43 additions & 0 deletions crates/ruff/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};

/// ## What it does
/// Checks for uses of `os.path.getatime`.
///
/// ## 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.stat()` can
/// improve readability over the `os` module's counterparts (e.g.,
/// `os.path.getsize()`).
///
/// Note that `os` functions may be preferable if performance is a concern,
/// e.g., in hot loops.
///
/// ## Examples
/// ```python
/// os.path.getsize(__file__)
/// ```
///
/// Use instead:
/// ```python
/// Path(__file__).stat().st_size
/// ```
///
/// ## References
/// - [Python documentation: `Path.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
/// - [Python documentation: `os.path.getsize`](https://docs.python.org/3/library/os.path.html#os.path.getsize)
/// - [PEP 428](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/)
#[violation]
pub struct OsPathGetctime;

impl Violation for OsPathGetctime {
#[derive_message_formats]
fn message(&self) -> String {
format!("`os.path.getctime` should be replaced by `Path.stat().st_ctime`")
}
}
43 changes: 43 additions & 0 deletions crates/ruff/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};

/// ## What it does
/// Checks for uses of `os.path.getatime`.
///
/// ## 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.stat()` can
/// improve readability over the `os` module's counterparts (e.g.,
/// `os.path.getsize()`).
///
/// Note that `os` functions may be preferable if performance is a concern,
/// e.g., in hot loops.
///
/// ## Examples
/// ```python
/// os.path.getsize(__file__)
/// ```
///
/// Use instead:
/// ```python
/// Path(__file__).stat().st_size
/// ```
///
/// ## References
/// - [Python documentation: `Path.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
/// - [Python documentation: `os.path.getsize`](https://docs.python.org/3/library/os.path.html#os.path.getsize)
/// - [PEP 428](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/)
#[violation]
pub struct OsPathGetmtime;

impl Violation for OsPathGetmtime {
#[derive_message_formats]
fn message(&self) -> String {
format!("`os.path.getmtime` should be replaced by `Path.stat().st_mtime`")
}
}
43 changes: 43 additions & 0 deletions crates/ruff/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};

/// ## What it does
/// Checks for uses of `os.path.getsize`.
///
/// ## 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.stat()` can
/// improve readability over the `os` module's counterparts (e.g.,
/// `os.path.getsize()`).
///
/// Note that `os` functions may be preferable if performance is a concern,
/// e.g., in hot loops.
///
/// ## Examples
/// ```python
/// os.path.getsize(__file__)
/// ```
///
/// Use instead:
/// ```python
/// Path(__file__).stat().st_size
/// ```
///
/// ## References
/// - [Python documentation: `Path.stat`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.stat)
/// - [Python documentation: `os.path.getsize`](https://docs.python.org/3/library/os.path.html#os.path.getsize)
/// - [PEP 428](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/)
#[violation]
pub struct OsPathGetsize;

impl Violation for OsPathGetsize {
#[derive_message_formats]
fn message(&self) -> String {
format!("`os.path.getsize` should be replaced by `Path.stat().st_size`")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub(crate) fn path_constructor_current_directory(checker: &mut Checker, expr: &E
let [Expr::Constant(ExprConstant {
value: Constant::Str(value),
kind: _,
range
range,
})] = args.as_slice()
else {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ use ruff_diagnostics::{Diagnostic, DiagnosticKind};

use crate::checkers::ast::Checker;
use crate::registry::AsRule;
use crate::rules::flake8_use_pathlib::rules::{
OsPathGetatime, OsPathGetctime, OsPathGetmtime, OsPathGetsize,
};
use crate::rules::flake8_use_pathlib::violations::{
BuiltinOpen, OsChmod, OsGetcwd, OsMakedirs, OsMkdir, OsPathAbspath, OsPathBasename,
OsPathDirname, OsPathExists, OsPathExpanduser, OsPathIsabs, OsPathIsdir, OsPathIsfile,
Expand Down Expand Up @@ -41,6 +44,14 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, expr: &Expr) {
["os", "path", "dirname"] => Some(OsPathDirname.into()),
["os", "path", "samefile"] => Some(OsPathSamefile.into()),
["os", "path", "splitext"] => Some(OsPathSplitext.into()),
// PTH202
["os", "path", "getsize"] => Some(OsPathGetsize.into()),
// PTH203
["os", "path", "getatime"] => Some(OsPathGetatime.into()),
// PTH204
["os", "path", "getmtime"] => Some(OsPathGetmtime.into()),
// PTH205
["os", "path", "getctime"] => Some(OsPathGetctime.into()),
["", "open"] => Some(BuiltinOpen.into()),
["py", "path", "local"] => Some(PyPath.into()),
// Python 3.9+
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
source: crates/ruff/src/rules/flake8_use_pathlib/mod.rs
---
PTH202.py:6:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
6 | os.path.getsize("filename")
| ^^^^^^^^^^^^^^^ PTH202
7 | os.path.getsize(b"filename")
8 | os.path.getsize(Path("filename"))
|

PTH202.py:7:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
6 | os.path.getsize("filename")
7 | os.path.getsize(b"filename")
| ^^^^^^^^^^^^^^^ PTH202
8 | os.path.getsize(Path("filename"))
9 | os.path.getsize(__file__)
|

PTH202.py:8:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
6 | os.path.getsize("filename")
7 | os.path.getsize(b"filename")
8 | os.path.getsize(Path("filename"))
| ^^^^^^^^^^^^^^^ PTH202
9 | os.path.getsize(__file__)
|

PTH202.py:9:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
7 | os.path.getsize(b"filename")
8 | os.path.getsize(Path("filename"))
9 | os.path.getsize(__file__)
| ^^^^^^^^^^^^^^^ PTH202
10 |
11 | getsize("filename")
|

PTH202.py:11:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
9 | os.path.getsize(__file__)
10 |
11 | getsize("filename")
| ^^^^^^^ PTH202
12 | getsize(b"filename")
13 | getsize(Path("filename"))
|

PTH202.py:12:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
11 | getsize("filename")
12 | getsize(b"filename")
| ^^^^^^^ PTH202
13 | getsize(Path("filename"))
14 | getsize(__file__)
|

PTH202.py:13:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
11 | getsize("filename")
12 | getsize(b"filename")
13 | getsize(Path("filename"))
| ^^^^^^^ PTH202
14 | getsize(__file__)
|

PTH202.py:14:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
|
12 | getsize(b"filename")
13 | getsize(Path("filename"))
14 | getsize(__file__)
| ^^^^^^^ PTH202
|


Loading

0 comments on commit 4bba0bc

Please sign in to comment.