diff --git a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap index ce85d609df..e1ed539d35 100644 --- a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap +++ b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/flake8_logging_format/mod.rs --- -G001.py:4:14: G001 Logging statement uses `string.format()` +G001.py:4:14: G001 Logging statement uses `str.format` | 2 | import logging as foo 3 | @@ -11,7 +11,7 @@ G001.py:4:14: G001 Logging statement uses `string.format()` 6 | foo.info("Hello {}".format("World!")) | -G001.py:5:27: G001 Logging statement uses `string.format()` +G001.py:5:27: G001 Logging statement uses `str.format` | 4 | logging.info("Hello {}".format("World!")) 5 | logging.log(logging.INFO, "Hello {}".format("World!")) @@ -20,7 +20,7 @@ G001.py:5:27: G001 Logging statement uses `string.format()` 7 | logging.log(logging.INFO, msg="Hello {}".format("World!")) | -G001.py:6:10: G001 Logging statement uses `string.format()` +G001.py:6:10: G001 Logging statement uses `str.format` | 4 | logging.info("Hello {}".format("World!")) 5 | logging.log(logging.INFO, "Hello {}".format("World!")) @@ -30,7 +30,7 @@ G001.py:6:10: G001 Logging statement uses `string.format()` 8 | logging.log(level=logging.INFO, msg="Hello {}".format("World!")) | -G001.py:7:31: G001 Logging statement uses `string.format()` +G001.py:7:31: G001 Logging statement uses `str.format` | 5 | logging.log(logging.INFO, "Hello {}".format("World!")) 6 | foo.info("Hello {}".format("World!")) @@ -40,7 +40,7 @@ G001.py:7:31: G001 Logging statement uses `string.format()` 9 | logging.log(msg="Hello {}".format("World!"), level=logging.INFO) | -G001.py:8:37: G001 Logging statement uses `string.format()` +G001.py:8:37: G001 Logging statement uses `str.format` | 6 | foo.info("Hello {}".format("World!")) 7 | logging.log(logging.INFO, msg="Hello {}".format("World!")) @@ -49,7 +49,7 @@ G001.py:8:37: G001 Logging statement uses `string.format()` 9 | logging.log(msg="Hello {}".format("World!"), level=logging.INFO) | -G001.py:9:17: G001 Logging statement uses `string.format()` +G001.py:9:17: G001 Logging statement uses `str.format` | 7 | logging.log(logging.INFO, msg="Hello {}".format("World!")) 8 | logging.log(level=logging.INFO, msg="Hello {}".format("World!")) @@ -59,7 +59,7 @@ G001.py:9:17: G001 Logging statement uses `string.format()` 11 | # Flask support | -G001.py:16:31: G001 Logging statement uses `string.format()` +G001.py:16:31: G001 Logging statement uses `str.format` | 14 | from flask import current_app as app 15 | @@ -69,7 +69,7 @@ G001.py:16:31: G001 Logging statement uses `string.format()` 18 | app.logger.log(logging.INFO, "Hello {}".format("World!")) | -G001.py:17:25: G001 Logging statement uses `string.format()` +G001.py:17:25: G001 Logging statement uses `str.format` | 16 | flask.current_app.logger.info("Hello {}".format("World!")) 17 | current_app.logger.info("Hello {}".format("World!")) @@ -77,7 +77,7 @@ G001.py:17:25: G001 Logging statement uses `string.format()` 18 | app.logger.log(logging.INFO, "Hello {}".format("World!")) | -G001.py:18:30: G001 Logging statement uses `string.format()` +G001.py:18:30: G001 Logging statement uses `str.format` | 16 | flask.current_app.logger.info("Hello {}".format("World!")) 17 | current_app.logger.info("Hello {}".format("World!")) diff --git a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap index 4bdc1ccbd7..ba7d9daa0d 100644 --- a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap +++ b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/flake8_logging_format/mod.rs --- -G101_1.py:6:9: G101 Logging statement uses an extra field that clashes with a LogRecord field: `name` +G101_1.py:6:9: G101 Logging statement uses an `extra` field that clashes with a `LogRecord` field: `name` | 4 | "Hello world!", 5 | extra={ diff --git a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap index b1084f13a9..0db9761fbc 100644 --- a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap +++ b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/flake8_logging_format/mod.rs --- -G101_2.py:6:9: G101 Logging statement uses an extra field that clashes with a LogRecord field: `name` +G101_2.py:6:9: G101 Logging statement uses an `extra` field that clashes with a `LogRecord` field: `name` | 4 | "Hello world!", 5 | extra=dict( diff --git a/crates/ruff/src/rules/flake8_logging_format/violations.rs b/crates/ruff/src/rules/flake8_logging_format/violations.rs index 622c9b83ec..3bada9ec59 100644 --- a/crates/ruff/src/rules/flake8_logging_format/violations.rs +++ b/crates/ruff/src/rules/flake8_logging_format/violations.rs @@ -1,16 +1,130 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, Violation}; use ruff_macros::{derive_message_formats, violation}; +/// ## What it does +/// Checks for uses of `str.format` to format logging messages. +/// +/// ## Why is this bad? +/// The `logging` module provides a mechanism for passing additional values to +/// be logged using the `extra` keyword argument. This is more consistent, more +/// efficient, and less error-prone than formatting the string directly. +/// +/// Using `str.format` to format a logging message requires that Python eagerly +/// format the string, even if the logging statement is never executed (e.g., +/// if the log level is above the level of the logging statement), whereas +/// using the `extra` keyword argument defers formatting until required. +/// +/// Additionally, the use of `extra` will ensure that the values are made +/// available to all handlers, which can then be configured to log the values +/// in a consistent manner. +/// +/// As an alternative to `extra`, passing values as arguments to the logging +/// method can also be used to defer string formatting until required. +/// +/// ## Example +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("{} - Something happened".format(user)) +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(user_id)s - %(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("Something happened", extra={"user_id": user}) +/// ``` +/// +/// Or: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("%s - Something happened", user) +/// ``` +/// +/// ## References +/// - [Python documentation: `logging`](https://docs.python.org/3/library/logging.html) +/// - [Python documentation: Optimization](https://docs.python.org/3/howto/logging.html#optimization) #[violation] pub struct LoggingStringFormat; impl Violation for LoggingStringFormat { #[derive_message_formats] fn message(&self) -> String { - format!("Logging statement uses `string.format()`") + format!("Logging statement uses `str.format`") } } +/// ## What it does +/// Checks for uses of `printf`-style format strings to format logging +/// messages. +/// +/// ## Why is this bad? +/// The `logging` module provides a mechanism for passing additional values to +/// be logged using the `extra` keyword argument. This is more consistent, more +/// efficient, and less error-prone than formatting the string directly. +/// +/// Using `printf`-style format strings to format a logging message requires +/// that Python eagerly format the string, even if the logging statement is +/// never executed (e.g., if the log level is above the level of the logging +/// statement), whereas using the `extra` keyword argument defers formatting +/// until required. +/// +/// Additionally, the use of `extra` will ensure that the values are made +/// available to all handlers, which can then be configured to log the values +/// in a consistent manner. +/// +/// As an alternative to `extra`, passing values as arguments to the logging +/// method can also be used to defer string formatting until required. +/// +/// ## Example +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("%s - Something happened" % user) +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(user_id)s - %(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("Something happened", extra=dict(user_id=user)) +/// ``` +/// +/// Or: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("%s - Something happened", user) +/// ``` +/// +/// ## References +/// - [Python documentation: `logging`](https://docs.python.org/3/library/logging.html) +/// - [Python documentation: Optimization](https://docs.python.org/3/howto/logging.html#optimization) #[violation] pub struct LoggingPercentFormat; @@ -21,6 +135,63 @@ impl Violation for LoggingPercentFormat { } } +/// ## What it does +/// Checks for uses string concatenation via the `+` operator to format logging +/// messages. +/// +/// ## Why is this bad? +/// The `logging` module provides a mechanism for passing additional values to +/// be logged using the `extra` keyword argument. This is more consistent, more +/// efficient, and less error-prone than formatting the string directly. +/// +/// Using concatenation to format a logging message requires that Python +/// eagerly format the string, even if the logging statement is never executed +/// (e.g., if the log level is above the level of the logging statement), +/// whereas using the `extra` keyword argument defers formatting until required. +/// +/// Additionally, the use of `extra` will ensure that the values are made +/// available to all handlers, which can then be configured to log the values +/// in a consistent manner. +/// +/// As an alternative to `extra`, passing values as arguments to the logging +/// method can also be used to defer string formatting until required. +/// +/// ## Example +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info(user + " - Something happened") +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(user_id)s - %(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("Something happened", extra=dict(user_id=user)) +/// ``` +/// +/// Or: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("%s - Something happened", user) +/// ``` +/// +/// ## References +/// - [Python documentation: `logging`](https://docs.python.org/3/library/logging.html) +/// - [Python documentation: Optimization](https://docs.python.org/3/howto/logging.html#optimization) #[violation] pub struct LoggingStringConcat; @@ -31,6 +202,62 @@ impl Violation for LoggingStringConcat { } } +/// ## What it does +/// Checks for uses of f-strings to format logging messages. +/// +/// ## Why is this bad? +/// The `logging` module provides a mechanism for passing additional values to +/// be logged using the `extra` keyword argument. This is more consistent, more +/// efficient, and less error-prone than formatting the string directly. +/// +/// Using f-strings to format a logging message requires that Python eagerly +/// format the string, even if the logging statement is never executed (e.g., +/// if the log level is above the level of the logging statement), whereas +/// using the `extra` keyword argument defers formatting until required. +/// +/// Additionally, the use of `extra` will ensure that the values are made +/// available to all handlers, which can then be configured to log the values +/// in a consistent manner. +/// +/// As an alternative to `extra`, passing values as arguments to the logging +/// method can also be used to defer string formatting until required. +/// +/// ## Example +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info(f"{user} - Something happened") +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(user_id)s - %(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("Something happened", extra=dict(user_id=user)) +/// ``` +/// +/// Or: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(message)s", level=logging.INFO) +/// +/// user = "Maria" +/// +/// logging.info("%s - Something happened", user) +/// ``` +/// +/// ## References +/// - [Python documentation: `logging`](https://docs.python.org/3/library/logging.html) +/// - [Python documentation: Optimization](https://docs.python.org/3/howto/logging.html#optimization) #[violation] pub struct LoggingFString; @@ -41,6 +268,31 @@ impl Violation for LoggingFString { } } +/// ## What it does +/// Checks for uses of `logging.warn` and `logging.Logger.warn`. +/// +/// ## Why is this bad? +/// `logging.warn` and `logging.Logger.warn` are deprecated in favor of +/// `logging.warning` and `logging.Logger.warning`, which are functionally +/// equivalent. +/// +/// ## Example +/// ```python +/// import logging +/// +/// logging.warn("Something happened") +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// logging.warning("Something happened") +/// ``` +/// +/// ## References +/// - [Python documentation: `logging.warning`](https://docs.python.org/3/library/logging.html#logging.warning) +/// - [Python documentation: `logging.Logger.warning`](https://docs.python.org/3/library/logging.html#logging.Logger.warning) #[violation] pub struct LoggingWarn; @@ -55,6 +307,43 @@ impl AlwaysAutofixableViolation for LoggingWarn { } } +/// ## What it does +/// Checks for `extra` keywords in logging statements that clash with +/// `LogRecord` attributes. +/// +/// ## Why is this bad? +/// The `logging` module provides a mechanism for passing additional values to +/// be logged using the `extra` keyword argument. These values are then passed +/// to the `LogRecord` constructor. +/// +/// Providing a value via `extra` that clashes with one of the attributes of +/// the `LogRecord` constructor will raise a `KeyError` when the `LogRecord` is +/// constructed. +/// +/// ## Example +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(name) - %(message)s", level=logging.INFO) +/// +/// username = "Maria" +/// +/// logging.info("Something happened", extra=dict(name=username)) +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// logging.basicConfig(format="%(user_id)s - %(message)s", level=logging.INFO) +/// +/// username = "Maria" +/// +/// logging.info("Something happened", extra=dict(user=username)) +/// ``` +/// +/// ## References +/// - [Python documentation: LogRecord attributes](https://docs.python.org/3/library/logging.html#logrecord-attributes) #[violation] pub struct LoggingExtraAttrClash(pub String); @@ -63,11 +352,44 @@ impl Violation for LoggingExtraAttrClash { fn message(&self) -> String { let LoggingExtraAttrClash(key) = self; format!( - "Logging statement uses an extra field that clashes with a LogRecord field: `{key}`" + "Logging statement uses an `extra` field that clashes with a `LogRecord` field: `{key}`" ) } } +/// ## What it does +/// Checks for uses of `logging.error` that pass `exc_info=True`. +/// +/// ## Why is this bad? +/// Calling `logging.error` with `exc_info=True` is equivalent to calling +/// `logging.exception`. Using `logging.exception` is more concise, more +/// readable, and conveys the intent of the logging statement more clearly. +/// +/// ## Example +/// ```python +/// import logging +/// +/// try: +/// ... +/// except ValueError: +/// logging.error("Exception occurred", exc_info=True) +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// try: +/// ... +/// except ValueError: +/// logging.exception("Exception occurred") +/// ``` +/// +/// ## References +/// - [Python documentation: `logging.exception`](https://docs.python.org/3/library/logging.html#logging.exception) +/// - [Python documentation: `exception`](https://docs.python.org/3/library/logging.html#logging.Logger.exception) +/// - [Python documentation: `logging.error`](https://docs.python.org/3/library/logging.html#logging.error) +/// - [Python documentation: `error`](https://docs.python.org/3/library/logging.html#logging.Logger.error) #[violation] pub struct LoggingExcInfo; @@ -78,6 +400,41 @@ impl Violation for LoggingExcInfo { } } +/// ## What it does +/// Checks for redundant `exc_info` keyword arguments in logging statements. +/// +/// ## Why is this bad? +/// `exc_info` is `True` by default for `logging.exception`, and `False` by +/// default for `logging.error`. +/// +/// Passing `exc_info=True` to `logging.exception` calls is redundant, as is +/// passing `exc_info=False` to `logging.error` calls. +/// +/// ## Example +/// ```python +/// import logging +/// +/// try: +/// ... +/// except ValueError: +/// logging.exception("Exception occurred", exc_info=True) +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// try: +/// ... +/// except ValueError: +/// logging.exception("Exception occurred") +/// ``` +/// +/// ## References +/// - [Python documentation: `logging.exception`](https://docs.python.org/3/library/logging.html#logging.exception) +/// - [Python documentation: `exception`](https://docs.python.org/3/library/logging.html#logging.Logger.exception) +/// - [Python documentation: `logging.error`](https://docs.python.org/3/library/logging.html#logging.error) +/// - [Python documentation: `error`](https://docs.python.org/3/library/logging.html#logging.Logger.error) #[violation] pub struct LoggingRedundantExcInfo;