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] 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