Skip to content

Commit

Permalink
Allow f-strings as a valid logging string formatting choice
Browse files Browse the repository at this point in the history
Closes #2395
  • Loading branch information
AWhetter authored and PCManticore committed Sep 10, 2019
1 parent 527db31 commit eeca6cf
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 59 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ Release date: TBA
Caused by #1164. It means if you do a partial old_names for a message definition an exception will tell you that you
must rename the associated identification.

* Allow the choice of f-strings as a valid way of formatting logging strings.

Closes #2395


What's New in Pylint 2.3.0?
===========================
Expand Down
8 changes: 8 additions & 0 deletions doc/whatsnew/2.4.rst
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,11 @@ would mean that `no-value-for-parameter` would not be raised for::
@given(img=arrays(dtype=np.float32, shape=(3, 3, 3, 3)))
def test_image(img):
...

* Allow the option of f-strings as a valid logging string formatting method.

`logging-fstring--interpolation` has been merged into
`logging-format-interpolation` to allow the `logging-format-style` option
to control which logging string format style is valid.
To allow this, a new `fstr` value is valid for the `logging-format-style`
option.
51 changes: 30 additions & 21 deletions pylint/checkers/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,12 @@
"http://www.python.org/dev/peps/pep-0282/.",
),
"W1202": (
"Use % formatting in logging functions and pass the % "
"parameters as arguments",
"Use %s formatting in logging functions%s",
"logging-format-interpolation",
"Used when a logging statement has a call form of "
'"logging.<logging method>(format_string.format(format_args...))"'
". Such calls should use % formatting instead, but leave "
"interpolation to the logging function by passing the parameters "
"as arguments.",
),
"W1203": (
"Use % formatting in logging functions and pass the % "
"parameters as arguments",
"logging-fstring-interpolation",
"Used when a logging statement has a call form of "
'"logging.method(f"..."))"'
". Such calls should use % formatting instead, but leave "
"interpolation to the logging function by passing the parameters "
"as arguments.",
'"logging.<logging method>(<string formatting>)".'
" with invalid string formatting. "
"Use another way for format the string instead.",
),
"E1200": (
"Unsupported logging format character %r (%#02x) at index %d",
Expand Down Expand Up @@ -139,10 +127,11 @@ class LoggingChecker(checkers.BaseChecker):
{
"default": "old",
"type": "choice",
"metavar": "<old (%) or new ({)>",
"choices": ["old", "new"],
"metavar": "<old (%) or new ({) or fstr (f'')>",
"choices": ["old", "new", "fstr"],
"help": "Format style used to check logging format string. "
"`old` means using % formatting, while `new` is for `{}` formatting.",
"`old` means using % formatting, `new` is for `{}` formatting,"
"and `fstr` is for f-strings.",
},
),
)
Expand All @@ -156,6 +145,13 @@ def visit_module(self, node): # pylint: disable=unused-argument
logging_mods = self.config.logging_modules

self._format_style = self.config.logging_format_style
format_styles = {"old": "%", "new": "{", "fstr": "f-string"}
format_style_help = ""
if self._format_style == "old":
format_style_help = " and pass the % parameters as arguments"

self._format_style_args = (format_styles[self._format_style], format_style_help)

self._logging_modules = set(logging_mods)
self._from_imports = {}
for logging_mod in logging_mods:
Expand Down Expand Up @@ -251,7 +247,12 @@ def _check_log_method(self, node, name):
elif isinstance(
node.args[format_pos], (astroid.FormattedValue, astroid.JoinedStr)
):
self.add_message("logging-fstring-interpolation", node=node)
if self._format_style != "fstr":
self.add_message(
"logging-format-interpolation",
node=node,
args=self._format_style_args,
)

@staticmethod
def _is_operand_literal_str(operand):
Expand All @@ -273,7 +274,9 @@ def _check_call_func(self, node):
if is_method_call(func, types, methods) and not is_complex_format_str(
func.bound
):
self.add_message("logging-format-interpolation", node=node)
self.add_message(
"logging-format-interpolation", node=node, args=self._format_style_args
)

def _check_format_string(self, node, format_arg):
"""Checks that format string tokens match the supplied arguments.
Expand Down Expand Up @@ -313,6 +316,12 @@ def _check_format_string(self, node, format_arg):
required_num_args = (
keyword_args_cnt + implicit_pos_args + explicit_pos_args
)
else:
self.add_message(
"logging-format-interpolation",
node=node,
args=self._format_style_args,
)
except utils.UnsupportedFormatCharacter as ex:
char = format_string[ex.index]
self.add_message(
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/l/logging_format_interpolation_py36.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
import logging as renamed_logging


renamed_logging.info(f'Read {renamed_logging} from globals') # [logging-fstring-interpolation]
renamed_logging.info(f'Read {renamed_logging} from globals') # [logging-format-interpolation]
2 changes: 1 addition & 1 deletion tests/functional/l/logging_format_interpolation_py36.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
logging-fstring-interpolation:5::Use % formatting in logging functions and pass the % parameters as arguments
logging-format-interpolation:5::Use % formatting in logging functions and pass the % parameters as arguments
12 changes: 6 additions & 6 deletions tests/functional/l/logging_fstring_interpolation_py36.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Test logging-fstring-interpolation for Python 3.6"""
"""Test logging-format-interpolation for Python 3.6"""
# pylint: disable=invalid-name

