From 7d81b99ba41a4fbc21220f673dbe1af2eef2a9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Fri, 20 Aug 2021 12:59:47 +0200 Subject: [PATCH 1/5] Add ``consider-using-f-string`` checker This adds a checker for normal strings which are formatted with ``.format()`` or '%'. The message is a convention to nudge users towards using f-strings. This closes #3592 --- ChangeLog | 5 +- doc/whatsnew/2.10.rst | 4 + pylint/checkers/strings.py | 71 ++++++++++++ .../c/consider/consider_using_f_string.py | 107 ++++++++++++++++++ .../c/consider/consider_using_f_string.txt | 30 +++++ 5 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 tests/functional/c/consider/consider_using_f_string.py create mode 100644 tests/functional/c/consider/consider_using_f_string.txt diff --git a/ChangeLog b/ChangeLog index 53c8708530..8ed85ed8fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -153,7 +153,6 @@ Release date: TBA * Allow ``true`` and ``false`` values in ``pylintrc`` for better compatibility with ``toml`` config. - * Class methods' signatures are ignored the same way as functions' with similarities "ignore-signatures" option enabled Closes #4653 @@ -162,6 +161,10 @@ Release date: TBA * Improve error message for invalid-metaclass when the node is an Instance. +* Added ``consider-using-f-string``: Emitted when .format() or '%' is being used to format a string. + + Closes #3592 + What's New in Pylint 2.9.6? =========================== diff --git a/doc/whatsnew/2.10.rst b/doc/whatsnew/2.10.rst index 501c9156a3..0d666fd59e 100644 --- a/doc/whatsnew/2.10.rst +++ b/doc/whatsnew/2.10.rst @@ -34,6 +34,10 @@ New checkers * Added ``use-sequence-for-iteration``: Emitted when iterating over an in-place defined ``set``. +* Added ``consider-using-f-string``: Emitted when .format() or '%' is being used to format a string. + + Closes #3592 + Extensions ========== diff --git a/pylint/checkers/strings.py b/pylint/checkers/strings.py index a0944825ea..515bc61b47 100644 --- a/pylint/checkers/strings.py +++ b/pylint/checkers/strings.py @@ -672,6 +672,12 @@ class StringConstantChecker(BaseTokenChecker): "in Python 2 to indicate a string was Unicode, but since Python 3.0 strings " "are Unicode by default.", ), + "C1407": ( + "Formatting a regular string which could be a f-string", + "consider-using-f-string", + "Used when we detect a string that is being formatted with format() or % " + "which could potentially be a f-string. The use of f-strings is preferred.", + ), } options = ( ( @@ -909,11 +915,13 @@ def process_non_raw_string_token( index += 2 @check_messages("redundant-u-string-prefix") + @check_messages("consider-using-f-string") def visit_const(self, node: nodes.Const): if node.pytype() == "builtins.str" and not isinstance( node.parent, nodes.JoinedStr ): self._detect_u_string_prefix(node) + self._detect_replacable_format_call(node) def _detect_u_string_prefix(self, node: nodes.Const): """Check whether strings include a 'u' prefix like u'String'""" @@ -924,6 +932,69 @@ def _detect_u_string_prefix(self, node: nodes.Const): col_offset=node.col_offset, ) + def _detect_replacable_format_call(self, node: nodes.Const): + """Check whether a string is used in a call to format() or '%' and whether it + can be replaced by a f-string""" + if ( + isinstance(node.parent, nodes.Attribute) + and node.parent.attrname == "format" + ): + # Allow assigning .format to a variable + if isinstance(node.parent.parent, nodes.Assign): + return + + if node.parent.parent.args: + for arg in node.parent.parent.args: + # If star expressions with more than 1 element are being used + if isinstance(arg, nodes.Starred): + inferred = utils.safe_infer(arg.value) + if ( + isinstance(inferred, astroid.List) + and len(inferred.elts) > 1 + ): + return + + elif node.parent.parent.keywords: + keyword_args = [ + i[0] for i in utils.parse_format_method_string(node.value)[0] + ] + for keyword in node.parent.parent.keywords: + # If keyword is used multiple times + if keyword_args.count(keyword.arg) > 1: + return + + keyword = utils.safe_infer(keyword.value) + + # If lists of more than one element are being unpacked + if isinstance(keyword, nodes.Dict): + if len(keyword.items) > 1 and len(keyword_args) > 1: + return + + # If all tests pass, then raise message + self.add_message( + "consider-using-f-string", + line=node.lineno, + col_offset=node.col_offset, + ) + + elif isinstance(node.parent, nodes.BinOp) and node.parent.op == "%": + inferred_right = utils.safe_infer(node.parent.right) + + # If dicts or lists of length > 1 are used + if isinstance(inferred_right, nodes.Dict): + if len(inferred_right.items) > 1: + return + elif isinstance(inferred_right, nodes.List): + if len(inferred_right.elts) > 1: + return + + # If all tests pass, then raise message + self.add_message( + "consider-using-f-string", + line=node.lineno, + col_offset=node.col_offset, + ) + def register(linter): """required method to auto register this checker""" diff --git a/tests/functional/c/consider/consider_using_f_string.py b/tests/functional/c/consider/consider_using_f_string.py new file mode 100644 index 0000000000..825f3517ca --- /dev/null +++ b/tests/functional/c/consider/consider_using_f_string.py @@ -0,0 +1,107 @@ +"""Test to see if a f-string would be possible and consider-using-f-string should be raised""" +# pylint: disable=unused-variable, invalid-name, missing-function-docstring, pointless-statement +# pylint: disable=expression-not-assigned, repeated-keyword + +PARAM_1 = PARAM_2 = PARAM_3 = 1 +PARAM_LIST = [PARAM_1, PARAM_2, PARAM_3] +PARAM_LIST_SINGLE = [PARAM_1] +PARAM_DICT = {"Param_1": PARAM_1, "Param_2": PARAM_2, "Param_3": PARAM_3} +PARAM_DICT_SINGLE = {"Param_1": PARAM_1} + + +def return_parameter(): + return PARAM_1 + + +def return_list(): + return PARAM_LIST + + +def return_dict(): + return PARAM_DICT + + +def print_good(): + print("String {}, {} or {}".format(*PARAM_LIST)) + print("String {}, {}, {} or {}".format(*PARAM_LIST_SINGLE, *PARAM_LIST)) + print("String {Param}, {}, {} or {}".format(Param=PARAM_1, *PARAM_LIST)) + print("String {Param} {Param}".format(Param=PARAM_1)) + print("{Param_1} {Param_2}".format(**PARAM_DICT)) + print("{Param_1} {Param_2} {Param_3}".format(**PARAM_DICT_SINGLE, **PARAM_DICT)) + print("{Param_1} {Param_2} {Param_3}".format(Param_1=PARAM_1, **PARAM_DICT)) + print("{Param_1} {Param_2}".format(**PARAM_DICT)) + print("{Param_1} {Param_2}".format(**return_dict())) + print("%(Param_1)s %(Param_2)s" % PARAM_LIST) + print("%(Param_1)s %(Param_2)s" % PARAM_DICT) + print("%(Param_1)s %(Param_2)s" % return_dict()) + print("{a[Param_1]}{a[Param_2]}".format(a=PARAM_DICT)) + +def print_bad(): + print("String %f" % PARAM_1) # [consider-using-f-string] + print("String {}".format(PARAM_1)) # [consider-using-f-string] + print("String {Param_1}".format(Param_1=PARAM_1)) # [consider-using-f-string] + print("{} {}".format(PARAM_1, PARAM_2)) # [consider-using-f-string] + print("{Par_1}{Par_2}".format(Par_1=PARAM_1, Par_2=PARAM_2)) # [consider-using-f-string] + print("{Param_1}".format(*PARAM_LIST_SINGLE)) # [consider-using-f-string] + print("{Param_1}".format(**PARAM_DICT_SINGLE)) # [consider-using-f-string] + print("String %s" % (PARAM_1)) # [consider-using-f-string] + print("String %s %s" % (PARAM_1, PARAM_2)) # [consider-using-f-string] + print("String %s" % (PARAM_LIST_SINGLE)) # [consider-using-f-string] + + +def statement_good(): + "String {}, {} or {}".format(*PARAM_LIST) + "String {}, {}, {} or {}".format(*PARAM_LIST_SINGLE, *PARAM_LIST) + "String {Param}, {}, {} or {}".format(Param=PARAM_1, *PARAM_LIST) + "String {Param} {Param}".format(Param=PARAM_1) + "{Param_1} {Param_2}".format(**PARAM_DICT) + "{Param_1} {Param_2} {Param_3}".format(**PARAM_DICT_SINGLE, **PARAM_DICT) + "{Param_1} {Param_2} {Param_3}".format(Param_1=PARAM_1, **PARAM_DICT) + "{Param_1} {Param_2}".format(**PARAM_DICT) + "{Param_1} {Param_2}".format(**return_dict()) + "%(Param_1)s %(Param_2)s" % PARAM_LIST + "%(Param_1)s %(Param_2)s" % PARAM_DICT + "%(Param_1)s %(Param_2)s" % return_dict() + "{a[Param_1]}{a[Param_2]}".format(a=PARAM_DICT) + +def statement_bad(): + "String %f" % PARAM_1 # [consider-using-f-string] + "String {}".format(PARAM_1) # [consider-using-f-string] + "String {Param_1}".format(Param_1=PARAM_1) # [consider-using-f-string] + "{} {}".format(PARAM_1, PARAM_2) # [consider-using-f-string] + "{Par_1}{Par_2}".format(Par_1=PARAM_1, Par_2=PARAM_2) # [consider-using-f-string] + "{Param_1}".format(*PARAM_LIST_SINGLE) # [consider-using-f-string] + "{Param_1}".format(**PARAM_DICT_SINGLE) # [consider-using-f-string] + "String %s" % (PARAM_1) # [consider-using-f-string] + "String %s %s" % (PARAM_1, PARAM_2) # [consider-using-f-string] + "String %s" % (PARAM_LIST_SINGLE) # [consider-using-f-string] + + +def assignment_good(): + A = "String {}, {} or {}".format(*PARAM_LIST) + B = "String {}, {}, {} or {}".format(*PARAM_LIST_SINGLE, *PARAM_LIST) + C = "String {Param}, {}, {} or {}".format(Param=PARAM_1, *PARAM_LIST) + D = "String {Param} {Param}".format(Param=PARAM_1) + E = "{Param_1} {Param_2}".format(**PARAM_DICT) + F = "{Param_1} {Param_2} {Param_3}".format(**PARAM_DICT_SINGLE, **PARAM_DICT) + G = "{Param_1} {Param_2} {Param_3}".format(Param_1=PARAM_1, **PARAM_DICT) + H = "{Param_1} {Param_2}".format(**PARAM_DICT) + I = "{Param_1} {Param_2}".format(**return_dict()) + J = "%(Param_1)s %(Param_2)s" % PARAM_LIST + K = "%(Param_1)s %(Param_2)s" % PARAM_DICT + L = "%(Param_1)s %(Param_2)s" % return_dict() + M = "{a[Param_1]}{a[Param_2]}".format(a=PARAM_DICT) + N = "{Param}".format + + +def assignment_bad(): + a = "String %f" % PARAM_1 # [consider-using-f-string] + b = "String {}".format(PARAM_1) # [consider-using-f-string] + c = "String {Param_1}".format(Param_1=PARAM_1) # [consider-using-f-string] + d = "{} {}".format(PARAM_1, PARAM_2) # [consider-using-f-string] + e = "{Par_1}{Par_2}".format(Par_1=PARAM_1, Par_2=PARAM_2) # [consider-using-f-string] + f = "{Param_1}".format(*PARAM_LIST_SINGLE) # [consider-using-f-string] + g = "{Param_1}".format(**PARAM_DICT_SINGLE) # [consider-using-f-string] + h = "String %s" % (PARAM_1) # [consider-using-f-string] + i = "String %s %s" % (PARAM_1, PARAM_2) # [consider-using-f-string] + j = "String %s" % (PARAM_LIST_SINGLE) # [consider-using-f-string] diff --git a/tests/functional/c/consider/consider_using_f_string.txt b/tests/functional/c/consider/consider_using_f_string.txt new file mode 100644 index 0000000000..05288cde02 --- /dev/null +++ b/tests/functional/c/consider/consider_using_f_string.txt @@ -0,0 +1,30 @@ +consider-using-f-string:40:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:41:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:42:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:43:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:44:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:45:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:46:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:47:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:48:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:49:10::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:68:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:69:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:70:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:71:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:72:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:73:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:74:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:75:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:76:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:77:4::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:98:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:99:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:100:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:101:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:102:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:103:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:104:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:105:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:106:8::"Formatting a regular string which could be a f-string":HIGH +consider-using-f-string:107:8::"Formatting a regular string which could be a f-string":HIGH From 8bc96682b611846bf4f8a1c00f8579da56f1f05e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Fri, 20 Aug 2021 13:09:47 +0200 Subject: [PATCH 2/5] Update pylint code to use f-strings After adding `consider-using-f-strings` the codebase showed numerous cases of formatting which could be f-strings. This commit changes most of these to become f-strings, or adds ignores. --- doc/exts/pylint_extensions.py | 2 +- pylint/checkers/__init__.py | 4 +- pylint/checkers/base.py | 18 +-- pylint/checkers/base_checker.py | 20 +-- pylint/checkers/classes.py | 3 +- pylint/checkers/exceptions.py | 17 +-- pylint/checkers/imports.py | 22 ++- pylint/checkers/misc.py | 2 +- pylint/checkers/raw_metrics.py | 4 +- pylint/checkers/refactoring/not_checker.py | 6 +- .../refactoring/refactoring_checker.py | 6 +- pylint/checkers/similar.py | 8 +- pylint/checkers/spelling.py | 6 +- pylint/checkers/strings.py | 8 +- pylint/checkers/typecheck.py | 2 +- pylint/checkers/variables.py | 11 +- pylint/config/__init__.py | 2 +- pylint/config/man_help_formatter.py | 7 +- pylint/config/option.py | 5 +- pylint/config/option_manager_mixin.py | 2 +- pylint/epylint.py | 4 +- pylint/extensions/_check_docs_utils.py | 144 +++++++----------- pylint/extensions/broad_try_clause.py | 4 +- pylint/extensions/mccabe.py | 8 +- pylint/extensions/overlapping_exceptions.py | 6 +- pylint/graph.py | 24 +-- pylint/lint/pylinter.py | 13 +- pylint/lint/report_functions.py | 2 +- pylint/lint/run.py | 5 +- pylint/lint/utils.py | 4 +- pylint/message/message_definition.py | 12 +- pylint/message/message_handler_mix_in.py | 14 +- pylint/pyreverse/diadefslib.py | 4 +- pylint/pyreverse/diagrams.py | 4 +- pylint/pyreverse/inspector.py | 4 +- pylint/pyreverse/utils.py | 8 +- pylint/pyreverse/vcg_printer.py | 2 +- pylint/reporters/base_reporter.py | 2 +- pylint/reporters/reports_handler_mix_in.py | 2 +- pylint/reporters/text.py | 9 +- pylint/reporters/ureports/nodes.py | 4 +- pylint/reporters/ureports/text_writer.py | 2 +- pylint/testutils/constants.py | 6 +- pylint/testutils/lint_module_test.py | 16 +- pylint/testutils/reporter_for_tests.py | 2 +- pylint/utils/utils.py | 18 +-- script/bump_changelog.py | 10 +- tests/benchmark/test_baseline_benchmarks.py | 40 ++--- tests/checkers/unittest_python3.py | 4 +- tests/checkers/unittest_similar.py | 68 ++++----- tests/functional/a/arguments.py | 2 +- tests/functional/d/docstrings.py | 2 +- .../d/duplicate_string_formatting_argument.py | 2 +- .../l/logging_format_interpolation.py | 2 +- tests/functional/l/logging_not_lazy.py | 3 +- .../l/logging_not_lazy_with_logger.py | 1 + .../l/logging_not_lazy_with_logger.txt | 6 +- .../functional/m/misplaced_format_function.py | 2 +- tests/functional/n/new_style_class_py_30.py | 5 +- tests/functional/n/new_style_class_py_30.txt | 6 +- .../r/raise/raising_format_tuple.py | 6 +- .../r/renamed_import_logging_not_lazy.py | 2 +- tests/functional/s/slots_checks.py | 2 +- .../functional/s/string/string_formatting.py | 2 +- .../s/string/string_formatting_error.py | 2 +- .../string_formatting_failed_inference.py | 2 +- ...string_formatting_failed_inference_py35.py | 2 +- .../s/string/string_formatting_py3.py | 2 +- .../t/too/too_many_return_statements.py | 2 +- .../u/undefined/undefined_loop_variable.py | 2 +- tests/functional/u/unused/unused_argument.py | 2 +- .../u/use/used_before_assignement.py | 2 +- .../u/use/used_before_assignment_issue853.py | 2 +- tests/input/func_w0401_package/thing2.py | 2 +- .../profile/test_profile_against_externals.py | 7 +- tests/pyreverse/test_utils.py | 2 +- tests/test_check_parallel.py | 5 +- tests/test_epylint.py | 2 + tests/test_func.py | 5 +- tests/test_self.py | 12 +- 80 files changed, 311 insertions(+), 385 deletions(-) diff --git a/doc/exts/pylint_extensions.py b/doc/exts/pylint_extensions.py index 02e9be5bc2..4dc6f184f6 100755 --- a/doc/exts/pylint_extensions.py +++ b/doc/exts/pylint_extensions.py @@ -37,7 +37,7 @@ def builder_inited(app): if name[0] == "_" or name in DEPRECATED_MODULES: continue if ext == ".py": - modules.append("pylint.extensions.%s" % name) + modules.append(f"pylint.extensions.{name}") elif ext == ".rst": doc_files["pylint.extensions." + name] = os.path.join(ext_path, filename) modules.sort() diff --git a/pylint/checkers/__init__.py b/pylint/checkers/__init__.py index 4bc586a1c4..ffc9402232 100644 --- a/pylint/checkers/__init__.py +++ b/pylint/checkers/__init__.py @@ -65,8 +65,8 @@ def table_lines_from_stats(stats, old_stats, columns): diff_str = diff_string(old, new) else: old, diff_str = "NC", "NC" - new = "%.3f" % new if isinstance(new, float) else str(new) - old = "%.3f" % old if isinstance(old, float) else str(old) + new = f"{new:.3f}" if isinstance(new, float) else str(new) + old = f"{old:.3f}" if isinstance(old, float) else str(old) lines += (m_type.replace("_", " "), new, old, diff_str) return lines diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index 352e20ebe4..ae4ca15d28 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -193,7 +193,7 @@ class AnyStyle(NamingStyle): ["set()", "{}", "[]"], ), **{ - x: "%s()" % x + x: f"{x}()" for x in ( "collections.deque", "collections.ChainMap", @@ -407,12 +407,12 @@ def report_by_type_stats(sect, stats, old_stats): try: documented = total - stats["undocumented_" + node_type] percent = (documented * 100.0) / total - nice_stats[node_type]["percent_documented"] = "%.2f" % percent + nice_stats[node_type]["percent_documented"] = f"{percent:.2f}" except KeyError: nice_stats[node_type]["percent_documented"] = "NC" try: percent = (stats["badname_" + node_type] * 100.0) / total - nice_stats[node_type]["percent_badname"] = "%.2f" % percent + nice_stats[node_type]["percent_badname"] = f"{percent:.2f}" except KeyError: nice_stats[node_type]["percent_badname"] = "NC" lines = ("type", "number", "old number", "difference", "%documented", "%badname") @@ -1706,8 +1706,7 @@ def _create_naming_options(): "type": "choice", "choices": list(NAMING_STYLES.keys()), "metavar": "