[flake8-logging] Add ExcInfoOutsideExceptionHandler (LOG014)

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

@@ -0,0 +1,154 @@
+import logging
+
+# Logging ----------------------------------------------------------------------
+
+logging.exception("foo")  # OK
+logging.exception("foo", exc_info=False)  # OK
+logging.exception("foo", exc_info=[])  # OK
+logging.exception("foo", exc_info=True)  # LOG014
+logging.exception("foo", exc_info=[1])  # LOG014
+
+logging.error("foo", exc_info=False)  # OK
+logging.error("foo", exc_info=True)  # LOG014
+
+logging.info("foo", exc_info=False)  # OK
+logging.info("foo", exc_info=True)  # LOG014
+
+try:
+    a = 1 * 2
+except Exception:
+    logging.exception("foo")  # OK
+    logging.exception("foo", exc_info=False)  # OK
+    logging.exception("foo", exc_info=[])  # OK
+    logging.exception("foo", exc_info=True)  # OK
+    logging.exception("foo", exc_info=[1])  # OK
+
+    logging.error("foo", exc_info=False)  # OK
+    logging.error("foo", exc_info=True)  # OK
+
+    logging.info("foo", exc_info=False)  # OK
+    logging.info("foo", exc_info=True)  # OK
+
+# Logger -----------------------------------------------------------------------
+
+logger = logging.getLogger(__name__)
+
+logger.exception("foo")  # OK
+logger.exception("foo", exc_info=False)  # OK
+logger.exception("foo", exc_info=[])  # OK
+logger.exception("foo", exc_info=True)  # LOG014
+logger.exception("foo", exc_info=[1])  # LOG014
+
+logger.error("foo", exc_info=False)  # OK
+logger.error("foo", exc_info=True)  # LOG014
+
+logger.info("foo", exc_info=False)  # OK
+logger.info("foo", exc_info=True)  # LOG014
+
+try:
+    a = 1 * 2
+except Exception:
+    logger.exception("foo")  # OK
+    logger.exception("foo", exc_info=False)  # OK
+    logger.exception("foo", exc_info=[])  # OK
+    logger.exception("foo", exc_info=True)  # OK
+    logger.exception("foo", exc_info=[1])  # OK
+
+    logger.error("foo", exc_info=False)  # OK
+    logger.error("foo", exc_info=True)  # OK
+
+    logger.info("foo", exc_info=False)  # OK
+    logger.info("foo", exc_info=True)  # OK
+
+# Direct Call ------------------------------------------------------------------
+
+from logging import exception, error, info
+
+exception("foo")  # OK
+exception("foo", exc_info=False)  # OK
+exception("foo", exc_info=[])  # OK
+exception("foo", exc_info=True)  # LOG014
+exception("foo", exc_info=[1])  # LOG014
+
+error("foo", exc_info=False)  # OK
+error("foo", exc_info=True)  # LOG014
+
+info("foo", exc_info=False)  # OK
+info("foo", exc_info=True)  # LOG014
+
+try:
+    a = 1 * 2
+except Exception:
+    exception("foo")  # OK
+    exception("foo", exc_info=False)  # OK
+    exception("foo", exc_info=[])  # OK
+    exception("foo", exc_info=True)  # OK
+    exception("foo", exc_info=[1])  # OK
+
+    error("foo", exc_info=False)  # OK
+    error("foo", exc_info=True)  # OK
+
+    info("foo", exc_info=False)  # OK
+    info("foo", exc_info=True)  # OK
+
+# Fake Call --------------------------------------------------------------------
+
+def wrapper():
+    exception = lambda *args, **kwargs: None
+    error = lambda *args, **kwargs: None
+    info = lambda *args, **kwargs: None
+
+    exception("foo")  # OK
+    exception("foo", exc_info=False)  # OK
+    exception("foo", exc_info=[])  # OK
+    exception("foo", exc_info=True)  # OK
+    exception("foo", exc_info=[1])  # OK
+
+    error("foo", exc_info=False)  # OK
+    error("foo", exc_info=True)  # OK
+
+    info("foo", exc_info=False)  # OK
+    info("foo", exc_info=True)  # OK
+
+    try:
+        a = 1 * 2
+    except Exception:
+        exception("foo")  # OK
+        exception("foo", exc_info=False)  # OK
+        exception("foo", exc_info=[])  # OK
+        exception("foo", exc_info=True)  # OK
+        exception("foo", exc_info=[1])  # OK
+
+        error("foo", exc_info=False)  # OK
+        error("foo", exc_info=True)  # OK
+
+        info("foo", exc_info=False)  # OK
+        info("foo", exc_info=True)  # OK
+
+# Nested -----------------------------------------------------------------------
+
+def apple_1():
+    try:
+        logging.exception("foo", exc_info=True)  # LOG014
+    except Exception:
+        pass
+
+
+def apple_2():
+    def banana():
+        logging.exception("foo", exc_info=True)  # LOG014
+
+    try:
+        banana()
+    except Exception:
+        pass
+
+
+def apple_3():
+    def banana():
+        logging.exception("foo", exc_info=True)  # LOG014
+
+    try:
+        a = 1 * 2
+    except Exception:
+        banana()

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

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

