mirror of
https://github.com/astral-sh/ruff
synced 2026-01-20 21:10:48 -05:00
[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>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
) {
|
||||
|
||||
@@ -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
|
||||
|
|
||||
|
||||
@@ -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
|
||||
|
|
||||
@@ -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"
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user