From 47797b54e732c5708f95c230641316b72b44ddd6 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Mon, 7 Nov 2022 09:58:23 +0100 Subject: [PATCH] [compare-to-empty-string] More actionnable and understandable message We have to use " as string delimiter because node as string have ' as string delimiter. --- pylint/extensions/emptystring.py | 46 +++++++++++-------- .../emptystring/empty_string_comparison.py | 6 +++ .../emptystring/empty_string_comparison.txt | 10 ++-- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/pylint/extensions/emptystring.py b/pylint/extensions/emptystring.py index 5ff4ccb6d7..f96a980f59 100644 --- a/pylint/extensions/emptystring.py +++ b/pylint/extensions/emptystring.py @@ -20,18 +20,10 @@ class CompareToEmptyStringChecker(checkers.BaseChecker): - """Checks for comparisons to empty string. - - Most of the time you should use the fact that empty strings are false. - An exception to this rule is when an empty string value is allowed in the program - and has a different meaning than None! - """ - - # configuration section name name = "compare-to-empty-string" msgs = { "C1901": ( - "Avoid comparisons to empty string", + '"%s" can be simplified to "%s" as an empty string is falsey', "compare-to-empty-string", "Used when Pylint detects comparison to an empty string constant.", ) @@ -41,31 +33,45 @@ class CompareToEmptyStringChecker(checkers.BaseChecker): @utils.only_required_for_messages("compare-to-empty-string") def visit_compare(self, node: nodes.Compare) -> None: - _operators = ["!=", "==", "is not", "is"] - # note: astroid.Compare has the left most operand in node.left - # while the rest are a list of tuples in node.ops - # the format of the tuple is ('compare operator sign', node) - # here we squash everything into `ops` to make it easier for processing later + """Checks for comparisons to empty string. + + Most of the time you should use the fact that empty strings are false. + An exception to this rule is when an empty string value is allowed in the program + and has a different meaning than None! + """ + _operators = {"!=", "==", "is not", "is"} + # note: astroid.Compare has the left most operand in node.left while the rest + # are a list of tuples in node.ops the format of the tuple is + # ('compare operator sign', node) here we squash everything into `ops` + # to make it easier for processing later ops: list[tuple[str, nodes.NodeNG | None]] = [("", node.left)] ops.extend(node.ops) iter_ops = iter(ops) ops = list(itertools.chain(*iter_ops)) # type: ignore[arg-type] - for ops_idx in range(len(ops) - 2): op_1: nodes.NodeNG | None = ops[ops_idx] op_2: str = ops[ops_idx + 1] # type: ignore[assignment] op_3: nodes.NodeNG | None = ops[ops_idx + 2] error_detected = False - + if op_1 is None or op_3 is None or op_2 not in _operators: + continue + node_name = "" # x ?? "" - if utils.is_empty_str_literal(op_1) and op_2 in _operators: + if utils.is_empty_str_literal(op_1): error_detected = True + node_name = op_3.as_string() # '' ?? X - elif op_2 in _operators and utils.is_empty_str_literal(op_3): + elif utils.is_empty_str_literal(op_3): error_detected = True - + node_name = op_1.as_string() if error_detected: - self.add_message("compare-to-empty-string", node=node, confidence=HIGH) + suggestion = f"not {node_name}" if op_2 in {"==", "is"} else node_name + self.add_message( + "compare-to-empty-string", + args=(node.as_string(), suggestion), + node=node, + confidence=HIGH, + ) def register(linter: PyLinter) -> None: diff --git a/tests/functional/ext/emptystring/empty_string_comparison.py b/tests/functional/ext/emptystring/empty_string_comparison.py index c6dcf8ea8f..b61caeff61 100644 --- a/tests/functional/ext/emptystring/empty_string_comparison.py +++ b/tests/functional/ext/emptystring/empty_string_comparison.py @@ -14,3 +14,9 @@ if Y != '': # [compare-to-empty-string] pass + +if "" == Y: # [compare-to-empty-string] + pass + +if '' != X: # [compare-to-empty-string] + pass diff --git a/tests/functional/ext/emptystring/empty_string_comparison.txt b/tests/functional/ext/emptystring/empty_string_comparison.txt index 7d4ddedfbb..be9c91bc57 100644 --- a/tests/functional/ext/emptystring/empty_string_comparison.txt +++ b/tests/functional/ext/emptystring/empty_string_comparison.txt @@ -1,4 +1,6 @@ -compare-to-empty-string:6:3:6:10::Avoid comparisons to empty string:HIGH -compare-to-empty-string:9:3:9:14::Avoid comparisons to empty string:HIGH -compare-to-empty-string:12:3:12:10::Avoid comparisons to empty string:HIGH -compare-to-empty-string:15:3:15:10::Avoid comparisons to empty string:HIGH +compare-to-empty-string:6:3:6:10::"""X is ''"" can be simplified to ""not X"" as an empty string is falsey":HIGH +compare-to-empty-string:9:3:9:14::"""Y is not ''"" can be simplified to ""Y"" as an empty string is falsey":HIGH +compare-to-empty-string:12:3:12:10::"""X == ''"" can be simplified to ""not X"" as an empty string is falsey":HIGH +compare-to-empty-string:15:3:15:10::"""Y != ''"" can be simplified to ""Y"" as an empty string is falsey":HIGH +compare-to-empty-string:18:3:18:10::"""'' == Y"" can be simplified to ""not Y"" as an empty string is falsey":HIGH +compare-to-empty-string:21:3:21:10::"""'' != X"" can be simplified to ""X"" as an empty string is falsey":HIGH