crates/ruff_linter/src/codes.rs

@@ -1101,6 +1101,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, "014") => (RuleGroup::Preview, rules::flake8_logging::rules::ExcInfoOutsideExceptionHandler),
         (Flake8Logging, "015") => (RuleGroup::Preview, rules::flake8_logging::rules::RootLoggerCall),
 
         _ => return None,

crates/ruff_linter/src/registry/rule_set.rs

@@ -5,7 +5,7 @@ use ruff_macros::CacheKey;
 
 use crate::registry::Rule;
 
-const RULESET_SIZE: usize = 14;
+const RULESET_SIZE: usize = 15;
 
 /// A set of [`Rule`]s.
 ///

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

@@ -0,0 +1,30 @@
+use crate::checkers::ast::Checker;
+use ruff_python_ast::helpers::Truthiness;
+use ruff_python_ast::ExprCall;
+
+fn exc_info_arg_is_(checker: &mut Checker, call: &ExprCall, boolean: bool) -> bool {
+    call.arguments
+        .find_keyword("exc_info")
+        .map(|keyword| &keyword.value)
+        .is_some_and(|value| {
+            let truthiness =
+                Truthiness::from_expr(value, |id| checker.semantic().has_builtin_binding(id));
+            truthiness.into_bool() == Some(boolean)
+        })
+}
+
+pub(super) fn exc_info_arg_is_falsey(checker: &mut Checker, call: &ExprCall) -> bool {
+    exc_info_arg_is_(checker, call, false)
+}
+
+pub(super) fn exc_info_arg_is_truey(checker: &mut Checker, call: &ExprCall) -> bool {
+    exc_info_arg_is_(checker, call, true)
+}
+
+#[inline]
+pub(super) 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/mod.rs

@@ -1,4 +1,5 @@
 //! Rules from [flake8-logging](https://pypi.org/project/flake8-logging/).
+mod helpers;
 pub(crate) mod rules;
 
 #[cfg(test)]
@@ -28,6 +29,7 @@ mod tests {
         Ok(())
     }
 
+    #[test_case(Rule::ExcInfoOutsideExceptionHandler, Path::new("LOG014.py"))]
     #[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());

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

