diff --git a/crates/ruff_cli/tests/snapshots/integration_test__stdin_json.snap b/crates/ruff_cli/tests/snapshots/integration_test__stdin_json.snap index 01a422aa42629..c722ec3d6975f 100644 --- a/crates/ruff_cli/tests/snapshots/integration_test__stdin_json.snap +++ b/crates/ruff_cli/tests/snapshots/integration_test__stdin_json.snap @@ -6,7 +6,7 @@ info: - "-" - "--isolated" - "--no-cache" - - "--format" + - "--output-format" - json - "--stdin-filename" - F401.py diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB177.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB177.py new file mode 100644 index 0000000000000..48823dfb054a9 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB177.py @@ -0,0 +1,31 @@ +import pathlib +from pathlib import Path + +# Errors +_ = Path().resolve() +_ = pathlib.Path().resolve() + +_ = Path("").resolve() +_ = pathlib.Path("").resolve() + +_ = Path(".").resolve() +_ = pathlib.Path(".").resolve() + +_ = Path("", **kwargs).resolve() +_ = pathlib.Path("", **kwargs).resolve() + +_ = Path(".", **kwargs).resolve() +_ = pathlib.Path(".", **kwargs).resolve() + +# OK +_ = Path.cwd() +_ = pathlib.Path.cwd() + +_ = Path("foo").resolve() +_ = pathlib.Path("foo").resolve() + +_ = Path(".", "foo").resolve() +_ = pathlib.Path(".", "foo").resolve() + +_ = Path(*args).resolve() +_ = pathlib.Path(*args).resolve() diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 1d8a45018cf64..e33977e452f41 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -913,6 +913,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::ExceptionWithoutExcInfo) { flake8_logging::rules::exception_without_exc_info(checker, call); } + if checker.enabled(Rule::ImplicitCwd) { + refurb::rules::no_implicit_cwd(checker, call); + } } Expr::Dict( dict @ ast::ExprDict { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index c6741ce9b8027..9c0da4a5de4dc 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -925,6 +925,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap), (Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy), (Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate), + (Refurb, "177") => (RuleGroup::Preview, rules::refurb::rules::ImplicitCwd), // flake8-logging (Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation), diff --git a/crates/ruff_linter/src/registry/rule_set.rs b/crates/ruff_linter/src/registry/rule_set.rs index 77ce199b98b97..e5214d5e447b3 100644 --- a/crates/ruff_linter/src/registry/rule_set.rs +++ b/crates/ruff_linter/src/registry/rule_set.rs @@ -3,7 +3,7 @@ use ruff_macros::CacheKey; use std::fmt::{Debug, Formatter}; use std::iter::FusedIterator; -const RULESET_SIZE: usize = 11; +const RULESET_SIZE: usize = 12; /// A set of [`Rule`]s. /// diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 587b0f85fae6b..5a629d6fbac81 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -21,6 +21,7 @@ mod tests { #[test_case(Rule::SliceCopy, Path::new("FURB145.py"))] #[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))] #[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))] + #[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/refurb/rules/implicit_cwd.rs b/crates/ruff_linter/src/rules/refurb/rules/implicit_cwd.rs new file mode 100644 index 0000000000000..442909f496bcf --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/implicit_cwd.rs @@ -0,0 +1,108 @@ +use ruff_diagnostics::{Diagnostic, Edit, Fix, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Constant, Expr, ExprAttribute, ExprCall}; +use ruff_text_size::Ranged; + +use crate::{checkers::ast::Checker, importer::ImportRequest, registry::AsRule}; + +/// ## What it does +/// Checks for current-directory lookups using `Path().resolve()`. +/// +/// ## Why is this bad? +/// When looking up the current directory, prefer `Path.cwd()` over +/// `Path().resolve()`, as `Path.cwd()` is more explicit in its intent. +/// +/// ## Example +/// ```python +/// cwd = Path.resolve() +/// ``` +/// +/// Use instead: +/// ```python +/// cwd = Path.cwd() +/// ``` +/// +/// ## References +/// - [Python documentation: `Path.cwd`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd) + +#[violation] +pub struct ImplicitCwd; + +impl Violation for ImplicitCwd { + #[derive_message_formats] + fn message(&self) -> String { + format!("Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups") + } +} + +/// FURB177 +pub(crate) fn no_implicit_cwd(checker: &mut Checker, call: &ExprCall) { + if !call.arguments.is_empty() { + return; + } + + let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else { + return; + }; + + if attr != "resolve" { + return; + } + + let Expr::Call(ExprCall { + func, arguments, .. + }) = value.as_ref() + else { + return; + }; + + // Match on arguments, but ignore keyword arguments. `Path()` accepts keyword arguments, but + // ignores them. See: https://github.com/python/cpython/issues/98094. + match arguments.args.as_slice() { + // Ex) `Path().resolve()` + [] => {} + // Ex) `Path(".").resolve()` + [arg] => { + let Expr::Constant(ast::ExprConstant { + value: Constant::Str(str), + .. + }) = arg + else { + return; + }; + if !matches!(str.value.as_str(), "" | ".") { + return; + } + } + // Ex) `Path("foo", "bar").resolve()` + _ => return, + } + + if !checker + .semantic() + .resolve_call_path(func) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["pathlib", "Path"])) + { + return; + } + + let mut diagnostic = Diagnostic::new(ImplicitCwd, call.range()); + + if checker.patch(diagnostic.kind.rule()) { + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("pathlib", "Path"), + call.start(), + checker.semantic(), + )?; + Ok(Fix::suggested_edits( + Edit::range_replacement(format!("{binding}.cwd()"), call.range()), + [import_edit], + )) + }); + } + + checker + .diagnostics + .push(Diagnostic::new(ImplicitCwd, call.range())); +} diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index a3277d79f4667..9541fa9ddd5b6 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -1,5 +1,6 @@ pub(crate) use check_and_remove_from_set::*; pub(crate) use delete_full_slice::*; +pub(crate) use implicit_cwd::*; pub(crate) use print_empty_string::*; pub(crate) use reimplemented_starmap::*; pub(crate) use repeated_append::*; @@ -8,6 +9,7 @@ pub(crate) use unnecessary_enumerate::*; mod check_and_remove_from_set; mod delete_full_slice; +mod implicit_cwd; mod print_empty_string; mod reimplemented_starmap; mod repeated_append; diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB177_FURB177.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB177_FURB177.py.snap new file mode 100644 index 0000000000000..369b9a893a49d --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB177_FURB177.py.snap @@ -0,0 +1,94 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB177.py:5:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +4 | # Errors +5 | _ = Path().resolve() + | ^^^^^^^^^^^^^^^^ FURB177 +6 | _ = pathlib.Path().resolve() + | + +FURB177.py:6:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +4 | # Errors +5 | _ = Path().resolve() +6 | _ = pathlib.Path().resolve() + | ^^^^^^^^^^^^^^^^^^^^^^^^ FURB177 +7 | +8 | _ = Path("").resolve() + | + +FURB177.py:8:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +6 | _ = pathlib.Path().resolve() +7 | +8 | _ = Path("").resolve() + | ^^^^^^^^^^^^^^^^^^ FURB177 +9 | _ = pathlib.Path("").resolve() + | + +FURB177.py:9:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | + 8 | _ = Path("").resolve() + 9 | _ = pathlib.Path("").resolve() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB177 +10 | +11 | _ = Path(".").resolve() + | + +FURB177.py:11:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | + 9 | _ = pathlib.Path("").resolve() +10 | +11 | _ = Path(".").resolve() + | ^^^^^^^^^^^^^^^^^^^ FURB177 +12 | _ = pathlib.Path(".").resolve() + | + +FURB177.py:12:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +11 | _ = Path(".").resolve() +12 | _ = pathlib.Path(".").resolve() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB177 +13 | +14 | _ = Path("", **kwargs).resolve() + | + +FURB177.py:14:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +12 | _ = pathlib.Path(".").resolve() +13 | +14 | _ = Path("", **kwargs).resolve() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB177 +15 | _ = pathlib.Path("", **kwargs).resolve() + | + +FURB177.py:15:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +14 | _ = Path("", **kwargs).resolve() +15 | _ = pathlib.Path("", **kwargs).resolve() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB177 +16 | +17 | _ = Path(".", **kwargs).resolve() + | + +FURB177.py:17:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +15 | _ = pathlib.Path("", **kwargs).resolve() +16 | +17 | _ = Path(".", **kwargs).resolve() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB177 +18 | _ = pathlib.Path(".", **kwargs).resolve() + | + +FURB177.py:18:5: FURB177 Prefer `Path.cwd()` over `Path().resolve()` for current-directory lookups + | +17 | _ = Path(".", **kwargs).resolve() +18 | _ = pathlib.Path(".", **kwargs).resolve() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB177 +19 | +20 | # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index f2a2a4f0fe9ab..b445036b53530 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2676,6 +2676,8 @@ "FURB140", "FURB145", "FURB148", + "FURB17", + "FURB177", "G", "G0", "G00",