From 2aef46cb6f1c037e315a8b71312999a1c79f290d Mon Sep 17 00:00:00 2001 From: qdegraaf <34540841+qdegraaf@users.noreply.github.com> Date: Wed, 27 Sep 2023 02:21:22 +0200 Subject: [PATCH] Add `Expr::Name` checks to rules which use `is_logger_candidate` (#7521) ## Summary Expands several rules to also check for `Expr::Name` values. As they would previously not consider: ```python from logging import error error("foo") ``` as potential violations ```python import logging logging.error("foo") ``` as potential violations leading to inconsistent behaviour. The rules impacted are: - `BLE001` - `TRY400` - `TRY401` - `PLE1205` - `PLE1206` - `LOG007` - `G001`-`G004` - `G101` - `G201` - `G202` ## Test Plan Fixtures for all impacted rules expanded. ## Issue Link Refers: https://github.com/astral-sh/ruff/issues/7502 --- .../test/fixtures/flake8_blind_except/BLE.py | 32 +++++++ .../test/fixtures/flake8_logging/LOG007.py | 6 ++ .../fixtures/flake8_logging_format/G001.py | 9 ++ .../fixtures/flake8_logging_format/G002.py | 5 ++ .../fixtures/flake8_logging_format/G003.py | 5 ++ .../fixtures/flake8_logging_format/G004.py | 4 + .../fixtures/flake8_logging_format/G010.py | 5 ++ .../fixtures/flake8_logging_format/G101_1.py | 9 ++ .../fixtures/flake8_logging_format/G101_2.py | 9 ++ .../fixtures/flake8_logging_format/G201.py | 21 +++++ .../fixtures/flake8_logging_format/G202.py | 20 +++++ .../fixtures/pylint/logging_too_few_args.py | 26 ++++++ .../fixtures/pylint/logging_too_many_args.py | 22 +++++ .../test/fixtures/tryceratops/TRY400.py | 34 +++++++ .../test/fixtures/tryceratops/TRY401.py | 64 ++++++++++++++ .../flake8_blind_except/rules/blind_except.rs | 53 ++++++++--- ...e8_blind_except__tests__BLE001_BLE.py.snap | 27 ++++++ .../rules/exception_without_exc_info.rs | 57 +++++++----- ...ake8_logging__tests__LOG007_LOG007.py.snap | 9 ++ .../rules/logging_call.rs | 48 ++++++---- ...flake8_logging_format__tests__G001.py.snap | 58 ++++++++++++ ...flake8_logging_format__tests__G002.py.snap | 18 ++++ ...flake8_logging_format__tests__G003.py.snap | 18 ++++ ...flake8_logging_format__tests__G004.py.snap | 28 ++++-- ...flake8_logging_format__tests__G010.py.snap | 25 ++++++ ...ake8_logging_format__tests__G101_1.py.snap | 10 +++ ...ake8_logging_format__tests__G101_2.py.snap | 10 +++ ...flake8_logging_format__tests__G201.py.snap | 20 +++++ ...flake8_logging_format__tests__G202.py.snap | 20 +++++ .../src/rules/pylint/rules/logging.rs | 39 +++++--- ...sts__PLE1205_logging_too_many_args.py.snap | 20 +++++ ...ests__PLE1206_logging_too_few_args.py.snap | 10 +++ .../src/rules/tryceratops/helpers.rs | 29 ++++-- .../rules/error_instead_of_exception.rs | 18 ++-- .../tryceratops/rules/verbose_log_message.rs | 37 ++++---- ..._error-instead-of-exception_TRY400.py.snap | 17 ++++ ..._tests__verbose-log-message_TRY401.py.snap | 88 +++++++++++++++++++ 37 files changed, 823 insertions(+), 107 deletions(-) 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 6ce4ab3dcefbc..b2272b8ab591e 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 @@ -92,3 +92,35 @@ pass except Exception: logging.error("...", exc_info=True) + + +from logging import error, exception + +try: + pass +except Exception: + error("...") + + +try: + pass +except Exception: + error("...", exc_info=False) + + +try: + pass +except Exception: + error("...", exc_info=None) + + +try: + pass +except Exception: + exception("...") + + +try: + pass +except Exception: + error("...", exc_info=True) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG007.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG007.py index 00fe85bf9159a..24a4cff872bf7 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG007.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG007.py @@ -16,3 +16,9 @@ exception("foo", exc_info=False) # LOG007 exception("foo", exc_info=True) # OK + + +exception = lambda *args, **kwargs: None + +exception("foo", exc_info=False) # OK +exception("foo", exc_info=True) # OK diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G001.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G001.py index 7a3a49a6a7d10..f100eae9a4d6c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G001.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G001.py @@ -16,3 +16,12 @@ flask.current_app.logger.info("Hello {}".format("World!")) current_app.logger.info("Hello {}".format("World!")) app.logger.log(logging.INFO, "Hello {}".format("World!")) + +from logging import info, log + +info("Hello {}".format("World!")) +log(logging.INFO, "Hello {}".format("World!")) +info("Hello {}".format("World!")) +log(logging.INFO, msg="Hello {}".format("World!")) +log(level=logging.INFO, msg="Hello {}".format("World!")) +log(msg="Hello {}".format("World!"), level=logging.INFO) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G002.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G002.py index 11b61a1cb8e7a..6c01eb8819e19 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G002.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G002.py @@ -2,3 +2,8 @@ logging.info("Hello %s" % "World!") logging.log(logging.INFO, "Hello %s" % "World!") + +from logging import info, log + +info("Hello %s" % "World!") +log(logging.INFO, "Hello %s" % "World!") diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G003.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G003.py index 4e20dc6d9b285..1f52dbe3df615 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G003.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G003.py @@ -2,3 +2,8 @@ logging.info("Hello" + " " + "World!") logging.log(logging.INFO, "Hello" + " " + "World!") + +from logging import info, log + +info("Hello" + " " + "World!") +log(logging.INFO, "Hello" + " " + "World!") diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py index 56e005293a62a..ffc9ed7dd2c03 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py @@ -6,3 +6,7 @@ _LOGGER = logging.getLogger() _LOGGER.info(f"{__name__}") + +from logging import info +info(f"{name}") +info(f"{__name__}") diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G010.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G010.py index fc0029c40987a..3c1294069b06e 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G010.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G010.py @@ -8,3 +8,8 @@ logger.warn("Hello world!") logging . warn("Hello World!") + +from logging import warn, warning, exception +warn("foo") +warning("foo") +exception("foo") diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_1.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_1.py index 3386f5e480c88..08131551a2f7c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_1.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_1.py @@ -6,3 +6,12 @@ "name": "foobar", }, ) + +from logging import info + +info( + "Hello world!", + extra={ + "name": "foobar", + }, +) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_2.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_2.py index c73c54e4c67e0..c21e3c37cffc0 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_2.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G101_2.py @@ -6,3 +6,12 @@ name="foobar", ), ) + +from logging import info + +info( + "Hello world!", + extra=dict( + name="foobar", + ), +) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G201.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G201.py index e85490752028e..d8610e0e26789 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G201.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G201.py @@ -19,3 +19,24 @@ logging.error("Hello World", exc_info=False) logging.error("Hello World", exc_info=sys.exc_info()) + +# G201 +from logging import error +try: + pass +except: + error("Hello World", exc_info=True) + +try: + pass +except: + error("Hello World", exc_info=sys.exc_info()) + +# OK +try: + pass +except: + error("Hello World", exc_info=False) + +error("Hello World", exc_info=sys.exc_info()) + diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G202.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G202.py index 6af630930ca1b..aca5f3a45baa1 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G202.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G202.py @@ -19,3 +19,23 @@ logging.exception("Hello World", exc_info=False) logging.exception("Hello World", exc_info=True) + +# G202 +from logging import exception +try: + pass +except: + exception("Hello World", exc_info=True) + +try: + pass +except: + exception("Hello World", exc_info=sys.exc_info()) + +# OK +try: + pass +except: + exception("Hello World", exc_info=False) + +exception("Hello World", exc_info=True) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_few_args.py b/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_few_args.py index 65f021ea428e1..60a7704e5c936 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_few_args.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_few_args.py @@ -26,3 +26,29 @@ import warning warning.warning("Hello %s %s", "World!") + + +from logging import error, info, warning + +warning("Hello %s %s", "World!") # [logging-too-few-args] + +# do not handle calls with kwargs (like pylint) +warning("Hello %s", "World!", "again", something="else") + +warning("Hello %s", "World!") + +# do not handle calls without any args +info("100% dynamic") + +# do not handle calls with *args +error("Example log %s, %s", "foo", "bar", "baz", *args) + +# do not handle calls with **kwargs +error("Example log %s, %s", "foo", "bar", "baz", **kwargs) + +# do not handle keyword arguments +error("%(objects)d modifications: %(modifications)d errors: %(errors)d") + +info(msg="Hello %s") + +info(msg="Hello %s %s") diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_many_args.py b/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_many_args.py index c64641eaed224..467336f8f2c4e 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_many_args.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/logging_too_many_args.py @@ -22,3 +22,25 @@ import warning warning.warning("Hello %s", "World!", "again") + + +from logging import info, error, warning + +warning("Hello %s", "World!", "again") # [logging-too-many-args] + +warning("Hello %s", "World!", "again", something="else") + +warning("Hello %s", "World!") + +# do not handle calls with *args +error("Example log %s, %s", "foo", "bar", "baz", *args) + +# do not handle calls with **kwargs +error("Example log %s, %s", "foo", "bar", "baz", **kwargs) + +# do not handle keyword arguments +error("%(objects)d modifications: %(modifications)d errors: %(errors)d", {"objects": 1, "modifications": 1, "errors": 1}) + +info(msg="Hello") + +info(msg="Hello", something="else") diff --git a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py index 1c31197c26754..987c321907880 100644 --- a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py +++ b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py @@ -75,3 +75,37 @@ def fine(): a = 1 except Exception: logger.error("Context message here", exc_info=sys.exc_info()) + + +from logging import error, exception + + +def bad(): + try: + a = 1 + except Exception: + error("Context message here") + + if True: + error("Context message here") + + +def good(): + try: + a = 1 + except Exception: + exception("Context message here") + + +def fine(): + try: + a = 1 + except Exception: + error("Context message here", exc_info=True) + + +def fine(): + try: + a = 1 + except Exception: + error("Context message here", exc_info=sys.exc_info()) diff --git a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py index b96661a68ba5c..0aa58677837fd 100644 --- a/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py +++ b/crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py @@ -62,3 +62,67 @@ def main_function(): except Exception as ex: logger.exception(f"Found an error: {er}") logger.exception(f"Found an error: {ex.field}") + + + +from logging import error, exception + + +def main_function(): + try: + process() + handle() + finish() + except Exception as ex: + exception(f"Found an error: {ex}") # TRY401 + + +def main_function(): + try: + process() + handle() + finish() + except ValueError as bad: + if True is False: + for i in range(10): + exception(f"Found an error: {bad} {good}") # TRY401 + except IndexError as bad: + exception(f"Found an error: {bad} {bad}") # TRY401 + except Exception as bad: + exception(f"Found an error: {bad}") # TRY401 + exception(f"Found an error: {bad}") # TRY401 + + if True: + exception(f"Found an error: {bad}") # TRY401 + + +def func_fstr(): + try: + ... + except Exception as ex: + exception(f"Logging an error: {ex}") # TRY401 + + +def func_concat(): + try: + ... + except Exception as ex: + exception("Logging an error: " + str(ex)) # TRY401 + + +def func_comma(): + try: + ... + except Exception as ex: + exception("Logging an error:", ex) # TRY401 + + +# OK +def main_function(): + try: + process() + handle() + finish() + except Exception as ex: + exception(f"Found an error: {er}") + exception(f"Found an error: {ex.field}") 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 0dece92c29397..102f40be9913e 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 @@ -98,24 +98,49 @@ pub(crate) fn blind_except( func, arguments, .. }) = value.as_ref() { - if logging::is_logger_candidate( - func, - checker.semantic(), - &checker.settings.logger_objects, - ) { - if let Some(attribute) = func.as_attribute_expr() { - let attr = attribute.attr.as_str(); - if attr == "exception" { - return true; - } - if attr == "error" { - if let Some(keyword) = arguments.find_keyword("exc_info") { - if is_const_true(&keyword.value) { - return true; + match func.as_ref() { + Expr::Attribute(ast::ExprAttribute { attr, .. }) => { + if logging::is_logger_candidate( + func, + checker.semantic(), + &checker.settings.logger_objects, + ) { + match attr.as_str() { + "exception" => return true, + "error" => { + if let Some(keyword) = arguments.find_keyword("exc_info") { + if is_const_true(&keyword.value) { + return true; + } + } } + _ => {} } } } + Expr::Name(ast::ExprName { .. }) => { + if checker + .semantic() + .resolve_call_path(func.as_ref()) + .is_some_and(|call_path| match call_path.as_slice() { + ["logging", "exception"] => true, + ["logging", "error"] => { + if let Some(keyword) = arguments.find_keyword("exc_info") { + if is_const_true(&keyword.value) { + return true; + } + } + false + } + _ => false, + }) + { + return true; + } + } + _ => { + return 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 0555d9a748f6b..16313b5f91537 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 @@ -94,4 +94,31 @@ BLE.py:81:8: BLE001 Do not catch blind exception: `Exception` 82 | logging.error("...", exc_info=None) | +BLE.py:101:8: BLE001 Do not catch blind exception: `Exception` + | + 99 | try: +100 | pass +101 | except Exception: + | ^^^^^^^^^ BLE001 +102 | error("...") + | + +BLE.py:107:8: BLE001 Do not catch blind exception: `Exception` + | +105 | try: +106 | pass +107 | except Exception: + | ^^^^^^^^^ BLE001 +108 | error("...", exc_info=False) + | + +BLE.py:113:8: BLE001 Do not catch blind exception: `Exception` + | +111 | try: +112 | pass +113 | except Exception: + | ^^^^^^^^^ BLE001 +114 | error("...", exc_info=None) + | + diff --git a/crates/ruff_linter/src/rules/flake8_logging/rules/exception_without_exc_info.rs b/crates/ruff_linter/src/rules/flake8_logging/rules/exception_without_exc_info.rs index 49eb539b31595..d3eac436387f5 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/rules/exception_without_exc_info.rs +++ b/crates/ruff_linter/src/rules/flake8_logging/rules/exception_without_exc_info.rs @@ -1,8 +1,7 @@ -use ruff_python_ast::{self as ast, Expr, ExprCall}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::Truthiness; +use ruff_python_ast::{self as ast, Expr, ExprCall}; use ruff_python_semantic::analyze::logging; use ruff_python_stdlib::logging::LoggingLevel; use ruff_text_size::Ranged; @@ -43,35 +42,47 @@ impl Violation for ExceptionWithoutExcInfo { /// LOG007 pub(crate) fn exception_without_exc_info(checker: &mut Checker, call: &ExprCall) { - let Expr::Attribute(ast::ExprAttribute { value: _, attr, .. }) = call.func.as_ref() else { - return; - }; + match call.func.as_ref() { + Expr::Attribute(ast::ExprAttribute { attr, .. }) => { + if !matches!( + LoggingLevel::from_attribute(attr.as_str()), + Some(LoggingLevel::Exception) + ) { + return; + } - if !matches!( - LoggingLevel::from_attribute(attr.as_str()), - Some(LoggingLevel::Exception) - ) { - return; + if !logging::is_logger_candidate( + &call.func, + checker.semantic(), + &checker.settings.logger_objects, + ) { + return; + } + } + Expr::Name(_) => { + if !checker + .semantic() + .resolve_call_path(call.func.as_ref()) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "exception"])) + { + return; + } + } + _ => return, } - if !logging::is_logger_candidate( - &call.func, - checker.semantic(), - &checker.settings.logger_objects, - ) { - return; + if exc_info_arg_is_falsey(call, checker) { + checker + .diagnostics + .push(Diagnostic::new(ExceptionWithoutExcInfo, call.range())); } +} - if call - .arguments +fn exc_info_arg_is_falsey(call: &ExprCall, checker: &mut Checker) -> bool { + 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())); - } } diff --git a/crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG007_LOG007.py.snap b/crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG007_LOG007.py.snap index df6cb6a004be0..a8df5679127e4 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG007_LOG007.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG007_LOG007.py.snap @@ -40,4 +40,13 @@ LOG007.py:10:1: LOG007 Use of `logging.exception` with falsy `exc_info` 12 | logger.info("foo", exc_info=False) # OK | +LOG007.py:17:1: LOG007 Use of `logging.exception` with falsy `exc_info` + | +15 | from logging import exception +16 | +17 | exception("foo", exc_info=False) # LOG007 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007 +18 | exception("foo", exc_info=True) # OK + | + diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs index 29b2b9711b602..a526981a46487 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs +++ b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs @@ -153,22 +153,36 @@ impl LoggingCallType { /// Check logging calls for violations. pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) { - let Expr::Attribute(ast::ExprAttribute { value: _, attr, .. }) = call.func.as_ref() else { - return; - }; - - let Some(logging_call_type) = LoggingCallType::from_attribute(attr.as_str()) else { - return; + // Determine the call type (e.g., `info` vs. `exception`) and the range of the attribute. + let (logging_call_type, range) = match call.func.as_ref() { + Expr::Attribute(ast::ExprAttribute { value: _, attr, .. }) => { + let Some(call_type) = LoggingCallType::from_attribute(attr.as_str()) else { + return; + }; + if !logging::is_logger_candidate( + &call.func, + checker.semantic(), + &checker.settings.logger_objects, + ) { + return; + } + (call_type, attr.range()) + } + Expr::Name(_) => { + let Some(call_path) = checker.semantic().resolve_call_path(call.func.as_ref()) else { + return; + }; + let ["logging", attribute] = call_path.as_slice() else { + return; + }; + let Some(call_type) = LoggingCallType::from_attribute(attribute) else { + return; + }; + (call_type, call.func.range()) + } + _ => return, }; - if !logging::is_logger_candidate( - &call.func, - checker.semantic(), - &checker.settings.logger_objects, - ) { - return; - } - // G001 - G004 let msg_pos = usize::from(matches!(logging_call_type, LoggingCallType::LogCall)); if let Some(format_arg) = call.arguments.find_argument("msg", msg_pos) { @@ -181,11 +195,11 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) { logging_call_type, LoggingCallType::LevelCall(LoggingLevel::Warn) ) { - let mut diagnostic = Diagnostic::new(LoggingWarn, attr.range()); + let mut diagnostic = Diagnostic::new(LoggingWarn, range); if checker.patch(diagnostic.kind.rule()) { diagnostic.set_fix(Fix::automatic(Edit::range_replacement( "warning".to_string(), - attr.range(), + range, ))); } checker.diagnostics.push(diagnostic); @@ -213,7 +227,7 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) { if checker.enabled(Rule::LoggingExcInfo) { checker .diagnostics - .push(Diagnostic::new(LoggingExcInfo, attr.range())); + .push(Diagnostic::new(LoggingExcInfo, range)); } } LoggingLevel::Exception => { diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G001.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G001.py.snap index 7e2391e7b35b5..f6e052cf559f7 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G001.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G001.py.snap @@ -83,6 +83,64 @@ G001.py:18:30: G001 Logging statement uses `str.format` 17 | current_app.logger.info("Hello {}".format("World!")) 18 | app.logger.log(logging.INFO, "Hello {}".format("World!")) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001 +19 | +20 | from logging import info, log + | + +G001.py:22:6: G001 Logging statement uses `str.format` + | +20 | from logging import info, log +21 | +22 | info("Hello {}".format("World!")) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001 +23 | log(logging.INFO, "Hello {}".format("World!")) +24 | info("Hello {}".format("World!")) + | + +G001.py:23:19: G001 Logging statement uses `str.format` + | +22 | info("Hello {}".format("World!")) +23 | log(logging.INFO, "Hello {}".format("World!")) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001 +24 | info("Hello {}".format("World!")) +25 | log(logging.INFO, msg="Hello {}".format("World!")) + | + +G001.py:24:6: G001 Logging statement uses `str.format` + | +22 | info("Hello {}".format("World!")) +23 | log(logging.INFO, "Hello {}".format("World!")) +24 | info("Hello {}".format("World!")) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001 +25 | log(logging.INFO, msg="Hello {}".format("World!")) +26 | log(level=logging.INFO, msg="Hello {}".format("World!")) + | + +G001.py:25:23: G001 Logging statement uses `str.format` + | +23 | log(logging.INFO, "Hello {}".format("World!")) +24 | info("Hello {}".format("World!")) +25 | log(logging.INFO, msg="Hello {}".format("World!")) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001 +26 | log(level=logging.INFO, msg="Hello {}".format("World!")) +27 | log(msg="Hello {}".format("World!"), level=logging.INFO) + | + +G001.py:26:29: G001 Logging statement uses `str.format` + | +24 | info("Hello {}".format("World!")) +25 | log(logging.INFO, msg="Hello {}".format("World!")) +26 | log(level=logging.INFO, msg="Hello {}".format("World!")) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001 +27 | log(msg="Hello {}".format("World!"), level=logging.INFO) + | + +G001.py:27:9: G001 Logging statement uses `str.format` + | +25 | log(logging.INFO, msg="Hello {}".format("World!")) +26 | log(level=logging.INFO, msg="Hello {}".format("World!")) +27 | log(msg="Hello {}".format("World!"), level=logging.INFO) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001 | diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G002.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G002.py.snap index ebe2ad34825a0..6ba827bafa5e2 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G002.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G002.py.snap @@ -15,6 +15,24 @@ G002.py:4:27: G002 Logging statement uses `%` 3 | logging.info("Hello %s" % "World!") 4 | logging.log(logging.INFO, "Hello %s" % "World!") | ^^^^^^^^^^^^^^^^^^^^^ G002 +5 | +6 | from logging import info, log + | + +G002.py:8:6: G002 Logging statement uses `%` + | +6 | from logging import info, log +7 | +8 | info("Hello %s" % "World!") + | ^^^^^^^^^^^^^^^^^^^^^ G002 +9 | log(logging.INFO, "Hello %s" % "World!") + | + +G002.py:9:19: G002 Logging statement uses `%` + | +8 | info("Hello %s" % "World!") +9 | log(logging.INFO, "Hello %s" % "World!") + | ^^^^^^^^^^^^^^^^^^^^^ G002 | diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G003.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G003.py.snap index 873b9e74f91da..494761d268587 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G003.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G003.py.snap @@ -15,6 +15,24 @@ G003.py:4:27: G003 Logging statement uses `+` 3 | logging.info("Hello" + " " + "World!") 4 | logging.log(logging.INFO, "Hello" + " " + "World!") | ^^^^^^^^^^^^^^^^^^^^^^^^ G003 +5 | +6 | from logging import info, log + | + +G003.py:8:6: G003 Logging statement uses `+` + | +6 | from logging import info, log +7 | +8 | info("Hello" + " " + "World!") + | ^^^^^^^^^^^^^^^^^^^^^^^^ G003 +9 | log(logging.INFO, "Hello" + " " + "World!") + | + +G003.py:9:19: G003 Logging statement uses `+` + | +8 | info("Hello" + " " + "World!") +9 | log(logging.INFO, "Hello" + " " + "World!") + | ^^^^^^^^^^^^^^^^^^^^^^^^ G003 | diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap index a2d3ef00f8417..5594145d4fc34 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap @@ -20,10 +20,28 @@ G004.py:5:27: G004 Logging statement uses f-string | G004.py:8:14: G004 Logging statement uses f-string - | -7 | _LOGGER = logging.getLogger() -8 | _LOGGER.info(f"{__name__}") - | ^^^^^^^^^^^^^ G004 - | + | + 7 | _LOGGER = logging.getLogger() + 8 | _LOGGER.info(f"{__name__}") + | ^^^^^^^^^^^^^ G004 + 9 | +10 | from logging import info + | + +G004.py:11:6: G004 Logging statement uses f-string + | +10 | from logging import info +11 | info(f"{name}") + | ^^^^^^^^^ G004 +12 | info(f"{__name__}") + | + +G004.py:12:6: G004 Logging statement uses f-string + | +10 | from logging import info +11 | info(f"{name}") +12 | info(f"{__name__}") + | ^^^^^^^^^^^^^ G004 + | diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G010.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G010.py.snap index f56206e88c036..df5ab9dfa124f 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G010.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G010.py.snap @@ -41,6 +41,7 @@ G010.py:8:8: G010 [*] Logging statement uses `warn` instead of `warning` 8 |+logger.warning("Hello world!") 9 9 | 10 10 | logging . warn("Hello World!") +11 11 | G010.py:10:11: G010 [*] Logging statement uses `warn` instead of `warning` | @@ -48,6 +49,8 @@ G010.py:10:11: G010 [*] Logging statement uses `warn` instead of `warning` 9 | 10 | logging . warn("Hello World!") | ^^^^ G010 +11 | +12 | from logging import warn, warning, exception | = help: Convert to `warn` @@ -57,5 +60,27 @@ G010.py:10:11: G010 [*] Logging statement uses `warn` instead of `warning` 9 9 | 10 |-logging . warn("Hello World!") 10 |+logging . warning("Hello World!") +11 11 | +12 12 | from logging import warn, warning, exception +13 13 | warn("foo") + +G010.py:13:1: G010 [*] Logging statement uses `warn` instead of `warning` + | +12 | from logging import warn, warning, exception +13 | warn("foo") + | ^^^^ G010 +14 | warning("foo") +15 | exception("foo") + | + = help: Convert to `warn` + +ℹ Fix +10 10 | logging . warn("Hello World!") +11 11 | +12 12 | from logging import warn, warning, exception +13 |-warn("foo") +14 13 | warning("foo") + 14 |+warning("foo") +15 15 | exception("foo") diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_1.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_1.py.snap index d35d9f2bd46ba..e0fff24c03a91 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_1.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_1.py.snap @@ -11,4 +11,14 @@ G101_1.py:6:9: G101 Logging statement uses an `extra` field that clashes with a 8 | ) | +G101_1.py:15:9: G101 Logging statement uses an `extra` field that clashes with a `LogRecord` field: `name` + | +13 | "Hello world!", +14 | extra={ +15 | "name": "foobar", + | ^^^^^^ G101 +16 | }, +17 | ) + | + diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_2.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_2.py.snap index 2ea3d56ded6ad..7e5e636f87afb 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_2.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G101_2.py.snap @@ -11,4 +11,14 @@ G101_2.py:6:9: G101 Logging statement uses an `extra` field that clashes with a 8 | ) | +G101_2.py:15:9: G101 Logging statement uses an `extra` field that clashes with a `LogRecord` field: `name` + | +13 | "Hello world!", +14 | extra=dict( +15 | name="foobar", + | ^^^^^^^^^^^^^ G101 +16 | ), +17 | ) + | + diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G201.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G201.py.snap index a4cbafa5d2356..34c469ae68bbd 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G201.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G201.py.snap @@ -21,4 +21,24 @@ G201.py:13:13: G201 Logging `.exception(...)` should be used instead of `.error( 15 | # OK | +G201.py:28:5: G201 Logging `.exception(...)` should be used instead of `.error(..., exc_info=True)` + | +26 | pass +27 | except: +28 | error("Hello World", exc_info=True) + | ^^^^^ G201 +29 | +30 | try: + | + +G201.py:33:5: G201 Logging `.exception(...)` should be used instead of `.error(..., exc_info=True)` + | +31 | pass +32 | except: +33 | error("Hello World", exc_info=sys.exc_info()) + | ^^^^^ G201 +34 | +35 | # OK + | + diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G202.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G202.py.snap index 9fc9a87608bec..17daeba81a893 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G202.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G202.py.snap @@ -21,4 +21,24 @@ G202.py:13:38: G202 Logging statement has redundant `exc_info` 15 | # OK | +G202.py:28:30: G202 Logging statement has redundant `exc_info` + | +26 | pass +27 | except: +28 | exception("Hello World", exc_info=True) + | ^^^^^^^^^^^^^ G202 +29 | +30 | try: + | + +G202.py:33:30: G202 Logging statement has redundant `exc_info` + | +31 | pass +32 | except: +33 | exception("Hello World", exc_info=sys.exc_info()) + | ^^^^^^^^^^^^^^^^^^^^^^^ G202 +34 | +35 | # OK + | + diff --git a/crates/ruff_linter/src/rules/pylint/rules/logging.rs b/crates/ruff_linter/src/rules/pylint/rules/logging.rs index ac1f47e68a19d..e101a5e235fc6 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/logging.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/logging.rs @@ -101,14 +101,33 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) { return; } - let Expr::Attribute(ast::ExprAttribute { attr, .. }) = call.func.as_ref() else { - return; + match call.func.as_ref() { + Expr::Attribute(ast::ExprAttribute { attr, .. }) => { + if LoggingLevel::from_attribute(attr).is_none() { + return; + }; + if !logging::is_logger_candidate( + &call.func, + checker.semantic(), + &checker.settings.logger_objects, + ) { + return; + } + } + Expr::Name(_) => { + let Some(call_path) = checker.semantic().resolve_call_path(call.func.as_ref()) else { + return; + }; + let ["logging", attribute] = call_path.as_slice() else { + return; + }; + if LoggingLevel::from_attribute(attribute).is_none() { + return; + }; + } + _ => return, }; - if LoggingLevel::from_attribute(attr.as_str()).is_none() { - return; - } - let Some(Expr::Constant(ast::ExprConstant { value: Constant::Str(ast::StringConstant { value, .. }), .. @@ -117,14 +136,6 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) { return; }; - if !logging::is_logger_candidate( - &call.func, - checker.semantic(), - &checker.settings.logger_objects, - ) { - return; - } - let Ok(summary) = CFormatSummary::try_from(value.as_str()) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1205_logging_too_many_args.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1205_logging_too_many_args.py.snap index cdbccdbd758fa..f202189a23a82 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1205_logging_too_many_args.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1205_logging_too_many_args.py.snap @@ -21,4 +21,24 @@ logging_too_many_args.py:5:1: PLE1205 Too many arguments for `logging` format st 7 | logging.warning("Hello %s", "World!") | +logging_too_many_args.py:29:1: PLE1205 Too many arguments for `logging` format string + | +27 | from logging import info, error, warning +28 | +29 | warning("Hello %s", "World!", "again") # [logging-too-many-args] + | ^^^^^^^ PLE1205 +30 | +31 | warning("Hello %s", "World!", "again", something="else") + | + +logging_too_many_args.py:31:1: PLE1205 Too many arguments for `logging` format string + | +29 | warning("Hello %s", "World!", "again") # [logging-too-many-args] +30 | +31 | warning("Hello %s", "World!", "again", something="else") + | ^^^^^^^ PLE1205 +32 | +33 | warning("Hello %s", "World!") + | + diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1206_logging_too_few_args.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1206_logging_too_few_args.py.snap index b46b420741c94..abaec32b88bd9 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1206_logging_too_few_args.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1206_logging_too_few_args.py.snap @@ -11,4 +11,14 @@ logging_too_few_args.py:3:1: PLE1206 Not enough arguments for `logging` format s 5 | # do not handle calls with kwargs (like pylint) | +logging_too_few_args.py:33:1: PLE1206 Not enough arguments for `logging` format string + | +31 | from logging import error, info, warning +32 | +33 | warning("Hello %s %s", "World!") # [logging-too-few-args] + | ^^^^^^^ PLE1206 +34 | +35 | # do not handle calls with kwargs (like pylint) + | + diff --git a/crates/ruff_linter/src/rules/tryceratops/helpers.rs b/crates/ruff_linter/src/rules/tryceratops/helpers.rs index 6a765ec0e3263..0e707c1e3924e 100644 --- a/crates/ruff_linter/src/rules/tryceratops/helpers.rs +++ b/crates/ruff_linter/src/rules/tryceratops/helpers.rs @@ -1,15 +1,15 @@ -use ruff_python_ast::{self as ast, Expr}; - use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; +use ruff_python_ast::{self as ast, Expr}; use ruff_python_semantic::analyze::logging; use ruff_python_semantic::SemanticModel; +use ruff_python_stdlib::logging::LoggingLevel; /// Collect `logging`-like calls from an AST. pub(super) struct LoggerCandidateVisitor<'a, 'b> { semantic: &'a SemanticModel<'b>, logger_objects: &'a [String], - pub(super) calls: Vec<&'b ast::ExprCall>, + pub(super) calls: Vec<(&'b ast::ExprCall, LoggingLevel)>, } impl<'a, 'b> LoggerCandidateVisitor<'a, 'b> { @@ -25,8 +25,27 @@ impl<'a, 'b> LoggerCandidateVisitor<'a, 'b> { impl<'a, 'b> Visitor<'b> for LoggerCandidateVisitor<'a, 'b> { fn visit_expr(&mut self, expr: &'b Expr) { if let Expr::Call(call) = expr { - if logging::is_logger_candidate(&call.func, self.semantic, self.logger_objects) { - self.calls.push(call); + match call.func.as_ref() { + Expr::Attribute(ast::ExprAttribute { attr, .. }) => { + if logging::is_logger_candidate(&call.func, self.semantic, self.logger_objects) + { + if let Some(logging_level) = LoggingLevel::from_attribute(attr) { + self.calls.push((call, logging_level)); + }; + } + } + Expr::Name(_) => { + if let Some(call_path) = self.semantic.resolve_call_path(call.func.as_ref()) { + if let ["logging", attribute] = call_path.as_slice() { + if let Some(logging_level) = LoggingLevel::from_attribute(attribute) { + { + self.calls.push((call, logging_level)); + } + } + } + } + } + _ => {} } } visitor::walk_expr(self, expr); diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs b/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs index 2d0404a50406a..fda19f79e8b58 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs @@ -1,9 +1,9 @@ -use ruff_python_ast::{self as ast, ExceptHandler, Expr}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::visitor::Visitor; +use ruff_python_ast::{self as ast, ExceptHandler}; use ruff_python_semantic::analyze::logging::exc_info; +use ruff_python_stdlib::logging::LoggingLevel; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -64,14 +64,12 @@ pub(crate) fn error_instead_of_exception(checker: &mut Checker, handlers: &[Exce visitor.visit_body(body); visitor.calls }; - for expr in calls { - if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = expr.func.as_ref() { - if attr == "error" { - if exc_info(&expr.arguments, checker.semantic()).is_none() { - checker - .diagnostics - .push(Diagnostic::new(ErrorInsteadOfException, expr.range())); - } + for (expr, logging_level) in calls { + if matches!(logging_level, LoggingLevel::Error) { + if exc_info(&expr.arguments, checker.semantic()).is_none() { + checker + .diagnostics + .push(Diagnostic::new(ErrorInsteadOfException, expr.range())); } } } diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs b/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs index 4ddf0009b5892..8b3fad0f082d9 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/verbose_log_message.rs @@ -4,6 +4,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; +use ruff_python_stdlib::logging::LoggingLevel; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -74,25 +75,23 @@ pub(crate) fn verbose_log_message(checker: &mut Checker, handlers: &[ExceptHandl visitor.calls }; - for expr in calls { - if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = expr.func.as_ref() { - if attr == "exception" { - // Collect all referenced names in the `logging.exception` call. - let names: Vec<&ast::ExprName> = { - let mut names = Vec::new(); - for arg in &expr.arguments.args { - let mut visitor = NameVisitor::default(); - visitor.visit_expr(arg); - names.extend(visitor.names); - } - names - }; - for expr in names { - if expr.id == target.as_str() { - checker - .diagnostics - .push(Diagnostic::new(VerboseLogMessage, expr.range())); - } + for (expr, logging_level) in calls { + if matches!(logging_level, LoggingLevel::Exception) { + // Collect all referenced names in the `logging.exception` call. + let names: Vec<&ast::ExprName> = { + let mut names = Vec::new(); + for arg in &expr.arguments.args { + let mut visitor = NameVisitor::default(); + visitor.visit_expr(arg); + names.extend(visitor.names); + } + names + }; + for expr in names { + if expr.id == target.as_str() { + checker + .diagnostics + .push(Diagnostic::new(VerboseLogMessage, expr.range())); } } } diff --git a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap index 4eb4aad7a1767..35b36493ec3a9 100644 --- a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap +++ b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap @@ -69,4 +69,21 @@ TRY400.py:49:13: TRY400 Use `logging.exception` instead of `logging.error` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400 | +TRY400.py:87:9: TRY400 Use `logging.exception` instead of `logging.error` + | +85 | a = 1 +86 | except Exception: +87 | error("Context message here") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400 +88 | +89 | if True: + | + +TRY400.py:90:13: TRY400 Use `logging.exception` instead of `logging.error` + | +89 | if True: +90 | error("Context message here") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400 + | + diff --git a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap index 7143686b8708c..3681b880d1f06 100644 --- a/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap +++ b/crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__verbose-log-message_TRY401.py.snap @@ -89,4 +89,92 @@ TRY401.py:53:47: TRY401 Redundant exception object included in `logging.exceptio | ^^ TRY401 | +TRY401.py:77:38: TRY401 Redundant exception object included in `logging.exception` call + | +75 | finish() +76 | except Exception as ex: +77 | exception(f"Found an error: {ex}") # TRY401 + | ^^ TRY401 + | + +TRY401.py:88:46: TRY401 Redundant exception object included in `logging.exception` call + | +86 | if True is False: +87 | for i in range(10): +88 | exception(f"Found an error: {bad} {good}") # TRY401 + | ^^^ TRY401 +89 | except IndexError as bad: +90 | exception(f"Found an error: {bad} {bad}") # TRY401 + | + +TRY401.py:90:38: TRY401 Redundant exception object included in `logging.exception` call + | +88 | exception(f"Found an error: {bad} {good}") # TRY401 +89 | except IndexError as bad: +90 | exception(f"Found an error: {bad} {bad}") # TRY401 + | ^^^ TRY401 +91 | except Exception as bad: +92 | exception(f"Found an error: {bad}") # TRY401 + | + +TRY401.py:90:44: TRY401 Redundant exception object included in `logging.exception` call + | +88 | exception(f"Found an error: {bad} {good}") # TRY401 +89 | except IndexError as bad: +90 | exception(f"Found an error: {bad} {bad}") # TRY401 + | ^^^ TRY401 +91 | except Exception as bad: +92 | exception(f"Found an error: {bad}") # TRY401 + | + +TRY401.py:92:38: TRY401 Redundant exception object included in `logging.exception` call + | +90 | exception(f"Found an error: {bad} {bad}") # TRY401 +91 | except Exception as bad: +92 | exception(f"Found an error: {bad}") # TRY401 + | ^^^ TRY401 +93 | exception(f"Found an error: {bad}") # TRY401 + | + +TRY401.py:93:38: TRY401 Redundant exception object included in `logging.exception` call + | +91 | except Exception as bad: +92 | exception(f"Found an error: {bad}") # TRY401 +93 | exception(f"Found an error: {bad}") # TRY401 + | ^^^ TRY401 +94 | +95 | if True: + | + +TRY401.py:96:42: TRY401 Redundant exception object included in `logging.exception` call + | +95 | if True: +96 | exception(f"Found an error: {bad}") # TRY401 + | ^^^ TRY401 + | + +TRY401.py:103:40: TRY401 Redundant exception object included in `logging.exception` call + | +101 | ... +102 | except Exception as ex: +103 | exception(f"Logging an error: {ex}") # TRY401 + | ^^ TRY401 + | + +TRY401.py:110:46: TRY401 Redundant exception object included in `logging.exception` call + | +108 | ... +109 | except Exception as ex: +110 | exception("Logging an error: " + str(ex)) # TRY401 + | ^^ TRY401 + | + +TRY401.py:117:40: TRY401 Redundant exception object included in `logging.exception` call + | +115 | ... +116 | except Exception as ex: +117 | exception("Logging an error:", ex) # TRY401 + | ^^ TRY401 + | +