diff options
author | Svet <svet@hyperscience.com> | 2019-02-13 10:59:11 +0200 |
---|---|---|
committer | Claudiu Popa <pcmanticore@gmail.com> | 2019-02-13 09:59:11 +0100 |
commit | e4fec97bc5af1d1b5ef87a5a15195307858b10a2 (patch) | |
tree | 88e2b553f5a34b12bb0a8bfcb66087db68726687 | |
parent | 7a40b0b6d3113403a266ffef13431cdc8e6a8327 (diff) | |
download | pylint-git-e4fec97bc5af1d1b5ef87a5a15195307858b10a2.tar.gz |
Fixes for linting of new style logging format. (#2713)
The number of arguments was not handled properly, leading to an always successful check. See new tests for specific cases this fixes.
-rw-r--r-- | CONTRIBUTORS.txt | 2 | ||||
-rw-r--r-- | ChangeLog | 6 | ||||
-rw-r--r-- | pylint/checkers/logging.py | 14 | ||||
-rw-r--r-- | pylint/checkers/utils.py | 22 | ||||
-rw-r--r-- | pylint/test/unittest_checker_logging.py | 56 |
5 files changed, 80 insertions, 20 deletions
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index a479e9461..448eb880d 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -265,3 +265,5 @@ contributors: * Nicolas Dickreuter * Pascal Corpet + +* Svetoslav Neykov: contributor @@ -110,6 +110,12 @@ Release date: TBA * Add a new option 'check-str-concat-over-line-jumps' to check 'implicit-str-concat-in-sequence' +* Fixes for the new style logging format linter. + + The number of arguments was not handled properly, leading to an always + successful check. + + What's New in Pylint 2.2.2? =========================== diff --git a/pylint/checkers/logging.py b/pylint/checkers/logging.py index 442cf0ed9..13646cdce 100644 --- a/pylint/checkers/logging.py +++ b/pylint/checkers/logging.py @@ -286,8 +286,8 @@ class LoggingChecker(checkers.BaseChecker): """ num_args = _count_supplied_tokens(node.args[format_arg + 1 :]) if not num_args: - # If no args were supplied, then all format strings are valid - - # don't check any further. + # 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 if not isinstance(format_string, str): @@ -305,12 +305,16 @@ class LoggingChecker(checkers.BaseChecker): # special keywords - out of scope. return elif self._format_style == "new": - keys, num_args, manual_pos_arg = utils.parse_format_method_string( + keyword_arguments, implicit_pos_args, explicit_pos_args = utils.parse_format_method_string( format_string ) - kargs = len(set(k for k, l in keys if not isinstance(k, int))) - required_num_args = kargs + num_args + manual_pos_arg + keyword_args_cnt = len( + set(k for k, l in keyword_arguments if not isinstance(k, int)) + ) + required_num_args = ( + keyword_args_cnt + implicit_pos_args + explicit_pos_args + ) except utils.UnsupportedFormatCharacter as ex: char = format_string[ex.index] self.add_message( diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index d6083d4ef..3cc292115 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -571,31 +571,31 @@ def parse_format_method_string( ) -> Tuple[List[Tuple[str, List[Tuple[bool, str]]]], int, int]: """ Parses a PEP 3101 format string, returning a tuple of - (keys, num_args, manual_pos_arg), - where keys is the set of mapping keys in the format string, num_args + (keyword_arguments, implicit_pos_args_cnt, explicit_pos_args), + where keyword_arguments is the set of mapping keys in the format string, implicit_pos_args_cnt is the number of arguments required by the format string and - manual_pos_arg is the number of arguments passed with the position. + explicit_pos_args is the number of arguments passed with the position. """ - keys = [] - num_args = 0 - manual_pos_arg = set() + keyword_arguments = [] + implicit_pos_args_cnt = 0 + explicit_pos_args = set() for name in collect_string_fields(format_string): if name and str(name).isdigit(): - manual_pos_arg.add(str(name)) + explicit_pos_args.add(str(name)) elif name: keyname, fielditerator = split_format_field_names(name) if isinstance(keyname, numbers.Number): # In Python 2 it will return long which will lead # to different output between 2 and 3 - manual_pos_arg.add(str(keyname)) + explicit_pos_args.add(str(keyname)) keyname = int(keyname) try: - keys.append((keyname, list(fielditerator))) + keyword_arguments.append((keyname, list(fielditerator))) except ValueError: raise IncompleteFormatString() else: - num_args += 1 - return keys, num_args, len(manual_pos_arg) + implicit_pos_args_cnt += 1 + return keyword_arguments, implicit_pos_args_cnt, len(explicit_pos_args) def is_attr_protected(attrname: str) -> bool: diff --git a/pylint/test/unittest_checker_logging.py b/pylint/test/unittest_checker_logging.py index a1c111c9e..8fb4cae93 100644 --- a/pylint/test/unittest_checker_logging.py +++ b/pylint/test/unittest_checker_logging.py @@ -63,15 +63,63 @@ class TestLoggingModuleDetection(CheckerTestCase): with self.assertAddsMessages(Message("logging-not-lazy", node=stmts[1])): self.checker.visit_call(stmts[1]) - @set_config(logging_format_style="new") - def test_brace_format_style(self): + def _assert_brace_format_no_messages(self, stmt): stmts = astroid.extract_node( """ import logging #@ - logging.error('{}', 1) #@ - """ + logging.error<placeholder> #@ + """.replace( + "<placeholder>", stmt + ) ) self.checker.visit_module(None) self.checker.visit_import(stmts[0]) with self.assertNoMessages(): self.checker.visit_call(stmts[1]) + + def _assert_brace_format_message(self, msg, stmt): + stmts = astroid.extract_node( + """ + import logging #@ + logging.error<placeholder> #@ + """.replace( + "<placeholder>", stmt + ) + ) + self.checker.visit_module(None) + self.checker.visit_import(stmts[0]) + with self.assertAddsMessages(Message(msg, node=stmts[1])): + 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_brace_format_too_many_args(self, stmt): + self._assert_brace_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})") + + @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})") + + @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})") |