-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
## Summary This PR implements a new rule for `flake8-logging` plugin that checks for uses of `logging.exception()` with `exc_info` set to `False` or a falsy value. It suggests using `logging.error` in these cases instead. I am unsure about the name. Open to suggestions there, went with the most explicit name I could think of in the meantime. Refer #7248 ## Test Plan Added a new fixture cases and ran `cargo test`
- Loading branch information
Showing
8 changed files
with
129 additions
and
0 deletions.
There are no files selected for viewing
16 changes: 16 additions & 0 deletions
16
crates/ruff/resources/test/fixtures/flake8_logging/LOG007.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import logging | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
logging.exception("foo") # OK | ||
logging.exception("foo", exc_info=False) # LOG007 | ||
logging.exception("foo", exc_info=[]) # LOG007 | ||
logger.exception("foo") # OK | ||
logger.exception("foo", exc_info=False) # LOG007 | ||
logger.exception("foo", exc_info=[]) # LOG007 | ||
|
||
|
||
from logging import exception | ||
|
||
exception("foo", exc_info=False) # LOG007 | ||
exception("foo", exc_info=True) # OK |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65 changes: 65 additions & 0 deletions
65
crates/ruff/src/rules/flake8_logging/rules/exception_without_exc_info.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
use ruff_python_ast::ExprCall; | ||
|
||
use ruff_diagnostics::{Diagnostic, Violation}; | ||
use ruff_macros::{derive_message_formats, violation}; | ||
use ruff_python_ast::helpers::Truthiness; | ||
use ruff_python_semantic::analyze::logging::is_logger_candidate; | ||
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`. | ||
/// | ||
/// ## Why is this bad? | ||
/// The `logging.exception()` method captures the exception automatically, but | ||
/// accepts an optional `exc_info` argument to override this behavior. Setting | ||
/// `exc_info` to `False` disables the automatic capture of the exception and | ||
/// stack trace. | ||
/// | ||
/// Instead of setting `exc_info` to `False`, prefer `logging.error()`, which | ||
/// has equivalent behavior to `logging.exception()` with `exc_info` set to | ||
/// `False`, but is clearer in intent. | ||
/// | ||
/// ## Example | ||
/// ```python | ||
/// logging.exception("...", exc_info=False) | ||
/// ``` | ||
/// | ||
/// Use instead: | ||
/// ```python | ||
/// logging.error("...") | ||
/// ``` | ||
#[violation] | ||
pub struct ExceptionWithoutExcInfo; | ||
|
||
impl Violation for ExceptionWithoutExcInfo { | ||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
format!("Use of `logging.exception` with falsy `exc_info`") | ||
} | ||
} | ||
|
||
/// LOG007 | ||
pub(crate) fn exception_without_exc_info(checker: &mut Checker, call: &ExprCall) { | ||
if !is_logger_candidate( | ||
call.func.as_ref(), | ||
checker.semantic(), | ||
&["exception".to_string()], | ||
) { | ||
return; | ||
} | ||
|
||
if call | ||
.arguments | ||
.find_keyword("exc_info") | ||
.map(|keyword| &keyword.value) | ||
.is_some_and(|value| { | ||
Truthiness::from_expr(value, |id| checker.semantic().is_builtin(id)).is_falsey() | ||
}) | ||
{ | ||
checker | ||
.diagnostics | ||
.push(Diagnostic::new(ExceptionWithoutExcInfo, call.range())); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
pub(crate) use direct_logger_instantiation::*; | ||
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_without_exc_info; | ||
mod invalid_get_logger_argument; | ||
mod undocumented_warn; |
40 changes: 40 additions & 0 deletions
40
.../rules/flake8_logging/snapshots/ruff__rules__flake8_logging__tests__LOG007_LOG007.py.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
--- | ||
source: crates/ruff/src/rules/flake8_logging/mod.rs | ||
--- | ||
LOG007.py:6:1: LOG007 Use of `logging.exception` with falsy `exc_info` | ||
| | ||
5 | logging.exception("foo") # OK | ||
6 | logging.exception("foo", exc_info=False) # LOG007 | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 | ||
7 | logging.exception("foo", exc_info=[]) # LOG007 | ||
8 | logger.exception("foo") # OK | ||
| | ||
|
||
LOG007.py:7:1: LOG007 Use of `logging.exception` with falsy `exc_info` | ||
| | ||
5 | logging.exception("foo") # OK | ||
6 | logging.exception("foo", exc_info=False) # LOG007 | ||
7 | logging.exception("foo", exc_info=[]) # LOG007 | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 | ||
8 | logger.exception("foo") # OK | ||
9 | logger.exception("foo", exc_info=False) # LOG007 | ||
| | ||
|
||
LOG007.py:9:1: LOG007 Use of `logging.exception` with falsy `exc_info` | ||
| | ||
7 | logging.exception("foo", exc_info=[]) # LOG007 | ||
8 | logger.exception("foo") # OK | ||
9 | logger.exception("foo", exc_info=False) # LOG007 | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 | ||
10 | logger.exception("foo", exc_info=[]) # LOG007 | ||
| | ||
|
||
LOG007.py:10:1: LOG007 Use of `logging.exception` with falsy `exc_info` | ||
| | ||
8 | logger.exception("foo") # OK | ||
9 | logger.exception("foo", exc_info=False) # LOG007 | ||
10 | logger.exception("foo", exc_info=[]) # LOG007 | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 | ||
| | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.