diff --git a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py index 987c321907880..e5b4757691364 100644 --- a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py +++ b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py @@ -109,3 +109,13 @@ def fine(): a = 1 except Exception: error("Context message here", exc_info=sys.exc_info()) + + +def nested(): + try: + a = 1 + except Exception: + try: + b = 2 + except Exception: + error("Context message here") diff --git a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py index 0aa58677837fd..968e0f957b804 100644 --- a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py +++ b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py @@ -126,3 +126,25 @@ def main_function(): except Exception as ex: exception(f"Found an error: {er}") exception(f"Found an error: {ex.field}") + + +def nested(): + try: + process() + handle() + except Exception as ex: + try: + finish() + except Exception as ex: + logger.exception(f"Found an error: {ex}") # TRY401 + + +def nested(): + try: + process() + handle() + except Exception as ex: + try: + finish() + except Exception: + logger.exception(f"Found an error: {ex}") # TRY401 diff --git a/crates/ruff_linter/src/rules/tryceratops/helpers.rs b/crates/ruff_linter/src/rules/tryceratops/helpers.rs index 0e707c1e3924e..bd12ddebfe93b 100644 --- a/crates/ruff_linter/src/rules/tryceratops/helpers.rs +++ b/crates/ruff_linter/src/rules/tryceratops/helpers.rs @@ -1,6 +1,6 @@ use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; -use ruff_python_ast::{self as ast, Expr}; +use ruff_python_ast::{self as ast, ExceptHandler, Expr}; use ruff_python_semantic::analyze::logging; use ruff_python_semantic::SemanticModel; use ruff_python_stdlib::logging::LoggingLevel; @@ -50,4 +50,9 @@ impl<'a, 'b> Visitor<'b> for LoggerCandidateVisitor<'a, 'b> { } visitor::walk_expr(self, expr); } + + fn visit_except_handler(&mut self, _except_handler: &'b ExceptHandler) { + // Don't recurse into exception handlers, since we'll re-run the visitor on any such + // handlers. + } } diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs b/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs index fda19f79e8b58..d390dafda4492 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs @@ -23,7 +23,7 @@ use crate::rules::tryceratops::helpers::LoggerCandidateVisitor; /// import logging /// /// -/// def foo(): +/// def func(): /// try: /// raise NotImplementedError /// except NotImplementedError: @@ -35,10 +35,10 @@ use crate::rules::tryceratops::helpers::LoggerCandidateVisitor; /// import logging /// /// -/// def foo(): +/// def func(): /// try: /// raise NotImplementedError -/// except NotImplementedError as exc: +/// except NotImplementedError: /// logging.exception("Exception occurred") /// ``` /// diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs b/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs index 8b3fad0f082d9..701e9a575df6c 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs @@ -30,7 +30,7 @@ use crate::rules::tryceratops::helpers::LoggerCandidateVisitor; /// ```python /// try: /// ... -/// except ValueError as e: +/// except ValueError: /// logger.exception("Found an error") /// ``` #[violation] @@ -61,11 +61,7 @@ impl<'a> Visitor<'a> for NameVisitor<'a> { /// TRY401 pub(crate) fn verbose_log_message(checker: &mut Checker, handlers: &[ExceptHandler]) { for handler in handlers { - let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { name, body, .. }) = - handler; - let Some(target) = name else { - continue; - }; + let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { body, .. }) = handler; // Find all calls to `logging.exception`. let calls = { @@ -87,8 +83,14 @@ pub(crate) fn verbose_log_message(checker: &mut Checker, handlers: &[ExceptHandl } names }; + + // Find any bound exceptions in the call. for expr in names { - if expr.id == target.as_str() { + let Some(id) = checker.semantic().resolve_name(expr) else { + continue; + }; + let binding = checker.semantic().binding(id); + if binding.kind.is_bound_exception() { checker .diagnostics .push(Diagnostic::new(VerboseLogMessage, expr.range())); diff --git a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap index 35b36493ec3a9..918fae360ea3a 100644 --- a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap +++ b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap @@ -86,4 +86,12 @@ TRY400.py:90:13: TRY400 Use `logging.exception` instead of `logging.error` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400 | +TRY400.py:121:13: TRY400 Use `logging.exception` instead of `logging.error` + | +119 | b = 2 +120 | except Exception: +121 | error("Context message here") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400 + | + diff --git a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap index 3681b880d1f06..3f71c86e70cdc 100644 --- a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap +++ b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap @@ -177,4 +177,20 @@ TRY401.py:117:40: TRY401 Redundant exception object included in `logging.excepti | ^^ TRY401 | +TRY401.py:139:49: TRY401 Redundant exception object included in `logging.exception` call + | +137 | finish() +138 | except Exception as ex: +139 | logger.exception(f"Found an error: {ex}") # TRY401 + | ^^ TRY401 + | + +TRY401.py:150:49: TRY401 Redundant exception object included in `logging.exception` call + | +148 | finish() +149 | except Exception: +150 | logger.exception(f"Found an error: {ex}") # TRY401 + | ^^ TRY401 + | +