From 4d932c7b59a99d00d2c41576a4872538746038cc Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Sat, 9 Sep 2023 01:23:45 +0200 Subject: [PATCH] Add `ExcInfoFalseInException` Violation --- .../test/fixtures/flake8_logging/LOG007.py | 17 ++++++ .../src/checkers/ast/analyze/expression.rs | 3 ++ crates/ruff/src/codes.rs | 1 + crates/ruff/src/rules/flake8_logging/mod.rs | 1 + .../rules/exc_info_false_in_exception.rs | 54 +++++++++++++++++++ .../src/rules/flake8_logging/rules/mod.rs | 3 +- ...ake8_logging__tests__LOG007_LOG007.py.snap | 22 ++++++++ ruff.schema.json | 1 + 8 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_logging/LOG007.py create mode 100644 crates/ruff/src/rules/flake8_logging/rules/exc_info_false_in_exception.rs create mode 100644 crates/ruff/src/rules/flake8_logging/snapshots/ruff__rules__flake8_logging__tests__LOG007_LOG007.py.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_logging/LOG007.py b/crates/ruff/resources/test/fixtures/flake8_logging/LOG007.py new file mode 100644 index 00000000000000..45d49c6c0d6c6f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_logging/LOG007.py @@ -0,0 +1,17 @@ +import logging +from logging import exception + + +logging.exception("foo") # OK + + +logging.exception("foo", exc_info=False) # LOG007 + + +logging.exception("foo", exc_info=[]) # LOG007 + + +exception("foo", exc_info=False) # LOG007 + + +logging.exception("foo", exc_info=True) # OK diff --git a/crates/ruff/src/checkers/ast/analyze/expression.rs b/crates/ruff/src/checkers/ast/analyze/expression.rs index 0f3b756b730964..d06fc6ecb4a2bd 100644 --- a/crates/ruff/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff/src/checkers/ast/analyze/expression.rs @@ -890,6 +890,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::QuadraticListSummation) { ruff::rules::quadratic_list_summation(checker, call); } + if checker.enabled(Rule::ExcInfoFalseInException) { + flake8_logging::rules::exc_info_false_in_exception(checker, call); + } } Expr::Dict(ast::ExprDict { keys, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 461f4f720e1241..e36b576e14d5b6 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -871,6 +871,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Refurb, "132") => (RuleGroup::Nursery, rules::refurb::rules::CheckAndRemoveFromSet), // flake8-logging + (Flake8Logging, "007") => (RuleGroup::Nursery, rules::flake8_logging::rules::ExcInfoFalseInException), (Flake8Logging, "009") => (RuleGroup::Nursery, rules::flake8_logging::rules::UndocumentedWarn), _ => return None, diff --git a/crates/ruff/src/rules/flake8_logging/mod.rs b/crates/ruff/src/rules/flake8_logging/mod.rs index 05700213f993fd..56e2742d727e35 100644 --- a/crates/ruff/src/rules/flake8_logging/mod.rs +++ b/crates/ruff/src/rules/flake8_logging/mod.rs @@ -13,6 +13,7 @@ mod tests { use crate::settings::Settings; use crate::test::test_path; + #[test_case(Rule::ExcInfoFalseInException, 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()); diff --git a/crates/ruff/src/rules/flake8_logging/rules/exc_info_false_in_exception.rs b/crates/ruff/src/rules/flake8_logging/rules/exc_info_false_in_exception.rs new file mode 100644 index 00000000000000..69d4d66a405636 --- /dev/null +++ b/crates/ruff/src/rules/flake8_logging/rules/exc_info_false_in_exception.rs @@ -0,0 +1,54 @@ +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_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 `exception()` method captures the exception automatically. Disabling this by setting +/// `exc_info=False` is the same as using `error()`, which is clearer and doesn’t need the +/// `exc_info`argument. This rule detects `exception()` calls with an exc_info argument that is +/// falsy. +/// +/// ## Example +/// ```python +/// logging.exception("foo", exc_info=False) +/// ``` +/// +/// Use instead: +/// ```python +/// logging.error("foo") +/// ``` +#[violation] +pub struct ExcInfoFalseInException; + +impl Violation for ExcInfoFalseInException { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use of `logging.exception` with falsy `exc_info`") + } +} + +/// LOG007 +pub(crate) fn exc_info_false_in_exception(checker: &mut Checker, call: &ExprCall) { + if checker + .semantic() + .resolve_call_path(call.func.as_ref()) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "exception"])) + { + if let Some(_) = call.arguments.find_keyword("exc_info").filter(|keyword| { + Truthiness::from_expr(&keyword.value, |id| checker.semantic().is_builtin(id)) + .is_falsey() + }) { + checker + .diagnostics + .push(Diagnostic::new(ExcInfoFalseInException, call.range())); + } + } +} diff --git a/crates/ruff/src/rules/flake8_logging/rules/mod.rs b/crates/ruff/src/rules/flake8_logging/rules/mod.rs index 07b712a4f4239d..fbef448a16b29e 100644 --- a/crates/ruff/src/rules/flake8_logging/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_logging/rules/mod.rs @@ -1,3 +1,4 @@ +pub(crate) use exc_info_false_in_exception::*; pub(crate) use undocumented_warn::*; - +mod exc_info_false_in_exception; mod undocumented_warn; diff --git a/crates/ruff/src/rules/flake8_logging/snapshots/ruff__rules__flake8_logging__tests__LOG007_LOG007.py.snap b/crates/ruff/src/rules/flake8_logging/snapshots/ruff__rules__flake8_logging__tests__LOG007_LOG007.py.snap new file mode 100644 index 00000000000000..ea8b63f5060584 --- /dev/null +++ b/crates/ruff/src/rules/flake8_logging/snapshots/ruff__rules__flake8_logging__tests__LOG007_LOG007.py.snap @@ -0,0 +1,22 @@ +--- +source: crates/ruff/src/rules/flake8_logging/mod.rs +--- +LOG007.py:8:1: LOG007 Use of `logging.exception` with falsy `exc_info` + | +8 | logging.exception("foo", exc_info=False) # LOG007 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 + | + +LOG007.py:11:1: LOG007 Use of `logging.exception` with falsy `exc_info` + | +11 | logging.exception("foo", exc_info=[]) # LOG007 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 + | + +LOG007.py:14:1: LOG007 Use of `logging.exception` with falsy `exc_info` + | +14 | exception("foo", exc_info=False) # LOG007 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 11e692634832fb..750e31a8e1e8e3 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2108,6 +2108,7 @@ "ISC002", "ISC003", "LOG", + "LOG007", "LOG009", "N", "N8",