mirror of https://github.com/astral-sh/ruff
[`refurb`] Implement `implicit-cwd` (FURB177) (#7704)
## Summary Implement [`no-implicit-cwd`](https://github.com/dosisod/refurb/blob/master/docs/checks.md#furb177-no-implicit-cwd) as `implicit-cwd` Related to #1348. ## Test Plan `cargo test`
This commit is contained in:
parent
246d93ec37
commit
78b8741352
|
|
@ -6,7 +6,7 @@ info:
|
||||||
- "-"
|
- "-"
|
||||||
- "--isolated"
|
- "--isolated"
|
||||||
- "--no-cache"
|
- "--no-cache"
|
||||||
- "--format"
|
- "--output-format"
|
||||||
- json
|
- json
|
||||||
- "--stdin-filename"
|
- "--stdin-filename"
|
||||||
- F401.py
|
- F401.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()
|
||||||
|
|
@ -913,6 +913,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::ExceptionWithoutExcInfo) {
|
if checker.enabled(Rule::ExceptionWithoutExcInfo) {
|
||||||
flake8_logging::rules::exception_without_exc_info(checker, call);
|
flake8_logging::rules::exception_without_exc_info(checker, call);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::ImplicitCwd) {
|
||||||
|
refurb::rules::no_implicit_cwd(checker, call);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Expr::Dict(
|
Expr::Dict(
|
||||||
dict @ ast::ExprDict {
|
dict @ ast::ExprDict {
|
||||||
|
|
|
||||||
|
|
@ -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, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap),
|
||||||
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
|
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
|
||||||
(Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
|
(Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
|
||||||
|
(Refurb, "177") => (RuleGroup::Preview, rules::refurb::rules::ImplicitCwd),
|
||||||
|
|
||||||
// flake8-logging
|
// flake8-logging
|
||||||
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
|
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,7 @@ use ruff_macros::CacheKey;
|
||||||
use std::fmt::{Debug, Formatter};
|
use std::fmt::{Debug, Formatter};
|
||||||
use std::iter::FusedIterator;
|
use std::iter::FusedIterator;
|
||||||
|
|
||||||
const RULESET_SIZE: usize = 11;
|
const RULESET_SIZE: usize = 12;
|
||||||
|
|
||||||
/// A set of [`Rule`]s.
|
/// A set of [`Rule`]s.
|
||||||
///
|
///
|
||||||
|
|
|
||||||
|
|
@ -21,6 +21,7 @@ mod tests {
|
||||||
#[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
|
#[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
|
||||||
#[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
|
#[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
|
||||||
#[test_case(Rule::PrintEmptyString, Path::new("FURB105.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<()> {
|
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
||||||
|
|
@ -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()));
|
||||||
|
}
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
pub(crate) use check_and_remove_from_set::*;
|
pub(crate) use check_and_remove_from_set::*;
|
||||||
pub(crate) use delete_full_slice::*;
|
pub(crate) use delete_full_slice::*;
|
||||||
|
pub(crate) use implicit_cwd::*;
|
||||||
pub(crate) use print_empty_string::*;
|
pub(crate) use print_empty_string::*;
|
||||||
pub(crate) use reimplemented_starmap::*;
|
pub(crate) use reimplemented_starmap::*;
|
||||||
pub(crate) use repeated_append::*;
|
pub(crate) use repeated_append::*;
|
||||||
|
|
@ -8,6 +9,7 @@ pub(crate) use unnecessary_enumerate::*;
|
||||||
|
|
||||||
mod check_and_remove_from_set;
|
mod check_and_remove_from_set;
|
||||||
mod delete_full_slice;
|
mod delete_full_slice;
|
||||||
|
mod implicit_cwd;
|
||||||
mod print_empty_string;
|
mod print_empty_string;
|
||||||
mod reimplemented_starmap;
|
mod reimplemented_starmap;
|
||||||
mod repeated_append;
|
mod repeated_append;
|
||||||
|
|
|
||||||
|
|
@ -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
|
||||||
|
|
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -2676,6 +2676,8 @@
|
||||||
"FURB140",
|
"FURB140",
|
||||||
"FURB145",
|
"FURB145",
|
||||||
"FURB148",
|
"FURB148",
|
||||||
|
"FURB17",
|
||||||
|
"FURB177",
|
||||||
"G",
|
"G",
|
||||||
"G0",
|
"G0",
|
||||||
"G00",
|
"G00",
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue