mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-01 09:22:19 +00:00
[flake8-logging
] Implement LOG002
: invalid-get-logger-argument
(#7399)
## Summary This PR implements a new rule for `flake8-logging` plugin that checks for `logging.getLogger` calls with either `__file__` or `__cached__` as the first argument and generates a suggested fix to use `__name__` instead. Refer: #7248 ## Test Plan Add test cases and `cargo test`
This commit is contained in:
parent
c907317199
commit
0d1fb823d6
10 changed files with 208 additions and 0 deletions
24
crates/ruff/resources/test/fixtures/flake8_logging/LOG002.py
vendored
Normal file
24
crates/ruff/resources/test/fixtures/flake8_logging/LOG002.py
vendored
Normal file
|
@ -0,0 +1,24 @@
|
||||||
|
import logging
|
||||||
|
from logging import getLogger
|
||||||
|
|
||||||
|
# Ok
|
||||||
|
logging.getLogger(__name__)
|
||||||
|
logging.getLogger(name=__name__)
|
||||||
|
logging.getLogger("custom")
|
||||||
|
logging.getLogger(name="custom")
|
||||||
|
|
||||||
|
# LOG002
|
||||||
|
getLogger(__file__)
|
||||||
|
logging.getLogger(name=__file__)
|
||||||
|
|
||||||
|
logging.getLogger(__cached__)
|
||||||
|
getLogger(name=__cached__)
|
||||||
|
|
||||||
|
|
||||||
|
# Override `logging.getLogger`
|
||||||
|
class logging:
|
||||||
|
def getLogger(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
logging.getLogger(__file__)
|
|
@ -895,6 +895,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::DirectLoggerInstantiation) {
|
if checker.enabled(Rule::DirectLoggerInstantiation) {
|
||||||
flake8_logging::rules::direct_logger_instantiation(checker, call);
|
flake8_logging::rules::direct_logger_instantiation(checker, call);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::InvalidGetLoggerArgument) {
|
||||||
|
flake8_logging::rules::invalid_get_logger_argument(checker, call);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Expr::Dict(ast::ExprDict {
|
Expr::Dict(ast::ExprDict {
|
||||||
keys,
|
keys,
|
||||||
|
|
|
@ -922,6 +922,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
|
|
||||||
// flake8-logging
|
// flake8-logging
|
||||||
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
|
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
|
||||||
|
(Flake8Logging, "002") => (RuleGroup::Preview, rules::flake8_logging::rules::InvalidGetLoggerArgument),
|
||||||
(Flake8Logging, "009") => (RuleGroup::Preview, rules::flake8_logging::rules::UndocumentedWarn),
|
(Flake8Logging, "009") => (RuleGroup::Preview, rules::flake8_logging::rules::UndocumentedWarn),
|
||||||
|
|
||||||
_ => return None,
|
_ => return None,
|
||||||
|
|
|
@ -14,6 +14,7 @@ mod tests {
|
||||||
use crate::test::test_path;
|
use crate::test::test_path;
|
||||||
|
|
||||||
#[test_case(Rule::DirectLoggerInstantiation, Path::new("LOG001.py"))]
|
#[test_case(Rule::DirectLoggerInstantiation, Path::new("LOG001.py"))]
|
||||||
|
#[test_case(Rule::InvalidGetLoggerArgument, Path::new("LOG002.py"))]
|
||||||
#[test_case(Rule::UndocumentedWarn, Path::new("LOG009.py"))]
|
#[test_case(Rule::UndocumentedWarn, Path::new("LOG009.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());
|
||||||
|
|
|
@ -0,0 +1,92 @@
|
||||||
|
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
|
||||||
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_ast::{self as ast, Expr};
|
||||||
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
use crate::registry::AsRule;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for any usage of `__cached__` and `__file__` as an argument to
|
||||||
|
/// `logging.getLogger()`.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// The [logging documentation] recommends this pattern:
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// logging.getLogger(__name__)
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Here, `__name__` is the fully qualified module name, such as `foo.bar`,
|
||||||
|
/// which is the intended format for logger names.
|
||||||
|
///
|
||||||
|
/// This rule detects probably-mistaken usage of similar module-level dunder constants:
|
||||||
|
///
|
||||||
|
/// * `__cached__` - the pathname of the module's compiled version, such as `foo/__pycache__/bar.cpython-311.pyc`.
|
||||||
|
/// * `__file__` - the pathname of the module, such as `foo/bar.py`.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// import logging
|
||||||
|
///
|
||||||
|
/// logger = logging.getLogger(__file__)
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// import logging
|
||||||
|
///
|
||||||
|
/// logger = logging.getLogger(__name__)
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// [logging documentation]: https://docs.python.org/3/library/logging.html#logger-objects
|
||||||
|
#[violation]
|
||||||
|
pub struct InvalidGetLoggerArgument;
|
||||||
|
|
||||||
|
impl Violation for InvalidGetLoggerArgument {
|
||||||
|
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
|
||||||
|
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
format!("Use `__name__` with `logging.getLogger()`")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn autofix_title(&self) -> Option<String> {
|
||||||
|
Some(format!("Replace with `__name__`"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// LOG002
|
||||||
|
pub(crate) fn invalid_get_logger_argument(checker: &mut Checker, call: &ast::ExprCall) {
|
||||||
|
let Some(Expr::Name(expr @ ast::ExprName { id, .. })) = call.arguments.find_argument("name", 0)
|
||||||
|
else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
if !matches!(id.as_ref(), "__file__" | "__cached__") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if !checker.semantic().is_builtin(id) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if !checker
|
||||||
|
.semantic()
|
||||||
|
.resolve_call_path(call.func.as_ref())
|
||||||
|
.is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "getLogger"]))
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let mut diagnostic = Diagnostic::new(InvalidGetLoggerArgument, expr.range());
|
||||||
|
if checker.patch(diagnostic.kind.rule()) {
|
||||||
|
if checker.semantic().is_builtin("__name__") {
|
||||||
|
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
|
||||||
|
"__name__".to_string(),
|
||||||
|
expr.range(),
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checker.diagnostics.push(diagnostic);
|
||||||
|
}
|
|
@ -1,5 +1,7 @@
|
||||||
pub(crate) use direct_logger_instantiation::*;
|
pub(crate) use direct_logger_instantiation::*;
|
||||||
|
pub(crate) use invalid_get_logger_argument::*;
|
||||||
pub(crate) use undocumented_warn::*;
|
pub(crate) use undocumented_warn::*;
|
||||||
|
|
||||||
mod direct_logger_instantiation;
|
mod direct_logger_instantiation;
|
||||||
|
mod invalid_get_logger_argument;
|
||||||
mod undocumented_warn;
|
mod undocumented_warn;
|
||||||
|
|
|
@ -0,0 +1,82 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_logging/mod.rs
|
||||||
|
---
|
||||||
|
LOG002.py:11:11: LOG002 [*] Use `__name__` with `logging.getLogger()`
|
||||||
|
|
|
||||||
|
10 | # LOG002
|
||||||
|
11 | getLogger(__file__)
|
||||||
|
| ^^^^^^^^ LOG002
|
||||||
|
12 | logging.getLogger(name=__file__)
|
||||||
|
|
|
||||||
|
= help: Replace with `name`
|
||||||
|
|
||||||
|
ℹ Suggested fix
|
||||||
|
8 8 | logging.getLogger(name="custom")
|
||||||
|
9 9 |
|
||||||
|
10 10 | # LOG002
|
||||||
|
11 |-getLogger(__file__)
|
||||||
|
11 |+getLogger(__name__)
|
||||||
|
12 12 | logging.getLogger(name=__file__)
|
||||||
|
13 13 |
|
||||||
|
14 14 | logging.getLogger(__cached__)
|
||||||
|
|
||||||
|
LOG002.py:12:24: LOG002 [*] Use `__name__` with `logging.getLogger()`
|
||||||
|
|
|
||||||
|
10 | # LOG002
|
||||||
|
11 | getLogger(__file__)
|
||||||
|
12 | logging.getLogger(name=__file__)
|
||||||
|
| ^^^^^^^^ LOG002
|
||||||
|
13 |
|
||||||
|
14 | logging.getLogger(__cached__)
|
||||||
|
|
|
||||||
|
= help: Replace with `name`
|
||||||
|
|
||||||
|
ℹ Suggested fix
|
||||||
|
9 9 |
|
||||||
|
10 10 | # LOG002
|
||||||
|
11 11 | getLogger(__file__)
|
||||||
|
12 |-logging.getLogger(name=__file__)
|
||||||
|
12 |+logging.getLogger(name=__name__)
|
||||||
|
13 13 |
|
||||||
|
14 14 | logging.getLogger(__cached__)
|
||||||
|
15 15 | getLogger(name=__cached__)
|
||||||
|
|
||||||
|
LOG002.py:14:19: LOG002 [*] Use `__name__` with `logging.getLogger()`
|
||||||
|
|
|
||||||
|
12 | logging.getLogger(name=__file__)
|
||||||
|
13 |
|
||||||
|
14 | logging.getLogger(__cached__)
|
||||||
|
| ^^^^^^^^^^ LOG002
|
||||||
|
15 | getLogger(name=__cached__)
|
||||||
|
|
|
||||||
|
= help: Replace with `name`
|
||||||
|
|
||||||
|
ℹ Suggested fix
|
||||||
|
11 11 | getLogger(__file__)
|
||||||
|
12 12 | logging.getLogger(name=__file__)
|
||||||
|
13 13 |
|
||||||
|
14 |-logging.getLogger(__cached__)
|
||||||
|
14 |+logging.getLogger(__name__)
|
||||||
|
15 15 | getLogger(name=__cached__)
|
||||||
|
16 16 |
|
||||||
|
17 17 |
|
||||||
|
|
||||||
|
LOG002.py:15:16: LOG002 [*] Use `__name__` with `logging.getLogger()`
|
||||||
|
|
|
||||||
|
14 | logging.getLogger(__cached__)
|
||||||
|
15 | getLogger(name=__cached__)
|
||||||
|
| ^^^^^^^^^^ LOG002
|
||||||
|
|
|
||||||
|
= help: Replace with `name`
|
||||||
|
|
||||||
|
ℹ Suggested fix
|
||||||
|
12 12 | logging.getLogger(name=__file__)
|
||||||
|
13 13 |
|
||||||
|
14 14 | logging.getLogger(__cached__)
|
||||||
|
15 |-getLogger(name=__cached__)
|
||||||
|
15 |+getLogger(name=__name__)
|
||||||
|
16 16 |
|
||||||
|
17 17 |
|
||||||
|
18 18 | # Override `logging.getLogger`
|
||||||
|
|
||||||
|
|
|
@ -167,6 +167,7 @@ pub const MAGIC_GLOBALS: &[&str] = &[
|
||||||
"WindowsError",
|
"WindowsError",
|
||||||
"__annotations__",
|
"__annotations__",
|
||||||
"__builtins__",
|
"__builtins__",
|
||||||
|
"__cached__",
|
||||||
"__file__",
|
"__file__",
|
||||||
];
|
];
|
||||||
|
|
||||||
|
|
|
@ -859,6 +859,7 @@ mod tests {
|
||||||
Rule::TooManyPublicMethods,
|
Rule::TooManyPublicMethods,
|
||||||
Rule::TooManyPublicMethods,
|
Rule::TooManyPublicMethods,
|
||||||
Rule::UndocumentedWarn,
|
Rule::UndocumentedWarn,
|
||||||
|
Rule::InvalidGetLoggerArgument,
|
||||||
];
|
];
|
||||||
|
|
||||||
#[allow(clippy::needless_pass_by_value)]
|
#[allow(clippy::needless_pass_by_value)]
|
||||||
|
|
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
@ -2139,6 +2139,7 @@
|
||||||
"LOG0",
|
"LOG0",
|
||||||
"LOG00",
|
"LOG00",
|
||||||
"LOG001",
|
"LOG001",
|
||||||
|
"LOG002",
|
||||||
"LOG009",
|
"LOG009",
|
||||||
"N",
|
"N",
|
||||||
"N8",
|
"N8",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue