Skip to content

Commit

Permalink
[flake8-logging] Implement LOG007: ExceptionWithoutExcInfo (#7410)
Browse files Browse the repository at this point in the history
## 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
qdegraaf authored Sep 18, 2023
1 parent 0bfdb15 commit bccba5d
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 0 deletions.
16 changes: 16 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_logging/LOG007.py
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
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::InvalidGetLoggerArgument) {
flake8_logging::rules::invalid_get_logger_argument(checker, call);
}
if checker.enabled(Rule::ExceptionWithoutExcInfo) {
flake8_logging::rules::exception_without_exc_info(checker, call);
}
}
Expr::Dict(ast::ExprDict {
keys,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
// flake8-logging
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
(Flake8Logging, "002") => (RuleGroup::Preview, rules::flake8_logging::rules::InvalidGetLoggerArgument),
(Flake8Logging, "007") => (RuleGroup::Preview, rules::flake8_logging::rules::ExceptionWithoutExcInfo),
(Flake8Logging, "009") => (RuleGroup::Preview, rules::flake8_logging::rules::UndocumentedWarn),

_ => return None,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_logging/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::ExceptionWithoutExcInfo, Path::new("LOG007.py"))]
#[test_case(Rule::UndocumentedWarn, Path::new("LOG009.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Expand Down
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()));
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_logging/rules/mod.rs
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;
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
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit bccba5d

Please sign in to comment.