From 5d99ef9d95b22c1d8217a23219483c3fe6ffb024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=98=D0=BB=D1=8C=D1=8F=20=D0=9B=D1=8E=D0=B1=D0=B0=D0=B2?= =?UTF-8?q?=D1=81=D0=BA=D0=B8=D0=B9?= <100635212+lubaskinc0de@users.noreply.github.com> Date: Thu, 15 Jan 2026 00:18:13 +0300 Subject: [PATCH] [`flake8-blind-except`] Allow more logging methods (`BLE001`) (#22057) ## Summary Fix issue #21889 by checking that the logging method is one of the ``debug``, ``info``, ``warning``, ``error``, ``exception``, ``critical``, ``log`` methods that support ``exc_info`` passing. Also fixed the behavior in which ``exc_info`` was considered passed only when it was equal to the literal ``True``, now the ``Truthiness`` of the expression is checked (we will leave additional checks to type checkers) ## Test Plan Additional snapshot tests have been added for all logging functions, as well as tests in which an exception object is passed as ``exc_info`` --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../test/fixtures/flake8_blind_except/BLE.py | 65 ++++++++++++++---- crates/ruff_linter/src/preview.rs | 5 ++ .../src/rules/flake8_blind_except/mod.rs | 25 ++++++- .../flake8_blind_except/rules/blind_except.rs | 61 ++++++++++++++--- ...e8_blind_except__tests__BLE001_BLE.py.snap | 68 ++++++++++++++++--- ..._except__tests__preview_BLE001_BLE.py.snap | 65 ++++++++++++++++++ .../src/rules/flake8_logging/helpers.rs | 8 +++ .../src/rules/flake8_logging/mod.rs | 2 +- .../flake8_logging/rules/root_logger_call.rs | 9 +-- 9 files changed, 263 insertions(+), 45 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__preview_BLE001_BLE.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_blind_except/BLE.py b/crates/ruff_linter/resources/test/fixtures/flake8_blind_except/BLE.py index f16c7f7e61..76210a03bc 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_blind_except/BLE.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_blind_except/BLE.py @@ -67,31 +67,31 @@ import logging try: pass except Exception: - logging.error("...") + logging.error("...") # BLE001 try: pass except Exception: - logging.error("...", exc_info=False) + logging.error("...", exc_info=False) # BLE001 try: pass except Exception: - logging.error("...", exc_info=None) + logging.error("...", exc_info=None) # BLE001 try: pass except Exception: - logging.exception("...") + logging.exception("...") # ok try: pass except Exception: - logging.error("...", exc_info=True) + logging.error("...", exc_info=True) # ok from logging import critical, error, exception @@ -99,54 +99,54 @@ from logging import critical, error, exception try: pass except Exception: - error("...") + error("...") # BLE001 try: pass except Exception: - error("...", exc_info=False) + error("...", exc_info=False) # BLE001 try: pass except Exception: - error("...", exc_info=None) + error("...", exc_info=None) # BLE001 try: pass except Exception: - critical("...") + critical("...") # BLE001 try: pass except Exception: - critical("...", exc_info=False) + critical("...", exc_info=False) # BLE001 try: pass except Exception: - critical("...", exc_info=None) + critical("...", exc_info=None) # BLE001 try: pass except Exception: - exception("...") + exception("...") # ok try: pass except Exception: - error("...", exc_info=True) + error("...", exc_info=True) # ok try: pass except Exception: - critical("...", exc_info=True) + critical("...", exc_info=True) # ok try: @@ -239,7 +239,7 @@ except (OSError, FileNotFoundError) as e: try: pass except (Exception, ValueError): - critical("...", exc_info=True) + critical("...", exc_info=True) # ok try: pass @@ -256,3 +256,38 @@ try: pass except BaseException as e: raise e from None + + +try: + pass +except Exception as e: + logging.error("...", exc_info=e) # ok + + +try: + pass +except Exception as e: + logging.debug("...", exc_info=e) # ok + + +try: + pass +except Exception: + logging.info("...", exc_info=True) # ok + +try: + pass +except Exception as e: + logging.warn("...", exc_info=e) # ok + + +try: + pass +except Exception: + logging.warning("...", exc_info=True) # ok + + +try: + pass +except Exception as e: + logging.log(logging.INFO, "...", exc_info=e) # ok diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index b28474889e..76d5880614 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -297,6 +297,11 @@ pub(crate) const fn is_range_suppressions_enabled(settings: &LinterSettings) -> settings.preview.is_enabled() } +// https://github.com/astral-sh/ruff/pull/22057 +pub(crate) const fn is_ble001_exc_info_suppression_enabled(settings: &LinterSettings) -> bool { + settings.preview.is_enabled() +} + // https://github.com/astral-sh/ruff/pull/22419 pub(crate) const fn is_py315_support_enabled(settings: &LinterSettings) -> bool { settings.preview.is_enabled() diff --git a/crates/ruff_linter/src/rules/flake8_blind_except/mod.rs b/crates/ruff_linter/src/rules/flake8_blind_except/mod.rs index 658908efb6..88f1c9099c 100644 --- a/crates/ruff_linter/src/rules/flake8_blind_except/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_blind_except/mod.rs @@ -9,8 +9,9 @@ mod tests { use test_case::test_case; use crate::registry::Rule; + use crate::settings::types::PreviewMode; use crate::test::test_path; - use crate::{assert_diagnostics, settings}; + use crate::{assert_diagnostics, assert_diagnostics_diff, settings}; #[test_case(Rule::BlindExcept, Path::new("BLE.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { @@ -22,4 +23,26 @@ mod tests { assert_diagnostics!(snapshot, diagnostics); Ok(()) } + + #[test_case(Rule::BlindExcept, Path::new("BLE.py"))] + fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "preview_{}_{}", + rule_code.noqa_code(), + path.to_string_lossy() + ); + assert_diagnostics_diff!( + snapshot, + Path::new("flake8_blind_except").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Disabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + &settings::LinterSettings { + preview: PreviewMode::Enabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + ); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/flake8_blind_except/rules/blind_except.rs b/crates/ruff_linter/src/rules/flake8_blind_except/rules/blind_except.rs index 8e0992f0aa..185db9e495 100644 --- a/crates/ruff_linter/src/rules/flake8_blind_except/rules/blind_except.rs +++ b/crates/ruff_linter/src/rules/flake8_blind_except/rules/blind_except.rs @@ -1,5 +1,5 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::helpers::is_const_true; +use ruff_python_ast::helpers::Truthiness; use ruff_python_ast::statement_visitor::{StatementVisitor, walk_stmt}; use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_python_semantic::SemanticModel; @@ -8,6 +8,9 @@ use ruff_text_size::Ranged; use crate::Violation; use crate::checkers::ast::Checker; +use crate::preview::is_ble001_exc_info_suppression_enabled; +use crate::rules::flake8_logging::helpers::is_logger_method_name; +use crate::settings::LinterSettings; /// ## What it does /// Checks for `except` clauses that catch all exceptions. This includes @@ -47,8 +50,7 @@ use crate::checkers::ast::Checker; /// raise /// ``` /// -/// Exceptions that are logged via `logging.exception()` or are logged via -/// `logging.error()` or `logging.critical()` with `exc_info` enabled will +/// Exceptions that are logged with `exc_info` enabled will /// _not_ be flagged, as this is a common pattern for propagating exception /// traces: /// ```python @@ -120,7 +122,11 @@ pub(crate) fn blind_except( } // If the exception is logged, don't flag an error. - let mut visitor = LogExceptionVisitor::new(semantic, &checker.settings().logger_objects); + let mut visitor = LogExceptionVisitor::new( + semantic, + &checker.settings().logger_objects, + checker.settings(), + ); visitor.visit_body(body); if visitor.seen() { return; @@ -184,19 +190,44 @@ impl<'a> StatementVisitor<'a> for ReraiseVisitor<'a> { } } +/// Returns `true` if the `exc_info` keyword argument is truthy. +fn is_exc_info_enabled( + method_name: &str, + arguments: &ast::Arguments, + semantic: &SemanticModel, + settings: &LinterSettings, +) -> bool { + if is_ble001_exc_info_suppression_enabled(settings) + || matches!(method_name, "error" | "critical") + { + arguments.find_keyword("exc_info").is_some_and(|keyword| { + Truthiness::from_expr(&keyword.value, |id| semantic.has_builtin_binding(id)).into_bool() + != Some(false) + }) + } else { + false + } +} + /// A visitor to detect whether the exception was logged. struct LogExceptionVisitor<'a> { semantic: &'a SemanticModel<'a>, logger_objects: &'a [String], + settings: &'a LinterSettings, seen: bool, } impl<'a> LogExceptionVisitor<'a> { /// Create a new [`LogExceptionVisitor`] with the given exception name. - fn new(semantic: &'a SemanticModel<'a>, logger_objects: &'a [String]) -> Self { + fn new( + semantic: &'a SemanticModel<'a>, + logger_objects: &'a [String], + settings: &'a LinterSettings, + ) -> Self { Self { semantic, logger_objects, + settings, seen: false, } } @@ -227,9 +258,12 @@ impl<'a> StatementVisitor<'a> for LogExceptionVisitor<'a> { ) { if match attr.as_str() { "exception" => true, - "error" | "critical" => arguments - .find_keyword("exc_info") - .is_some_and(|keyword| is_const_true(&keyword.value)), + _ if is_logger_method_name(attr) => is_exc_info_enabled( + attr, + arguments, + self.semantic, + self.settings, + ), _ => false, } { self.seen = true; @@ -240,9 +274,14 @@ impl<'a> StatementVisitor<'a> for LogExceptionVisitor<'a> { if self.semantic.resolve_qualified_name(func).is_some_and( |qualified_name| match qualified_name.segments() { ["logging", "exception"] => true, - ["logging", "error" | "critical"] => arguments - .find_keyword("exc_info") - .is_some_and(|keyword| is_const_true(&keyword.value)), + ["logging", method] if is_logger_method_name(method) => { + is_exc_info_enabled( + method, + arguments, + self.semantic, + self.settings, + ) + } _ => false, }, ) { diff --git a/crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__BLE001_BLE.py.snap b/crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__BLE001_BLE.py.snap index 1924cd03aa..5011575277 100644 --- a/crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__BLE001_BLE.py.snap +++ b/crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__BLE001_BLE.py.snap @@ -81,7 +81,7 @@ BLE001 Do not catch blind exception: `Exception` 68 | pass 69 | except Exception: | ^^^^^^^^^ -70 | logging.error("...") +70 | logging.error("...") # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -91,7 +91,7 @@ BLE001 Do not catch blind exception: `Exception` 74 | pass 75 | except Exception: | ^^^^^^^^^ -76 | logging.error("...", exc_info=False) +76 | logging.error("...", exc_info=False) # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -101,7 +101,7 @@ BLE001 Do not catch blind exception: `Exception` 80 | pass 81 | except Exception: | ^^^^^^^^^ -82 | logging.error("...", exc_info=None) +82 | logging.error("...", exc_info=None) # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -111,7 +111,7 @@ BLE001 Do not catch blind exception: `Exception` 100 | pass 101 | except Exception: | ^^^^^^^^^ -102 | error("...") +102 | error("...") # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -121,7 +121,7 @@ BLE001 Do not catch blind exception: `Exception` 106 | pass 107 | except Exception: | ^^^^^^^^^ -108 | error("...", exc_info=False) +108 | error("...", exc_info=False) # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -131,7 +131,7 @@ BLE001 Do not catch blind exception: `Exception` 112 | pass 113 | except Exception: | ^^^^^^^^^ -114 | error("...", exc_info=None) +114 | error("...", exc_info=None) # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -141,7 +141,7 @@ BLE001 Do not catch blind exception: `Exception` 118 | pass 119 | except Exception: | ^^^^^^^^^ -120 | critical("...") +120 | critical("...") # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -151,7 +151,7 @@ BLE001 Do not catch blind exception: `Exception` 124 | pass 125 | except Exception: | ^^^^^^^^^ -126 | critical("...", exc_info=False) +126 | critical("...", exc_info=False) # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -161,7 +161,7 @@ BLE001 Do not catch blind exception: `Exception` 130 | pass 131 | except Exception: | ^^^^^^^^^ -132 | critical("...", exc_info=None) +132 | critical("...", exc_info=None) # BLE001 | BLE001 Do not catch blind exception: `Exception` @@ -263,3 +263,53 @@ BLE001 Do not catch blind exception: `BaseException` | ^^^^^^^^^^^^^ 221 | pass | + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:269:8 + | +267 | try: +268 | pass +269 | except Exception as e: + | ^^^^^^^^^ +270 | logging.debug("...", exc_info=e) # ok + | + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:275:8 + | +273 | try: +274 | pass +275 | except Exception: + | ^^^^^^^^^ +276 | logging.info("...", exc_info=True) # ok + | + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:280:8 + | +278 | try: +279 | pass +280 | except Exception as e: + | ^^^^^^^^^ +281 | logging.warn("...", exc_info=e) # ok + | + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:286:8 + | +284 | try: +285 | pass +286 | except Exception: + | ^^^^^^^^^ +287 | logging.warning("...", exc_info=True) # ok + | + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:292:8 + | +290 | try: +291 | pass +292 | except Exception as e: + | ^^^^^^^^^ +293 | logging.log(logging.INFO, "...", exc_info=e) # ok + | diff --git a/crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__preview_BLE001_BLE.py.snap b/crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__preview_BLE001_BLE.py.snap new file mode 100644 index 0000000000..7fc7dd0407 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_blind_except/snapshots/ruff_linter__rules__flake8_blind_except__tests__preview_BLE001_BLE.py.snap @@ -0,0 +1,65 @@ +--- +source: crates/ruff_linter/src/rules/flake8_blind_except/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 5 +Added: 0 + +--- Removed --- +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:269:8 + | +267 | try: +268 | pass +269 | except Exception as e: + | ^^^^^^^^^ +270 | logging.debug("...", exc_info=e) # ok + | + + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:275:8 + | +273 | try: +274 | pass +275 | except Exception: + | ^^^^^^^^^ +276 | logging.info("...", exc_info=True) # ok + | + + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:280:8 + | +278 | try: +279 | pass +280 | except Exception as e: + | ^^^^^^^^^ +281 | logging.warn("...", exc_info=e) # ok + | + + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:286:8 + | +284 | try: +285 | pass +286 | except Exception: + | ^^^^^^^^^ +287 | logging.warning("...", exc_info=True) # ok + | + + +BLE001 Do not catch blind exception: `Exception` + --> BLE.py:292:8 + | +290 | try: +291 | pass +292 | except Exception as e: + | ^^^^^^^^^ +293 | logging.log(logging.INFO, "...", exc_info=e) # ok + | diff --git a/crates/ruff_linter/src/rules/flake8_logging/helpers.rs b/crates/ruff_linter/src/rules/flake8_logging/helpers.rs index 47c57d14be..63d7ba5f7b 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_logging/helpers.rs @@ -22,3 +22,11 @@ pub(super) fn outside_handlers(offset: TextSize, semantic: &SemanticModel) -> bo true } + +#[inline] +pub(crate) fn is_logger_method_name(attr: &str) -> bool { + matches!( + attr, + "debug" | "info" | "warn" | "warning" | "error" | "critical" | "log" | "exception" + ) +} diff --git a/crates/ruff_linter/src/rules/flake8_logging/mod.rs b/crates/ruff_linter/src/rules/flake8_logging/mod.rs index a4ec5974c3..7c736814b6 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_logging/mod.rs @@ -1,5 +1,5 @@ //! Rules from [flake8-logging](https://pypi.org/project/flake8-logging/). -mod helpers; +pub(crate) mod helpers; pub(crate) mod rules; #[cfg(test)] diff --git a/crates/ruff_linter/src/rules/flake8_logging/rules/root_logger_call.rs b/crates/ruff_linter/src/rules/flake8_logging/rules/root_logger_call.rs index f7b6a55706..48e9087848 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/rules/root_logger_call.rs +++ b/crates/ruff_linter/src/rules/flake8_logging/rules/root_logger_call.rs @@ -4,6 +4,7 @@ use ruff_python_semantic::Modules; use crate::Violation; use crate::checkers::ast::Checker; +use crate::rules::flake8_logging::helpers::is_logger_method_name; /// ## What it does /// Checks for usages of the following `logging` top-level functions: @@ -66,11 +67,3 @@ pub(crate) fn root_logger_call(checker: &Checker, call: &ExprCall) { }; checker.report_diagnostic(kind, call.range); } - -#[inline] -fn is_logger_method_name(attr: &str) -> bool { - matches!( - attr, - "debug" | "info" | "warn" | "warning" | "error" | "critical" | "log" | "exception" - ) -}