[flake8-logging] Implement check for logging.exception() outside …

…exception handler (`LOG004`)

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

@@ -0,0 +1,14 @@
+import logging
+
+logging.exception("foo")  # LOG004
+logging.info("shouldnt_be_found")  # OK
+try:
+    logging.exception("bar")  # LOG004
+    logging.info("baz")  # OK
+    _ = 1 / 0
+except ZeroDivisionError:
+    logging.exception("bar")  # OK
+    logging.info("baz")  # OK
+
+    def handle():
+        logging.exception("qux")  # LOG004

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

@@ -988,6 +988,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::ExceptionOutsideExcept) {
+                flake8_logging::rules::exception_outside_except(checker, call);
+            }
             if checker.enabled(Rule::IsinstanceTypeNone) {
                 refurb::rules::isinstance_type_none(checker, call);
             }

crates/ruff_linter/src/codes.rs

@@ -1078,6 +1078,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         // flake8-logging
         (Flake8Logging, "001") => (RuleGroup::Stable, rules::flake8_logging::rules::DirectLoggerInstantiation),
         (Flake8Logging, "002") => (RuleGroup::Stable, rules::flake8_logging::rules::InvalidGetLoggerArgument),
+        (Flake8Logging, "004") => (RuleGroup::Stable, rules::flake8_logging::rules::ExceptionOutsideExcept),
         (Flake8Logging, "007") => (RuleGroup::Stable, rules::flake8_logging::rules::ExceptionWithoutExcInfo),
         (Flake8Logging, "009") => (RuleGroup::Stable, rules::flake8_logging::rules::UndocumentedWarn),
 

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

@@ -15,6 +15,7 @@ mod tests {
 
     #[test_case(Rule::DirectLoggerInstantiation, Path::new("LOG001.py"))]
     #[test_case(Rule::InvalidGetLoggerArgument, Path::new("LOG002.py"))]
+    #[test_case(Rule::ExceptionOutsideExcept, Path::new("LOG004.py"))]
     #[test_case(Rule::ExceptionWithoutExcInfo, Path::new("LOG007.py"))]
     #[test_case(Rule::UndocumentedWarn, Path::new("LOG009.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {

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

@@ -0,0 +1,92 @@
+use ruff_python_ast::ExprCall;
+
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_semantic::Modules;
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+use crate::importer::ImportRequest;
+
+/// ## What it does
+/// Checks for uses of `logging.exception()` outside of exception handlers.
+///
+/// ## Why is this bad?
+/// Calling `exception()` outside of an exception handler attaches `None`
+/// exception information, leading to confusing messages.
+///
+/// ## Example
+/// ```python
+/// >>> logging.exception("example")
+/// ERROR:root:example
+/// NoneType: None
+/// ```
+///
+/// Use instead:
+/// ```python
+/// >>> logging.error("example")
+/// ERROR:root:example
+/// ```
+#[violation]
+pub struct ExceptionOutsideExcept;
+
+impl Violation for ExceptionOutsideExcept {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Always;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "Use of `logging.exception` outside exception handler".to_string()
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace `logging.exception` with `logging.error`".to_string())
+    }
+}
+
+/// LOG004
+pub(crate) fn exception_outside_except(checker: &mut Checker, expr: &ExprCall) {
+    if !checker.semantic().seen_module(Modules::LOGGING) {
+        return;
+    }
+
+    let parents = checker.semantic().current_statements();
+    let mut acceptable_position = false;
+    for parent in parents {
+        if let ruff_python_ast::Stmt::Try(stmt_try) = parent {
+            for handler in &stmt_try.handlers {
+                if handler.range().contains_range(expr.range()) {
+                    acceptable_position = true;
+                    break;
+                }
+            }
+        } else if let ruff_python_ast::Stmt::FunctionDef(_) = parent {
+            acceptable_position = false;
+            break;
+        }
+        if acceptable_position {
+            break;
+        }
+    }
+
+    if acceptable_position {
+        return;
+    }
+
+    if checker
+        .semantic()
+        .resolve_qualified_name(&expr.func)
+        .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["logging", "exception"]))
+    {
+        let mut diagnostic = Diagnostic::new(ExceptionOutsideExcept, expr.func.range());
+        diagnostic.try_set_fix(|| {
+            let (import_edit, binding) = checker.importer().get_or_import_symbol(
+                &ImportRequest::import("logging", "error"),
+                expr.start(),
+                checker.semantic(),
+            )?;
+            let reference_edit = Edit::range_replacement(binding, expr.func.range());
+            Ok(Fix::safe_edits(import_edit, [reference_edit]))
+        });
+        checker.diagnostics.push(diagnostic);
+    }
+}

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

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

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

@@ -0,0 +1,59 @@
+---
+source: crates/ruff_linter/src/rules/flake8_logging/mod.rs
+snapshot_kind: text
+---
+LOG004.py:3:1: LOG004 [*] Use of `logging.exception` outside exception handler
+  |
+1 | import logging
+2 | 
+3 | logging.exception("foo")  # LOG004
+  | ^^^^^^^^^^^^^^^^^ LOG004
+4 | logging.info("shouldnt_be_found")  # OK
+5 | try:
+  |
+  = help: Replace `logging.exception` with `logging.error`
+
+ℹ Safe fix
+1 1 | import logging
+2 2 | 
+3   |-logging.exception("foo")  # LOG004
+  3 |+logging.error("foo")  # LOG004
+4 4 | logging.info("shouldnt_be_found")  # OK
+5 5 | try:
+6 6 |     logging.exception("bar")  # LOG004
+
+LOG004.py:6:5: LOG004 [*] Use of `logging.exception` outside exception handler
+  |
+4 | logging.info("shouldnt_be_found")  # OK
+5 | try:
+6 |     logging.exception("bar")  # LOG004
+  |     ^^^^^^^^^^^^^^^^^ LOG004
+7 |     logging.info("baz")  # OK
+8 |     _ = 1 / 0
+  |
+  = help: Replace `logging.exception` with `logging.error`
+
+ℹ Safe fix
+3 3 | logging.exception("foo")  # LOG004
+4 4 | logging.info("shouldnt_be_found")  # OK
+5 5 | try:
+6   |-    logging.exception("bar")  # LOG004
+  6 |+    logging.error("bar")  # LOG004
+7 7 |     logging.info("baz")  # OK
+8 8 |     _ = 1 / 0
+9 9 | except ZeroDivisionError:
+
+LOG004.py:14:9: LOG004 [*] Use of `logging.exception` outside exception handler
+   |
+13 |     def handle():
+14 |         logging.exception("qux")  # LOG004
+   |         ^^^^^^^^^^^^^^^^^ LOG004
+   |
+   = help: Replace `logging.exception` with `logging.error`
+
+ℹ Safe fix
+11 11 |     logging.info("baz")  # OK
+12 12 | 
+13 13 |     def handle():
+14    |-        logging.exception("qux")  # LOG004
+   14 |+        logging.error("qux")  # LOG004