from datetime import datetime
Expand All @@ -14,8 +14,8 @@
may_14 = datetime(year=2018, month=5, day=14)

# Statements that should be flagged:
renamed_logging.debug(f'{local_var_1} {local_var_2}') # [logging-fstring-interpolation]
renamed_logging.log(renamed_logging.DEBUG, f'msg: {local_var_2}') # [logging-fstring-interpolation]
renamed_logging.log(renamed_logging.DEBUG, f'pi: {pi:.3f}') # [logging-fstring-interpolation]
renamed_logging.info(f"{local_var_2.upper()}") # [logging-fstring-interpolation]
renamed_logging.info(f"{may_14:'%b %d: %Y'}") # [logging-fstring-interpolation]
renamed_logging.debug(f'{local_var_1} {local_var_2}') # [logging-format-interpolation]
renamed_logging.log(renamed_logging.DEBUG, f'msg: {local_var_2}') # [logging-format-interpolation]
renamed_logging.log(renamed_logging.DEBUG, f'pi: {pi:.3f}') # [logging-format-interpolation]
renamed_logging.info(f"{local_var_2.upper()}") # [logging-format-interpolation]
renamed_logging.info(f"{may_14:'%b %d: %Y'}") # [logging-format-interpolation]
10 changes: 5 additions & 5 deletions tests/functional/l/logging_fstring_interpolation_py36.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
logging-fstring-interpolation:17::Use % formatting in logging functions and pass the % parameters as arguments
logging-fstring-interpolation:18::Use % formatting in logging functions and pass the % parameters as arguments
logging-fstring-interpolation:19::Use % formatting in logging functions and pass the % parameters as arguments
logging-fstring-interpolation:20::Use % formatting in logging functions and pass the % parameters as arguments
logging-fstring-interpolation:21::Use % formatting in logging functions and pass the % parameters as arguments
logging-format-interpolation:17::Use % formatting in logging functions and pass the % parameters as arguments
logging-format-interpolation:18::Use % formatting in logging functions and pass the % parameters as arguments
logging-format-interpolation:19::Use % formatting in logging functions and pass the % parameters as arguments
logging-format-interpolation:20::Use % formatting in logging functions and pass the % parameters as arguments
logging-format-interpolation:21::Use % formatting in logging functions and pass the % parameters as arguments
92 changes: 67 additions & 25 deletions tests/unittest_checker_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@
# For details: https://github.com/PyCQA/pylint/blob/master/COPYING

"""Unittest for the logging checker."""
import sys

import astroid
import pytest

from pylint.checkers import logging
from pylint.interfaces import UNDEFINED
from pylint.testutils import CheckerTestCase, Message, set_config


Expand Down Expand Up @@ -63,7 +67,7 @@ def test_nonstandard_logging_module(self):
with self.assertAddsMessages(Message("logging-not-lazy", node=stmts[1])):
self.checker.visit_call(stmts[1])

def _assert_brace_format_no_messages(self, stmt):
def _assert_logging_format_no_messages(self, stmt):
stmts = astroid.extract_node(
"""
import logging #@
Expand All @@ -77,7 +81,7 @@ def _assert_brace_format_no_messages(self, stmt):
with self.assertNoMessages():
self.checker.visit_call(stmts[1])

def _assert_brace_format_message(self, msg, stmt):
def _assert_logging_format_message(self, msg, stmt, args=None, with_too_many=False):
stmts = astroid.extract_node(
"""
import logging #@
Expand All @@ -88,38 +92,76 @@ def _assert_brace_format_message(self, msg, stmt):
)
self.checker.visit_module(None)
self.checker.visit_import(stmts[0])
with self.assertAddsMessages(Message(msg, node=stmts[1])):
messages = [
Message(msg, node=stmts[1], args=args, confidence=UNDEFINED)
]
if with_too_many:
messages.append(Message("logging-too-many-args", node=stmts[1], confidence=UNDEFINED))
with self.assertAddsMessages(*messages):
self.checker.visit_call(stmts[1])

def _assert_brace_format_too_few_args(self, stmt):
self._assert_brace_format_message("logging-too-few-args", stmt)
def _assert_logging_format_too_few_args(self, stmt):
self._assert_logging_format_message("logging-too-few-args", stmt)

def _assert_brace_format_too_many_args(self, stmt):
self._assert_brace_format_message("logging-too-many-args", stmt)
def _assert_logging_format_too_many_args(self, stmt):
self._assert_logging_format_message("logging-too-many-args", stmt)

