From 96211b28bb0e392db4086ce7c1cc5492ee433bb2 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sun, 30 Mar 2025 11:26:33 +0200 Subject: [PATCH 1/2] [feat] Detect missing formatting args when the string is not interpolated too The original decision was taken in db8b3a47be during implementation with a different rational ("If no args were supplied, then all format strings are valid don't check any further.") and was not discussed again when the comment was updated in #2713. I think the new behavior make sense especially considering that the primer shows 4 false negative in home-assistant. Co-authored-by: Alex Prabhat Bara --- doc/whatsnew/fragments/9999.false_negative | 5 +++++ pylint/checkers/logging.py | 5 ----- tests/functional/l/logging/logging_too_few_args.txt | 6 ------ ...too_few_args.py => logging_too_few_args_new_style.py} | 5 ++++- ...too_few_args.rc => logging_too_few_args_new_style.rc} | 0 .../l/logging/logging_too_few_args_new_style.txt | 9 +++++++++ .../l/logging/logging_too_few_args_old_style.py | 6 ++++++ .../l/logging/logging_too_few_args_old_style.rc | 2 ++ .../l/logging/logging_too_few_args_old_style.txt | 2 ++ 9 files changed, 28 insertions(+), 12 deletions(-) create mode 100644 doc/whatsnew/fragments/9999.false_negative delete mode 100644 tests/functional/l/logging/logging_too_few_args.txt rename tests/functional/l/logging/{logging_too_few_args.py => logging_too_few_args_new_style.py} (67%) rename tests/functional/l/logging/{logging_too_few_args.rc => logging_too_few_args_new_style.rc} (100%) create mode 100644 tests/functional/l/logging/logging_too_few_args_new_style.txt create mode 100644 tests/functional/l/logging/logging_too_few_args_old_style.py create mode 100644 tests/functional/l/logging/logging_too_few_args_old_style.rc create mode 100644 tests/functional/l/logging/logging_too_few_args_old_style.txt diff --git a/doc/whatsnew/fragments/9999.false_negative b/doc/whatsnew/fragments/9999.false_negative new file mode 100644 index 0000000000..de5a393d6a --- /dev/null +++ b/doc/whatsnew/fragments/9999.false_negative @@ -0,0 +1,5 @@ +We now raise a ``logging-too-few-args`` for format string with no +interpolation arguments at all (i.e. for something like ``logging.debug("Awaiting process %s")`` +or ``logging.debug("Awaiting process {pid}")``). Previously we did not raise for such case. + +Closes #9999 diff --git a/pylint/checkers/logging.py b/pylint/checkers/logging.py index d057c78ecb..583143bc60 100644 --- a/pylint/checkers/logging.py +++ b/pylint/checkers/logging.py @@ -326,11 +326,6 @@ def _check_format_string(self, node: nodes.Call, format_arg: Literal[0, 1]) -> N format_arg: Index of the format string in the node arguments. """ num_args = _count_supplied_tokens(node.args[format_arg + 1 :]) - if not num_args: - # If no args were supplied the string is not interpolated and can contain - # formatting characters - it's used verbatim. Don't check any further. - return - format_string = node.args[format_arg].value required_num_args = 0 if isinstance(format_string, bytes): diff --git a/tests/functional/l/logging/logging_too_few_args.txt b/tests/functional/l/logging/logging_too_few_args.txt deleted file mode 100644 index 4545fd0d11..0000000000 --- a/tests/functional/l/logging/logging_too_few_args.txt +++ /dev/null @@ -1,6 +0,0 @@ -logging-too-few-args:5:0:5:26::Not enough arguments for logging format string:UNDEFINED -logging-too-few-args:6:0:6:28::Not enough arguments for logging format string:UNDEFINED -logging-too-few-args:7:0:7:50::Not enough arguments for logging format string:UNDEFINED -logging-too-few-args:8:0:8:32::Not enough arguments for logging format string:UNDEFINED -logging-too-few-args:9:0:9:42::Not enough arguments for logging format string:UNDEFINED -logging-too-few-args:10:0:10:43::Not enough arguments for logging format string:UNDEFINED diff --git a/tests/functional/l/logging/logging_too_few_args.py b/tests/functional/l/logging/logging_too_few_args_new_style.py similarity index 67% rename from tests/functional/l/logging/logging_too_few_args.py rename to tests/functional/l/logging/logging_too_few_args_new_style.py index fc00619866..77097e2c22 100644 --- a/tests/functional/l/logging/logging_too_few_args.py +++ b/tests/functional/l/logging/logging_too_few_args_new_style.py @@ -1,9 +1,12 @@ -"""Tests for logging-too-few-args""" +"""Tests for logging-too-few-args new style""" import logging logging.error("{}, {}", 1) # [logging-too-few-args] logging.error("{0}, {1}", 1) # [logging-too-few-args] +logging.error("{}") # [logging-too-few-args] +logging.error("{0}") # [logging-too-few-args] +logging.error("{named}") # [logging-too-few-args] logging.error("{named1}, {named2}", {"named1": 1}) # [logging-too-few-args] logging.error("{0}, {named}", 1) # [logging-too-few-args] logging.error("{}, {named}", {"named": 1}) # [logging-too-few-args] diff --git a/tests/functional/l/logging/logging_too_few_args.rc b/tests/functional/l/logging/logging_too_few_args_new_style.rc similarity index 100% rename from tests/functional/l/logging/logging_too_few_args.rc rename to tests/functional/l/logging/logging_too_few_args_new_style.rc diff --git a/tests/functional/l/logging/logging_too_few_args_new_style.txt b/tests/functional/l/logging/logging_too_few_args_new_style.txt new file mode 100644 index 0000000000..219f455a2e --- /dev/null +++ b/tests/functional/l/logging/logging_too_few_args_new_style.txt @@ -0,0 +1,9 @@ +logging-too-few-args:5:0:5:26::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:6:0:6:28::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:7:0:7:19::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:8:0:8:20::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:9:0:9:24::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:10:0:10:50::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:11:0:11:32::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:12:0:12:42::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:13:0:13:43::Not enough arguments for logging format string:UNDEFINED diff --git a/tests/functional/l/logging/logging_too_few_args_old_style.py b/tests/functional/l/logging/logging_too_few_args_old_style.py new file mode 100644 index 0000000000..4455bc9f7e --- /dev/null +++ b/tests/functional/l/logging/logging_too_few_args_old_style.py @@ -0,0 +1,6 @@ +"""Tests for logging-too-few-args old style""" + +import logging + +logging.error("%s, %s", 1) # [logging-too-few-args] +logging.debug("Sisyphus table %s: sleep") # [logging-too-few-args] diff --git a/tests/functional/l/logging/logging_too_few_args_old_style.rc b/tests/functional/l/logging/logging_too_few_args_old_style.rc new file mode 100644 index 0000000000..7df00b851a --- /dev/null +++ b/tests/functional/l/logging/logging_too_few_args_old_style.rc @@ -0,0 +1,2 @@ +[LOGGING] +logging-format-style=old diff --git a/tests/functional/l/logging/logging_too_few_args_old_style.txt b/tests/functional/l/logging/logging_too_few_args_old_style.txt new file mode 100644 index 0000000000..2d315e2963 --- /dev/null +++ b/tests/functional/l/logging/logging_too_few_args_old_style.txt @@ -0,0 +1,2 @@ +logging-too-few-args:5:0:5:26::Not enough arguments for logging format string:UNDEFINED +logging-too-few-args:6:0:6:41::Not enough arguments for logging format string:UNDEFINED From f7200cf8e1dddfbc29c90fc561b955f122e7bd71 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sun, 30 Mar 2025 11:26:33 +0200 Subject: [PATCH 2/2] Fix required because logging are tested elsewhere --- tests/config/data/logging_format_interpolation_style.py | 1 - .../functional/l/logging/logging_format_interpolation_style.py | 1 - tests/functional/s/string/string_log_formatting.py | 2 +- tests/functional/s/string/string_log_formatting.txt | 1 + 4 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/config/data/logging_format_interpolation_style.py b/tests/config/data/logging_format_interpolation_style.py index 04b357abd6..74bdfef388 100644 --- a/tests/config/data/logging_format_interpolation_style.py +++ b/tests/config/data/logging_format_interpolation_style.py @@ -5,7 +5,6 @@ import logging logging.error("constant string") -logging.error("{}") logging.error("{}", 1) logging.error("{0}", 1) logging.error("{named}", {"named": 1}) diff --git a/tests/functional/l/logging/logging_format_interpolation_style.py b/tests/functional/l/logging/logging_format_interpolation_style.py index f568de5fda..49946a9f0f 100644 --- a/tests/functional/l/logging/logging_format_interpolation_style.py +++ b/tests/functional/l/logging/logging_format_interpolation_style.py @@ -3,7 +3,6 @@ import logging logging.error("constant string") -logging.error("{}") logging.error("{}", 1) logging.error("{0}", 1) logging.error("{named}", {"named": 1}) diff --git a/tests/functional/s/string/string_log_formatting.py b/tests/functional/s/string/string_log_formatting.py index 3bed980f72..6a4912802a 100644 --- a/tests/functional/s/string/string_log_formatting.py +++ b/tests/functional/s/string/string_log_formatting.py @@ -22,7 +22,7 @@ def pprint(): logging.info(1) logging.info(True) logging.info('') - logging.info('%s%') + logging.info('%s%') # [logging-format-truncated] logging.info('%s', '') logging.info('%s%%', '') logging.info('%s%s', '', '') diff --git a/tests/functional/s/string/string_log_formatting.txt b/tests/functional/s/string/string_log_formatting.txt index 21eb4255a8..6fdf5debb3 100644 --- a/tests/functional/s/string/string_log_formatting.txt +++ b/tests/functional/s/string/string_log_formatting.txt @@ -4,3 +4,4 @@ logging-format-truncated:16:4:16:27:pprint:Logging format string ends in middle logging-too-few-args:17:4:17:28:pprint:Not enough arguments for logging format string:UNDEFINED logging-unsupported-format:18:4:18:32:pprint:Unsupported logging format character 'y' (0x79) at index 3:UNDEFINED logging-too-many-args:19:4:19:36:pprint:Too many arguments for logging format string:HIGH +logging-format-truncated:25:4:25:23:pprint:Logging format string ends in middle of conversion specifier:UNDEFINED