Skip to content

Commit 450fb9b

Browse files
[flake8-logging] Implement LOG001: direct-logger-instantiation (#7397)
## Summary See: #7248.
1 parent 6163c99 commit 450fb9b

File tree

11 files changed

+135
-2
lines changed

11 files changed

+135
-2
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import logging
2+
3+
logging.Logger(__name__)
4+
logging.Logger()
5+
logging.getLogger(__name__)

crates/ruff/src/checkers/ast/analyze/expression.rs

+3
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
892892
if checker.enabled(Rule::QuadraticListSummation) {
893893
ruff::rules::quadratic_list_summation(checker, call);
894894
}
895+
if checker.enabled(Rule::DirectLoggerInstantiation) {
896+
flake8_logging::rules::direct_logger_instantiation(checker, call);
897+
}
895898
}
896899
Expr::Dict(ast::ExprDict {
897900
keys,

crates/ruff/src/codes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
921921
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
922922

923923
// flake8-logging
924+
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
924925
(Flake8Logging, "009") => (RuleGroup::Preview, rules::flake8_logging::rules::UndocumentedWarn),
925926

926927
_ => return None,

crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ impl Violation for MutableArgumentDefault {
7070
fn message(&self) -> String {
7171
format!("Do not use mutable data structures for argument defaults")
7272
}
73+
7374
fn autofix_title(&self) -> Option<String> {
7475
Some(format!("Replace with `None`; initialize within function"))
7576
}

crates/ruff/src/rules/flake8_logging/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ mod tests {
1313
use crate::settings::Settings;
1414
use crate::test::test_path;
1515

16+
#[test_case(Rule::DirectLoggerInstantiation, Path::new("LOG001.py"))]
1617
#[test_case(Rule::UndocumentedWarn, Path::new("LOG009.py"))]
1718
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
1819
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
2+
use ruff_macros::{derive_message_formats, violation};
3+
use ruff_python_ast as ast;
4+
use ruff_text_size::Ranged;
5+
6+
use crate::checkers::ast::Checker;
7+
use crate::importer::ImportRequest;
8+
use crate::registry::AsRule;
9+
10+
/// ## What it does
11+
/// Checks for direct instantiation of `logging.Logger`, as opposed to using
12+
/// `logging.getLogger()`.
13+
///
14+
/// ## Why is this bad?
15+
/// The [Logger Objects] documentation states that:
16+
///
17+
/// > Note that Loggers should NEVER be instantiated directly, but always
18+
/// > through the module-level function `logging.getLogger(name)`.
19+
///
20+
/// If a logger is directly instantiated, it won't be added to the logger
21+
/// tree, and will bypass all configuration. Messages logged to it will
22+
/// only be sent to the "handler of last resort", skipping any filtering
23+
/// or formatting.
24+
///
25+
/// ## Example
26+
/// ```python
27+
/// import logging
28+
///
29+
/// logger = logging.Logger(__name__)
30+
/// ```
31+
///
32+
/// Use instead:
33+
/// ```python
34+
/// import logging
35+
///
36+
/// logger = logging.getLogger(__name__)
37+
/// ```
38+
///
39+
/// [Logger Objects]: https://docs.python.org/3/library/logging.html#logger-objects
40+
#[violation]
41+
pub struct DirectLoggerInstantiation;
42+
43+
impl Violation for DirectLoggerInstantiation {
44+
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
45+
46+
#[derive_message_formats]
47+
fn message(&self) -> String {
48+
format!("Use `logging.getLogger()` to instantiate loggers")
49+
}
50+
51+
fn autofix_title(&self) -> Option<String> {
52+
Some(format!("Replace with `logging.getLogger()`"))
53+
}
54+
}
55+
56+
/// LOG001
57+
pub(crate) fn direct_logger_instantiation(checker: &mut Checker, call: &ast::ExprCall) {
58+
if checker
59+
.semantic()
60+
.resolve_call_path(call.func.as_ref())
61+
.is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "Logger"]))
62+
{
63+
let mut diagnostic = Diagnostic::new(DirectLoggerInstantiation, call.func.range());
64+
if checker.patch(diagnostic.kind.rule()) {
65+
diagnostic.try_set_fix(|| {
66+
let (import_edit, binding) = checker.importer().get_or_import_symbol(
67+
&ImportRequest::import("logging", "getLogger"),
68+
call.func.start(),
69+
checker.semantic(),
70+
)?;
71+
let reference_edit = Edit::range_replacement(binding, call.func.range());
72+
Ok(Fix::suggested_edits(import_edit, [reference_edit]))
73+
});
74+
}
75+
checker.diagnostics.push(diagnostic);
76+
}
77+
}
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
pub(crate) use direct_logger_instantiation::*;
12
pub(crate) use undocumented_warn::*;
23

4+
mod direct_logger_instantiation;
35
mod undocumented_warn;

crates/ruff/src/rules/flake8_logging/rules/undocumented_warn.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ pub(crate) fn undocumented_warn(checker: &mut Checker, expr: &Expr) {
5959
diagnostic.try_set_fix(|| {
6060
let (import_edit, binding) = checker.importer().get_or_import_symbol(
6161
&ImportRequest::import("logging", "WARNING"),
62-
expr.range().start(),
62+
expr.start(),
6363
checker.semantic(),
6464
)?;
6565
let reference_edit = Edit::range_replacement(binding, expr.range());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
source: crates/ruff/src/rules/flake8_logging/mod.rs
3+
---
4+
LOG001.py:3:1: LOG001 [*] Use `logging.getLogger()` to instantiate loggers
5+
|
6+
1 | import logging
7+
2 |
8+
3 | logging.Logger(__name__)
9+
| ^^^^^^^^^^^^^^ LOG001
10+
4 | logging.Logger()
11+
5 | logging.getLogger(__name__)
12+
|
13+
= help: Replace with `logging.getLogger()`
14+
15+
Suggested fix
16+
1 1 | import logging
17+
2 2 |
18+
3 |-logging.Logger(__name__)
19+
3 |+logging.getLogger(__name__)
20+
4 4 | logging.Logger()
21+
5 5 | logging.getLogger(__name__)
22+
23+
LOG001.py:4:1: LOG001 [*] Use `logging.getLogger()` to instantiate loggers
24+
|
25+
3 | logging.Logger(__name__)
26+
4 | logging.Logger()
27+
| ^^^^^^^^^^^^^^ LOG001
28+
5 | logging.getLogger(__name__)
29+
|
30+
= help: Replace with `logging.getLogger()`
31+
32+
Suggested fix
33+
1 1 | import logging
34+
2 2 |
35+
3 3 | logging.Logger(__name__)
36+
4 |-logging.Logger()
37+
4 |+logging.getLogger()
38+
5 5 | logging.getLogger(__name__)
39+
40+

crates/ruff_workspace/src/configuration.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -853,9 +853,11 @@ mod tests {
853853
];
854854

855855
const PREVIEW_RULES: &[Rule] = &[
856+
Rule::DirectLoggerInstantiation,
856857
Rule::ManualDictComprehension,
857-
Rule::TooManyPublicMethods,
858858
Rule::SliceCopy,
859+
Rule::TooManyPublicMethods,
860+
Rule::TooManyPublicMethods,
859861
Rule::UndocumentedWarn,
860862
];
861863

ruff.schema.json

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)