@set_config(logging_format_style="new")
def test_brace_format_style_matching_arguments(self):
self._assert_brace_format_no_messages("('constant string')")
self._assert_brace_format_no_messages("('{}')")
self._assert_brace_format_no_messages("('{}', 1)")
self._assert_brace_format_no_messages("('{0}', 1)")
self._assert_brace_format_no_messages("('{named}', {'named': 1})")
self._assert_brace_format_no_messages("('{} {named}', 1, {'named': 1})")
self._assert_brace_format_no_messages("('{0} {named}', 1, {'named': 1})")
self._assert_logging_format_no_messages("('constant string')")
self._assert_logging_format_no_messages("('{}')")
self._assert_logging_format_no_messages("('{}', 1)")
self._assert_logging_format_no_messages("('{0}', 1)")
self._assert_logging_format_no_messages("('{named}', {'named': 1})")
self._assert_logging_format_no_messages("('{} {named}', 1, {'named': 1})")
self._assert_logging_format_no_messages("('{0} {named}', 1, {'named': 1})")

@set_config(logging_format_style="new")
def test_brace_format_style_too_few_args(self):
self._assert_brace_format_too_few_args("('{}, {}', 1)")
self._assert_brace_format_too_few_args("('{0}, {1}', 1)")
self._assert_brace_format_too_few_args("('{named1}, {named2}', {'named1': 1})")
self._assert_brace_format_too_few_args("('{0}, {named}', 1)")
self._assert_brace_format_too_few_args("('{}, {named}', {'named': 1})")
self._assert_brace_format_too_few_args("('{0}, {named}', {'named': 1})")
self._assert_logging_format_too_few_args("('{}, {}', 1)")
self._assert_logging_format_too_few_args("('{0}, {1}', 1)")
self._assert_logging_format_too_few_args("('{named1}, {named2}', {'named1': 1})")
self._assert_logging_format_too_few_args("('{0}, {named}', 1)")
self._assert_logging_format_too_few_args("('{}, {named}', {'named': 1})")
self._assert_logging_format_too_few_args("('{0}, {named}', {'named': 1})")

@set_config(logging_format_style="new")
def test_brace_format_style_not_enough_arguments(self):
self._assert_brace_format_too_many_args("('constant string', 1, 2)")
self._assert_brace_format_too_many_args("('{}', 1, 2)")
self._assert_brace_format_too_many_args("('{0}', 1, 2)")
self._assert_brace_format_too_many_args("('{}, {named}', 1, 2, {'named': 1})")
self._assert_brace_format_too_many_args("('{0}, {named}', 1, 2, {'named': 1})")
self._assert_logging_format_too_many_args("('constant string', 1, 2)")
self._assert_logging_format_too_many_args("('{}', 1, 2)")
self._assert_logging_format_too_many_args("('{0}', 1, 2)")
self._assert_logging_format_too_many_args("('{}, {named}', 1, 2, {'named': 1})")
self._assert_logging_format_too_many_args("('{0}, {named}', 1, 2, {'named': 1})")

@pytest.mark.skipif(sys.version_info[0] < (3, 6), reason="F-string require >=3.6")
@set_config(logging_format_style="new")
def test_fstr_not_new_format_style_matching_arguments(self):
msg = "logging-format-interpolation"
args=('{', '')
self._assert_logging_format_message(msg, "(f'{named}')", args)

@set_config(logging_format_style="fstr")
def test_modulo_not_fstr_format_style_matching_arguments(self):
msg = "logging-format-interpolation"
args=('f-string', '')
with_too_many = True
self._assert_logging_format_message(msg, "('%s', 1)", args, with_too_many)
self._assert_logging_format_message(msg, "('%(named)s', {'named': 1})", args, with_too_many)
self._assert_logging_format_message(msg, "('%s %(named)s', 1, {'named': 1})", args, with_too_many)

@set_config(logging_format_style="fstr")
def test_brace_not_fstr_format_style_matching_arguments(self):
msg = "logging-format-interpolation"
args=('f-string', '')
with_too_many = True
self._assert_logging_format_message(msg, "('{}', 1)", args, with_too_many)
self._assert_logging_format_message(msg, "('{0}', 1)", args, with_too_many)
self._assert_logging_format_message(msg, "('{named}', {'named': 1})", args, with_too_many)
self._assert_logging_format_message(msg, "('{} {named}', 1, {'named': 1})", args, with_too_many)
self._assert_logging_format_message(msg, "('{0} {named}', 1, {'named': 1})", args, with_too_many)

@pytest.mark.skipif(sys.version_info[0] < (3, 6), reason="F-string require >=3.6")
@set_config(logging_format_style="fstr")
def test_fstr_format_style_matching_arguments(self):
self._assert_logging_format_no_messages("(f'constant string')")
self._assert_logging_format_no_messages("(f'{named}')")

0 comments on commit eeca6cf

Please sign in to comment.