Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,81 @@
from pathlib import Path
from os.path import getsize

filename = "filename"
filename1 = __file__
filename2 = Path("filename")


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

os.path.getsize(filename)
os.path.getsize(filename1)
os.path.getsize(filename2)

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

getsize("filename")
getsize(b"filename")
getsize(Path("filename"))
getsize(__file__)

getsize(filename="filename")
getsize(filename=b"filename")
getsize(filename=Path("filename"))
getsize(filename=__file__)

getsize(filename)
getsize(filename1)
getsize(filename2)


os.path.getsize(
"filename", # comment
)

os.path.getsize(
# comment
"filename"
,
# comment
)

os.path.getsize(
# comment
b"filename"
# comment
)

os.path.getsize( # comment
Path(__file__)
# comment
) # comment

getsize( # comment
"filename")

getsize( # comment
b"filename",
#comment
)

os.path.getsize("file" + "name")

getsize \
\
\
( # comment
"filename",
)

getsize(Path("filename").resolve())

import pathlib

os.path.getsize(pathlib.Path("filename"))
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import os

os.path.getsize(filename="filename")
os.path.getsize(filename=b"filename")
os.path.getsize(filename=__file__)
4 changes: 3 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,6 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
Rule::OsPathSplitext,
Rule::BuiltinOpen,
Rule::PyPath,
Rule::OsPathGetsize,
Rule::OsPathGetatime,
Rule::OsPathGetmtime,
Rule::OsPathGetctime,
Expand All @@ -1066,6 +1065,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
]) {
flake8_use_pathlib::rules::replaceable_by_pathlib(checker, call);
}
if checker.is_rule_enabled(Rule::OsPathGetsize) {
flake8_use_pathlib::rules::os_path_getsize(checker, call);
}
if checker.is_rule_enabled(Rule::PathConstructorCurrentDirectory) {
flake8_use_pathlib::rules::path_constructor_current_directory(checker, call);
}
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ pub(crate) const fn is_fix_manual_list_comprehension_enabled(settings: &LinterSe
settings.preview.is_enabled()
}

// https://github.com/astral-sh/ruff/pull/18763
pub(crate) const fn is_fix_os_path_getsize_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 {
Expand Down
21 changes: 21 additions & 0 deletions crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod tests {
use crate::assert_diagnostics;
use crate::registry::Rule;
use crate::settings;
use crate::settings::types::PreviewMode;
use crate::test::test_path;

#[test_case(Path::new("full_name.py"))]
Expand Down Expand Up @@ -58,6 +59,7 @@ mod tests {
#[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::OsPathGetsize, Path::new("PTH202_2.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"))]
Expand All @@ -76,4 +78,23 @@ mod tests {
assert_diagnostics!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::OsPathGetsize, Path::new("PTH202.py"))]
#[test_case(Rule::OsPathGetsize, Path::new("PTH202_2.py"))]
fn preview_flake8_use_pathlib(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("flake8_use_pathlib").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_diagnostics!(snapshot, diagnostics);
Ok(())
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
use crate::preview::is_fix_os_path_getsize_enabled;
use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{ViolationMetadata, derive_message_formats};

use crate::Violation;
use ruff_python_ast::name::QualifiedName;
use ruff_python_ast::{Expr, ExprCall};
use ruff_text_size::Ranged;

/// ## What it does
/// Checks for uses of `os.path.getsize`.
Expand Down Expand Up @@ -32,6 +37,9 @@ use crate::Violation;
/// 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.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)
Expand All @@ -43,8 +51,77 @@ use crate::Violation;
pub(crate) struct OsPathGetsize;

impl Violation for OsPathGetsize {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
"`os.path.getsize` should be replaced by `Path.stat().st_size`".to_string()
}

fn fix_title(&self) -> Option<String> {
Some("Replace with `Path(...).stat().st_size`".to_string())
}
}

/// PTH202
pub(crate) fn os_path_getsize(checker: &Checker, call: &ExprCall) {
if !matches!(
checker
.semantic()
.resolve_qualified_name(&call.func)
.as_ref()
.map(QualifiedName::segments),
Some(["os", "path", "getsize"])
) {
return;
}

if call.arguments.len() != 1 {
return;
}

let Some(arg) = call.arguments.find_argument_value("filename", 0) else {
return;
};

let arg_code = checker.locator().slice(arg.range());
let range = call.range();

let applicability = if checker.comment_ranges().intersects(range) {
Applicability::Unsafe
} else {
Applicability::Safe
};

let mut diagnostic = checker.report_diagnostic(OsPathGetsize, range);

if is_fix_os_path_getsize_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 replacement = if is_path_call(checker, arg) {
format!("{arg_code}.stat().st_size")
} else {
format!("{binding}({arg_code}).stat().st_size")
};

Ok(
Fix::safe_edits(Edit::range_replacement(replacement, range), [import_edit])
.with_applicability(applicability),
)
});
}
}

fn is_path_call(checker: &Checker, expr: &Expr) -> bool {
expr.as_call_expr().is_some_and(|expr_call| {
checker
.semantic()
.resolve_qualified_name(&expr_call.func)
.is_some_and(|name| matches!(name.segments(), ["pathlib", "Path"]))
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::rules::flake8_use_pathlib::rules::{
Glob, OsPathGetatime, OsPathGetctime, OsPathGetmtime, OsPathGetsize,
Glob, OsPathGetatime, OsPathGetctime, OsPathGetmtime,
};
use crate::rules::flake8_use_pathlib::violations::{
BuiltinOpen, Joiner, OsChmod, OsGetcwd, OsListdir, OsMakedirs, OsMkdir, OsPathAbspath,
Expand Down Expand Up @@ -194,8 +194,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
["os", "path", "samefile"] => checker.report_diagnostic_if_enabled(OsPathSamefile, range),
// PTH122
["os", "path", "splitext"] => checker.report_diagnostic_if_enabled(OsPathSplitext, range),
// PTH202
["os", "path", "getsize"] => checker.report_diagnostic_if_enabled(OsPathGetsize, range),
// PTH203
["os", "path", "getatime"] => checker.report_diagnostic_if_enabled(OsPathGetatime, range),
// PTH204
Expand Down
Loading
Loading