[flake8-logging] Root logger calls (LOG015) (astral-sh#14302)

crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG015.py

@@ -0,0 +1,59 @@
+# All errors
+import logging
+
+logging.debug("Lorem")
+logging.info("ipsum")
+logging.warn("dolor")
+logging.warning("sit")
+logging.error("amet")
+logging.critical("consectetur")
+logging.log("adipiscing")
+logging.exception("elit.")
+
+
+# All errors
+from logging import (
+    debug,
+    info,
+    warn,
+    warning,
+    error,
+    critical,
+    log,
+    exception
+)
+
+debug("Lorem")
+info("ipsum")
+warn("dolor")
+warning("sit")
+error("amet")
+critical("consectetur")
+log("adipiscing")
+exception("elit.")
+
+
+# No errors
+logger = logging.getLogger("")
+
+logger.debug("Lorem")
+logger.info("ipsum")
+logger.warn("dolor")
+logger.warning("sit")
+logger.error("amet")
+logger.critical("consectetur")
+logger.log("adipiscing")
+logger.exception("elit.")
+
+
+# No errors
+logging = logging.getLogger("")
+
+logging.debug("Lorem")
+logging.info("ipsum")
+logging.warn("dolor")
+logging.warning("sit")
+logging.error("amet")
+logging.critical("consectetur")
+logging.log("adipiscing")
+logging.exception("elit.")

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

@@ -994,6 +994,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::ExceptionWithoutExcInfo) {
                 flake8_logging::rules::exception_without_exc_info(checker, call);
             }
+            if checker.enabled(Rule::RootLoggerCall) {
+                flake8_logging::rules::root_logger_call(checker, call);
+            }
             if checker.enabled(Rule::IsinstanceTypeNone) {
                 refurb::rules::isinstance_type_none(checker, call);
             }

crates/ruff_linter/src/codes.rs

@@ -1083,6 +1083,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Logging, "002") => (RuleGroup::Stable, rules::flake8_logging::rules::InvalidGetLoggerArgument),
         (Flake8Logging, "007") => (RuleGroup::Stable, rules::flake8_logging::rules::ExceptionWithoutExcInfo),
         (Flake8Logging, "009") => (RuleGroup::Stable, rules::flake8_logging::rules::UndocumentedWarn),
+        (Flake8Logging, "015") => (RuleGroup::Preview, rules::flake8_logging::rules::RootLoggerCall),
 
         _ => return None,
     })

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

@@ -10,6 +10,7 @@ mod tests {
 
     use crate::assert_messages;
     use crate::registry::Rule;
+    use crate::settings::types::PreviewMode;
     use crate::settings::LinterSettings;
     use crate::test::test_path;
 
@@ -26,4 +27,18 @@ mod tests {
         assert_messages!(snapshot, diagnostics);
         Ok(())
     }
+
+    #[test_case(Rule::RootLoggerCall, Path::new("LOG015.py"))]
+    fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
+        let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
+        let diagnostics = test_path(
+            Path::new("flake8_logging").join(path).as_path(),
+            &LinterSettings {
+                preview: PreviewMode::Enabled,
+                ..LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_messages!(snapshot, diagnostics);
+        Ok(())
+    }
 }

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

@@ -1,9 +1,11 @@
 pub(crate) use direct_logger_instantiation::*;
 pub(crate) use exception_without_exc_info::*;
 pub(crate) use invalid_get_logger_argument::*;
+pub(crate) use root_logger_call::*;
 pub(crate) use undocumented_warn::*;
 
 mod direct_logger_instantiation;
 mod exception_without_exc_info;
 mod invalid_get_logger_argument;
+mod root_logger_call;
 mod undocumented_warn;

crates/ruff_linter/src/rules/flake8_logging/rules/root_logger_call.rs

@@ -0,0 +1,77 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::ExprCall;
+use ruff_python_semantic::Modules;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for usages of the following `logging` top-level functions:
+/// `debug`, `info`, `warn`, `warning`, `error`, `critical`, `log`, `exception`.
+///
+/// ## Why is this bad?
+/// Using the root logger causes the messages to have no source information,
+/// making them less useful for debugging.
+///
+/// ## Example
+/// ```python
+/// import logging
+///
+/// logging.info("Foobar")
+/// ```
+///
+/// Use instead:
+/// ```python
+/// import logging
+///
+/// logger = logging.getLogger(__file__)
+/// logger.info("Foobar")
+/// ```
+#[violation]
+pub struct RootLoggerCall {
+    attr: String,
+}
+
+impl Violation for RootLoggerCall {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("`{}()` call on root logger", self.attr)
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Use own logger instead".to_string())
+    }
+}
+
+/// LOG015
+pub(crate) fn root_logger_call(checker: &mut Checker, call: &ExprCall) {
+    let semantic = checker.semantic();
+
+    if !semantic.seen_module(Modules::LOGGING) {
+        return;
+    }
+
+    let Some(qualified_name) = semantic.resolve_qualified_name(&call.func) else {
+        return;
+    };
+
+    let attr = match qualified_name.segments() {
+        ["logging", attr] if is_logger_method_name(attr) => attr,
+        _ => return,
+    };
+
+    let kind = RootLoggerCall {
+        attr: (*attr).to_string(),
+    };
+    let diagnostic = Diagnostic::new(kind, call.range);
+
+    checker.diagnostics.push(diagnostic);
+}
+
+#[inline]
+fn is_logger_method_name(attr: &str) -> bool {
+    matches!(
+        attr,
+        "debug" | "info" | "warn" | "warning" | "error" | "critical" | "log" | "exception"
+    )
+}

crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG015_LOG015.py.snap

@@ -0,0 +1,171 @@
+---
+source: crates/ruff_linter/src/rules/flake8_logging/mod.rs
+snapshot_kind: text
+---
+LOG015.py:4:1: LOG015 `debug()` call on root logger
+  |
+2 | import logging
+3 | 
+4 | logging.debug("Lorem")
+  | ^^^^^^^^^^^^^^^^^^^^^^ LOG015
+5 | logging.info("ipsum")
+6 | logging.warn("dolor")
+  |
+  = help: Use own logger instead
+
+LOG015.py:5:1: LOG015 `info()` call on root logger
+  |
+4 | logging.debug("Lorem")
+5 | logging.info("ipsum")
+  | ^^^^^^^^^^^^^^^^^^^^^ LOG015
+6 | logging.warn("dolor")
+7 | logging.warning("sit")
+  |
+  = help: Use own logger instead
+
+LOG015.py:6:1: LOG015 `warn()` call on root logger
+  |
+4 | logging.debug("Lorem")
+5 | logging.info("ipsum")
+6 | logging.warn("dolor")
+  | ^^^^^^^^^^^^^^^^^^^^^ LOG015
+7 | logging.warning("sit")
+8 | logging.error("amet")
+  |
+  = help: Use own logger instead
+
+LOG015.py:7:1: LOG015 `warning()` call on root logger
+  |
+5 | logging.info("ipsum")
+6 | logging.warn("dolor")
+7 | logging.warning("sit")
+  | ^^^^^^^^^^^^^^^^^^^^^^ LOG015
+8 | logging.error("amet")
+9 | logging.critical("consectetur")
+  |
+  = help: Use own logger instead
+
+LOG015.py:8:1: LOG015 `error()` call on root logger
+   |
+ 6 | logging.warn("dolor")
+ 7 | logging.warning("sit")
+ 8 | logging.error("amet")
+   | ^^^^^^^^^^^^^^^^^^^^^ LOG015
+ 9 | logging.critical("consectetur")
+10 | logging.log("adipiscing")
+   |
+   = help: Use own logger instead
+
+LOG015.py:9:1: LOG015 `critical()` call on root logger
+   |
+ 7 | logging.warning("sit")
+ 8 | logging.error("amet")
+ 9 | logging.critical("consectetur")
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG015
+10 | logging.log("adipiscing")
+11 | logging.exception("elit.")
+   |
+   = help: Use own logger instead
+
+LOG015.py:10:1: LOG015 `log()` call on root logger
+   |
+ 8 | logging.error("amet")
+ 9 | logging.critical("consectetur")
+10 | logging.log("adipiscing")
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^ LOG015
+11 | logging.exception("elit.")
+   |
+   = help: Use own logger instead
+
+LOG015.py:11:1: LOG015 `exception()` call on root logger
+   |
+ 9 | logging.critical("consectetur")
+10 | logging.log("adipiscing")
+11 | logging.exception("elit.")
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG015
+   |
+   = help: Use own logger instead
+
+LOG015.py:26:1: LOG015 `debug()` call on root logger
+   |
+24 | )
+25 | 
+26 | debug("Lorem")
+   | ^^^^^^^^^^^^^^ LOG015
+27 | info("ipsum")
+28 | warn("dolor")
+   |
+   = help: Use own logger instead
+
+LOG015.py:27:1: LOG015 `info()` call on root logger
+   |
+26 | debug("Lorem")
+27 | info("ipsum")
+   | ^^^^^^^^^^^^^ LOG015
+28 | warn("dolor")
+29 | warning("sit")
+   |
+   = help: Use own logger instead
+
+LOG015.py:28:1: LOG015 `warn()` call on root logger
+   |
+26 | debug("Lorem")
+27 | info("ipsum")
+28 | warn("dolor")
+   | ^^^^^^^^^^^^^ LOG015
+29 | warning("sit")
+30 | error("amet")
+   |
+   = help: Use own logger instead
+
+LOG015.py:29:1: LOG015 `warning()` call on root logger
+   |
+27 | info("ipsum")
+28 | warn("dolor")
+29 | warning("sit")
+   | ^^^^^^^^^^^^^^ LOG015
+30 | error("amet")
+31 | critical("consectetur")
+   |
+   = help: Use own logger instead
+
+LOG015.py:30:1: LOG015 `error()` call on root logger
+   |
+28 | warn("dolor")
+29 | warning("sit")
+30 | error("amet")
+   | ^^^^^^^^^^^^^ LOG015
+31 | critical("consectetur")
+32 | log("adipiscing")
+   |
+   = help: Use own logger instead
+
+LOG015.py:31:1: LOG015 `critical()` call on root logger
+   |
+29 | warning("sit")
+30 | error("amet")
+31 | critical("consectetur")
+   | ^^^^^^^^^^^^^^^^^^^^^^^ LOG015
+32 | log("adipiscing")
+33 | exception("elit.")
+   |
+   = help: Use own logger instead
+
+LOG015.py:32:1: LOG015 `log()` call on root logger
+   |
+30 | error("amet")
+31 | critical("consectetur")
+32 | log("adipiscing")
+   | ^^^^^^^^^^^^^^^^^ LOG015
+33 | exception("elit.")
+   |
+   = help: Use own logger instead
+
+LOG015.py:33:1: LOG015 `exception()` call on root logger
+   |
+31 | critical("consectetur")
+32 | log("adipiscing")
+33 | exception("elit.")
+   | ^^^^^^^^^^^^^^^^^^ LOG015
+   |
+   = help: Use own logger instead