Skip to content

Commit

Permalink
[flake8-logging] Implement LOG001: direct-logger-instantiation (#…
Browse files Browse the repository at this point in the history
…7397)

## Summary

See: #7248.
  • Loading branch information
charliermarsh authored Sep 15, 2023
1 parent 6163c99 commit 450fb9b
Show file tree
Hide file tree
Showing 11 changed files with 135 additions and 2 deletions.
5 changes: 5 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_logging/LOG001.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import logging

logging.Logger(__name__)
logging.Logger()
logging.getLogger(__name__)
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::QuadraticListSummation) {
ruff::rules::quadratic_list_summation(checker, call);
}
if checker.enabled(Rule::DirectLoggerInstantiation) {
flake8_logging::rules::direct_logger_instantiation(checker, call);
}
}
Expr::Dict(ast::ExprDict {
keys,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),

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

_ => return None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl Violation for MutableArgumentDefault {
fn message(&self) -> String {
format!("Do not use mutable data structures for argument defaults")
}

fn autofix_title(&self) -> Option<String> {
Some(format!("Replace with `None`; initialize within function"))
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_logging/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ mod tests {
use crate::settings::Settings;
use crate::test::test_path;

#[test_case(Rule::DirectLoggerInstantiation, Path::new("LOG001.py"))]
#[test_case(Rule::UndocumentedWarn, Path::new("LOG009.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
use crate::registry::AsRule;

/// ## What it does
/// Checks for direct instantiation of `logging.Logger`, as opposed to using
/// `logging.getLogger()`.
///
/// ## Why is this bad?
/// The [Logger Objects] documentation states that:
///
/// > Note that Loggers should NEVER be instantiated directly, but always
/// > through the module-level function `logging.getLogger(name)`.
///
/// If a logger is directly instantiated, it won't be added to the logger
/// tree, and will bypass all configuration. Messages logged to it will
/// only be sent to the "handler of last resort", skipping any filtering
/// or formatting.
///
/// ## Example
/// ```python
/// import logging
///
/// logger = logging.Logger(__name__)
/// ```
///
/// Use instead:
/// ```python
/// import logging
///
/// logger = logging.getLogger(__name__)
/// ```
///
/// [Logger Objects]: https://docs.python.org/3/library/logging.html#logger-objects
#[violation]
pub struct DirectLoggerInstantiation;

impl Violation for DirectLoggerInstantiation {
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
format!("Use `logging.getLogger()` to instantiate loggers")
}

fn autofix_title(&self) -> Option<String> {
Some(format!("Replace with `logging.getLogger()`"))
}
}

/// LOG001
pub(crate) fn direct_logger_instantiation(checker: &mut Checker, call: &ast::ExprCall) {
if checker
.semantic()
.resolve_call_path(call.func.as_ref())
.is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "Logger"]))
{
let mut diagnostic = Diagnostic::new(DirectLoggerInstantiation, call.func.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import("logging", "getLogger"),
call.func.start(),
checker.semantic(),
)?;
let reference_edit = Edit::range_replacement(binding, call.func.range());
Ok(Fix::suggested_edits(import_edit, [reference_edit]))
});
}
checker.diagnostics.push(diagnostic);
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_logging/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub(crate) use direct_logger_instantiation::*;
pub(crate) use undocumented_warn::*;

mod direct_logger_instantiation;
mod undocumented_warn;
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub(crate) fn undocumented_warn(checker: &mut Checker, expr: &Expr) {
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import("logging", "WARNING"),
expr.range().start(),
expr.start(),
checker.semantic(),
)?;
let reference_edit = Edit::range_replacement(binding, expr.range());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
source: crates/ruff/src/rules/flake8_logging/mod.rs
---
LOG001.py:3:1: LOG001 [*] Use `logging.getLogger()` to instantiate loggers
|
1 | import logging
2 |
3 | logging.Logger(__name__)
| ^^^^^^^^^^^^^^ LOG001
4 | logging.Logger()
5 | logging.getLogger(__name__)
|
= help: Replace with `logging.getLogger()`

Suggested fix
1 1 | import logging
2 2 |
3 |-logging.Logger(__name__)
3 |+logging.getLogger(__name__)
4 4 | logging.Logger()
5 5 | logging.getLogger(__name__)

LOG001.py:4:1: LOG001 [*] Use `logging.getLogger()` to instantiate loggers
|
3 | logging.Logger(__name__)
4 | logging.Logger()
| ^^^^^^^^^^^^^^ LOG001
5 | logging.getLogger(__name__)
|
= help: Replace with `logging.getLogger()`

Suggested fix
1 1 | import logging
2 2 |
3 3 | logging.Logger(__name__)
4 |-logging.Logger()
4 |+logging.getLogger()
5 5 | logging.getLogger(__name__)


4 changes: 3 additions & 1 deletion crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,9 +853,11 @@ mod tests {
];

const PREVIEW_RULES: &[Rule] = &[
Rule::DirectLoggerInstantiation,
Rule::ManualDictComprehension,
Rule::TooManyPublicMethods,
Rule::SliceCopy,
Rule::TooManyPublicMethods,
Rule::TooManyPublicMethods,
Rule::UndocumentedWarn,
];

Expand Down
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 450fb9b

Please sign in to comment.