diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG004.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG004.py new file mode 100644 index 00000000000000..93a67dba3ead3e --- /dev/null +++ b/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 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 6cdc7725c62e72..9c2bae1b9a946a 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/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); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index be27d11fc6b1d9..89d2df4a90d7c8 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/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), diff --git a/crates/ruff_linter/src/rules/flake8_logging/mod.rs b/crates/ruff_linter/src/rules/flake8_logging/mod.rs index 1870a26e631aa2..6ea8099a61f133 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/mod.rs +++ b/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<()> { diff --git a/crates/ruff_linter/src/rules/flake8_logging/rules/exception_outside_except.rs b/crates/ruff_linter/src/rules/flake8_logging/rules/exception_outside_except.rs new file mode 100644 index 00000000000000..39c1f18f25ab5a --- /dev/null +++ b/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 { + 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); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_logging/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_logging/rules/mod.rs index 6a1756de94ce1b..61a3e30a9ba00c 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/rules/mod.rs +++ b/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; diff --git a/crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG004_LOG004.py.snap b/crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG004_LOG004.py.snap new file mode 100644 index 00000000000000..35ce191fc5a9cb --- /dev/null +++ b/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 diff --git a/ruff.schema.json b/ruff.schema.json index b1e24b47760305..e956904513d10c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3304,6 +3304,7 @@ "LOG00", "LOG001", "LOG002", + "LOG004", "LOG007", "LOG009", "N",