@@ -0,0 +1,102 @@
+use crate::checkers::ast::Checker;
+use crate::rules::flake8_logging::helpers;
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::{self as ast, Expr, ExprCall, Stmt};
+use ruff_python_semantic::analyze::logging;
+use ruff_python_stdlib::logging::LoggingLevel;
+use ruff_text_size::Ranged;
+
+/// ## What it does
+/// Checks for cases where a logging call is made with `exc_info` set to `True`,
+/// outside of an exception handling block.
+///
+/// ## Why is this bad?
+/// When outside of an exception handling block, the variable holding the
+/// exception information will be assigned to `None` as there is no active
+/// exception, which then causes the final line of the logging call to contain
+/// `NoneType: None`, which is meaningless
+///
+/// ## Example
+/// ```python
+/// logging.error("...", exc_info=True)
+/// ```
+///
+/// Either add an exception handling block:
+/// ```python
+/// try:
+///     logging.error("...", exc_info=True)
+/// except ...:
+///     ...
+/// ```
+///
+/// Or don't set `exc_info` to `True`:
+/// logging.error("...")
+#[derive(ViolationMetadata)]
+pub(crate) struct ExcInfoOutsideExceptionHandler;
+
+impl Violation for ExcInfoOutsideExceptionHandler {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "Use of `exc_info` outside an exception handler".to_string()
+    }
+}
+
+/// LOG014
+pub(crate) fn exc_info_outside_exception_handler(checker: &mut Checker, call: &ExprCall) {
+    if is_logging_call(checker, call)
+        && helpers::exc_info_arg_is_truey(checker, call)
+        && currently_outside_exception_handler(checker)
+    {
+        checker.diagnostics.push(Diagnostic::new(
+            ExcInfoOutsideExceptionHandler,
+            call.range(),
+        ));
+    }
+}
+
+fn currently_outside_exception_handler(checker: &mut Checker) -> bool {
+    for parent_stmt in checker.semantic().current_statements() {
+        if let Stmt::Try(_) = parent_stmt {
+            return false;
+        }
+    }
+
+    true
+}
+
+fn is_logging_call(checker: &mut Checker, call: &ExprCall) -> bool {
+    match call.func.as_ref() {
+        Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
+            // Match any logging level
+            if LoggingLevel::from_attribute(attr.as_str()).is_none() {
+                return false;
+            }
+
+            if !logging::is_logger_candidate(
+                &call.func,
+                checker.semantic(),
+                &checker.settings.logger_objects,
+            ) {
+                return false;
+            }
+        }
+        Expr::Name(_) => {
+            if !checker
+                .semantic()
+                .resolve_qualified_name(call.func.as_ref())
+                .is_some_and(|qualified_name| {
+                    matches!(
+                        qualified_name.segments(),
+                        ["logging", attr] if helpers::is_logger_method_name(attr)
+                    )
+                })
+            {
+                return false;
+            }
+        }
+        _ => return false,
+    }
+
+    true
+}

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

@@ -1,13 +1,12 @@
+use crate::checkers::ast::Checker;
+use crate::rules::flake8_logging::helpers;
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
-use ruff_python_ast::helpers::Truthiness;
 use ruff_python_ast::{self as ast, Expr, ExprCall};
 use ruff_python_semantic::analyze::logging;
 use ruff_python_stdlib::logging::LoggingLevel;
 use ruff_text_size::Ranged;
 
-use crate::checkers::ast::Checker;
-
 /// ## What it does
 /// Checks for uses of `logging.exception()` with `exc_info` set to `False`.
 ///
@@ -73,20 +72,9 @@ pub(crate) fn exception_without_exc_info(checker: &mut Checker, call: &ExprCall)
         _ => return,
     }
 
-    if exc_info_arg_is_falsey(call, checker) {
+    if helpers::exc_info_arg_is_falsey(checker, call) {
         checker
             .diagnostics
             .push(Diagnostic::new(ExceptionWithoutExcInfo, call.range()));
     }
 }
-
-fn exc_info_arg_is_falsey(call: &ExprCall, checker: &mut Checker) -> bool {
-    call.arguments
-        .find_keyword("exc_info")
-        .map(|keyword| &keyword.value)
-        .is_some_and(|value| {
-            let truthiness =
-                Truthiness::from_expr(value, |id| checker.semantic().has_builtin_binding(id));
-            truthiness.into_bool() == Some(false)
-        })
-}

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

@@ -1,10 +1,12 @@
 pub(crate) use direct_logger_instantiation::*;
+pub(crate) use exc_info_outside_exception_handler::*;
 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 exc_info_outside_exception_handler;
 mod exception_without_exc_info;
 mod invalid_get_logger_argument;
 mod root_logger_call;