Skip to content

Commit

Permalink
Fix logging-fstring-interpolation false positive (#7846) (#7854)
Browse files Browse the repository at this point in the history
Co-authored-by: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
(cherry picked from commit 978d1ab)

Co-authored-by: Dani Alcala <112832187+clavedeluna@users.noreply.github.com>
  • Loading branch information
github-actions[bot] and clavedeluna committed Nov 27, 2022
1 parent 86b8c64 commit ff73282
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 6 deletions.
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/4984.false_positive
@@ -0,0 +1,3 @@
Fix ``logging-fstring-interpolation`` false positive raised when logging and f-string with ``%s`` formatting.

Closes #4984
30 changes: 24 additions & 6 deletions pylint/checkers/logging.py
Expand Up @@ -105,6 +105,8 @@
"warning",
}

MOST_COMMON_FORMATTING = frozenset(["%s", "%d", "%f", "%r"])


def is_method_call(
func: bases.BoundMethod, types: tuple[str, ...] = (), methods: tuple[str, ...] = ()
Expand Down Expand Up @@ -240,8 +242,9 @@ def _check_log_method(self, node: nodes.Call, name: str) -> None:
else:
return

if isinstance(node.args[format_pos], nodes.BinOp):
binop = node.args[format_pos]
format_arg = node.args[format_pos]
if isinstance(format_arg, nodes.BinOp):
binop = format_arg
emit = binop.op == "%"
if binop.op == "+":
total_number_of_strings = sum(
Expand All @@ -256,11 +259,13 @@ def _check_log_method(self, node: nodes.Call, name: str) -> None:
node=node,
args=(self._helper_string(node),),
)
elif isinstance(node.args[format_pos], nodes.Call):
self._check_call_func(node.args[format_pos])
elif isinstance(node.args[format_pos], nodes.Const):
elif isinstance(format_arg, nodes.Call):
self._check_call_func(format_arg)
elif isinstance(format_arg, nodes.Const):
self._check_format_string(node, format_pos)
elif isinstance(node.args[format_pos], nodes.JoinedStr):
elif isinstance(format_arg, nodes.JoinedStr):
if str_formatting_in_f_string(format_arg):
return
self.add_message(
"logging-fstring-interpolation",
node=node,
Expand Down Expand Up @@ -393,5 +398,18 @@ def _count_supplied_tokens(args: list[nodes.NodeNG]) -> int:
return sum(1 for arg in args if not isinstance(arg, nodes.Keyword))


def str_formatting_in_f_string(node: nodes.JoinedStr) -> bool:
"""Determine whether the node represents an f-string with string formatting.
For example: `f'Hello %s'`
"""
# Check "%" presence first for performance.
return any(
"%" in val.value and any(x in val.value for x in MOST_COMMON_FORMATTING)
for val in node.values
if isinstance(val, nodes.Const)
)


def register(linter: PyLinter) -> None:
linter.register_checker(LoggingChecker(linter))
Expand Up @@ -3,3 +3,9 @@

VAR = "string"
logging.error(f"{VAR}") # [logging-fstring-interpolation]

WORLD = "world"
logging.error(f'Hello {WORLD}') # [logging-fstring-interpolation]

logging.error(f'Hello %s', 'World!') # [f-string-without-interpolation]
logging.error(f'Hello %d', 1) # [f-string-without-interpolation]
@@ -1 +1,4 @@
logging-fstring-interpolation:5:0:5:23::Use lazy % formatting in logging functions:UNDEFINED
logging-fstring-interpolation:8:0:8:31::Use lazy % formatting in logging functions:UNDEFINED
f-string-without-interpolation:10:14:10:25::Using an f-string that does not have any interpolated variables:UNDEFINED
f-string-without-interpolation:11:14:11:25::Using an f-string that does not have any interpolated variables:UNDEFINED

0 comments on commit ff73282

Please sign in to comment.