[flake8-logging] Implement LOG002: invalid-get-logger-argument

crates/ruff/resources/test/fixtures/flake8_logging/LOG002.py

@@ -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__)

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

@@ -895,6 +895,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::DirectLoggerInstantiation) {
                 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 {
             keys,

crates/ruff/src/codes.rs

@@ -922,6 +922,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
 
         // flake8-logging
         (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),
 
         _ => return None,

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

@@ -14,6 +14,7 @@ mod tests {
     use crate::test::test_path;
 
     #[test_case(Rule::DirectLoggerInstantiation, Path::new("LOG001.py"))]
+    #[test_case(Rule::InvalidGetLoggerArgument, Path::new("LOG002.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());

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

@@ -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);
+}

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

@@ -1,5 +1,7 @@
 pub(crate) use direct_logger_instantiation::*;
+pub(crate) use invalid_get_logger_argument::*;
 pub(crate) use undocumented_warn::*;
 
 mod direct_logger_instantiation;
+mod invalid_get_logger_argument;
 mod undocumented_warn;

crates/ruff/src/rules/flake8_logging/snapshots/ruff__rules__flake8_logging__tests__LOG002_LOG002.py.snap

@@ -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`
+
+

crates/ruff_python_stdlib/src/builtins.rs

@@ -167,6 +167,7 @@ pub const MAGIC_GLOBALS: &[&str] = &[
     "WindowsError",
     "__annotations__",
     "__builtins__",
+    "__cached__",
     "__file__",
 ];
 

crates/ruff_workspace/src/configuration.rs

@@ -859,6 +859,7 @@ mod tests {
         Rule::TooManyPublicMethods,
         Rule::TooManyPublicMethods,
         Rule::UndocumentedWarn,
+        Rule::InvalidGetLoggerArgument,
     ];
 
     #[allow(clippy::needless_pass_by_value)]