Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add documentation to flake8-logging-format rules (#5417)

Browse Source 
## Summary

Completes the documentation for the `flake8-logging-format` rules.
Related to #2646.

I included both the `flake8-logging-format` recommendation to use the
`extra` keyword and the Pylint recommendation to pass format values as
parameters so that formatting is done lazily, as #970 suggests the
Pylint logging rules are covered by this ruleset. Using lazy formatting
via parameters is probably more common than avoiding formatting entirely
in favour of the `extra` argument, regardless.

## Test Plan

`python scripts/check_docs_formatted.py`
This commit is contained in:
Tom Kuson
2023-06-29 02:30:11 +01:00
committed by GitHub
parent 0e89c94947
commit 5aa2a90e17
4 changed files with 370 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap
View File

@@ -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!"))

2
crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap
View File

@@ -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={

2
crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap
View File

@@ -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(

361
crates/ruff/src/rules/flake8_logging_format/violations.rs
View File

@@ -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;