From 19b4fda2689aa749ab542c1a85b86a4606cae469 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Thu, 25 Aug 2022 12:43:20 +0200 Subject: [literal-comparison] Make the message explicit with the solution Also update the confidence Closes #5237 --- doc/data/messages/l/literal-comparison/bad.py | 2 +- doc/user_guide/checkers/features.rst | 2 +- doc/whatsnew/fragments/5237.other | 4 ++++ pylint/checkers/base/comparison_checker.py | 17 +++++++++++++++-- tests/functional/c/comparison_of_constants.txt | 8 ++++---- tests/functional/l/literal_comparison.py | 12 ++++++------ tests/functional/l/literal_comparison.txt | 24 ++++++++++++------------ 7 files changed, 43 insertions(+), 26 deletions(-) create mode 100644 doc/whatsnew/fragments/5237.other diff --git a/doc/data/messages/l/literal-comparison/bad.py b/doc/data/messages/l/literal-comparison/bad.py index d85667b7e..154d7d6c7 100644 --- a/doc/data/messages/l/literal-comparison/bad.py +++ b/doc/data/messages/l/literal-comparison/bad.py @@ -1,2 +1,2 @@ def is_an_orange(fruit): - return fruit is "orange" # [literal-comparison] + return fruit is "orange" # [literal-comparison] diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst index ad6285f07..a23c57c3e 100644 --- a/doc/user_guide/checkers/features.rst +++ b/doc/user_guide/checkers/features.rst @@ -168,7 +168,7 @@ Basic checker Messages When two literals are compared with each other the result is a constant. Using the constant directly is both easier to read and more performant. Initializing 'True' and 'False' this way is not required since Python 2.3. -:literal-comparison (R0123): *Comparison to literal* +:literal-comparison (R0123): *In '%s', use '%s' when comparing constant literals not '%s' ('%s')* Used when comparing an object to a literal, which is usually what you do not want to do, since you can compare to a different literal than what was expected altogether. diff --git a/doc/whatsnew/fragments/5237.other b/doc/whatsnew/fragments/5237.other new file mode 100644 index 000000000..2b3fadfc1 --- /dev/null +++ b/doc/whatsnew/fragments/5237.other @@ -0,0 +1,4 @@ +The message for ``literal-comparison`` is now more explicit about the problem and the +solution. + +Closes #5237 diff --git a/pylint/checkers/base/comparison_checker.py b/pylint/checkers/base/comparison_checker.py index 35e85b82c..98b0ec258 100644 --- a/pylint/checkers/base/comparison_checker.py +++ b/pylint/checkers/base/comparison_checker.py @@ -48,7 +48,7 @@ class ComparisonChecker(_BasicChecker): {"old_names": [("W0154", "old-unidiomatic-typecheck")]}, ), "R0123": ( - "Comparison to literal", + "In '%s', use '%s' when comparing constant literals not '%s' ('%s')", "literal-comparison", "Used when comparing an object to a literal, which is usually " "what you do not want to do, since you can compare to a different " @@ -201,7 +201,20 @@ class ComparisonChecker(_BasicChecker): is_const = isinstance(literal.value, (bytes, str, int, float)) if is_const or is_other_literal: - self.add_message("literal-comparison", node=node) + bad = node.as_string() + if "is not" in bad: + equal_or_not_equal = "!=" + is_or_is_not = "is not" + else: + equal_or_not_equal = "==" + is_or_is_not = "is" + good = bad.replace(is_or_is_not, equal_or_not_equal) + self.add_message( + "literal-comparison", + args=(bad, equal_or_not_equal, is_or_is_not, good), + node=node, + confidence=HIGH, + ) def _check_logical_tautology(self, node: nodes.Compare) -> None: """Check if identifier is compared against itself. diff --git a/tests/functional/c/comparison_of_constants.txt b/tests/functional/c/comparison_of_constants.txt index 1a0984024..60b421009 100644 --- a/tests/functional/c/comparison_of_constants.txt +++ b/tests/functional/c/comparison_of_constants.txt @@ -1,9 +1,9 @@ comparison-of-constants:4:3:4:9::"Comparison between constants: '2 is 2' has a constant value":HIGH -literal-comparison:4:3:4:9::Comparison to literal:UNDEFINED +literal-comparison:4:3:4:9::In '2 is 2', use '==' when comparing constant literals not 'is' ('2 == 2'):HIGH comparison-of-constants:7:6:7:12::"Comparison between constants: '2 == 2' has a constant value":HIGH comparison-of-constants:10:6:10:11::"Comparison between constants: '2 > 2' has a constant value":HIGH comparison-of-constants:20:3:20:15::"Comparison between constants: 'True == True' has a constant value":HIGH singleton-comparison:20:3:20:15::Comparison 'True == True' should be 'True is True' if checking for the singleton value True, or 'True' if testing for truthiness:UNDEFINED -literal-comparison:25:3:25:13::Comparison to literal:UNDEFINED -literal-comparison:28:3:28:13::Comparison to literal:UNDEFINED -literal-comparison:31:3:31:14::Comparison to literal:UNDEFINED +literal-comparison:25:3:25:13::In 'CONST is 0', use '==' when comparing constant literals not 'is' ('CONST == 0'):HIGH +literal-comparison:28:3:28:13::In 'CONST is 1', use '==' when comparing constant literals not 'is' ('CONST == 1'):HIGH +literal-comparison:31:3:31:14::In 'CONST is 42', use '==' when comparing constant literals not 'is' ('CONST == 42'):HIGH diff --git a/tests/functional/l/literal_comparison.py b/tests/functional/l/literal_comparison.py index b5e31a0e7..603c0da7a 100644 --- a/tests/functional/l/literal_comparison.py +++ b/tests/functional/l/literal_comparison.py @@ -1,25 +1,25 @@ # pylint: disable=missing-docstring, comparison-with-itself -if 2 is 2: # [literal-comparison, comparison-of-constants] +if 2 is 2: # [literal-comparison, comparison-of-constants] pass -if "a" is b"a": # [literal-comparison, comparison-of-constants] +if "a" is b"a": # [literal-comparison, comparison-of-constants] pass -if 2.0 is 3.0: # [literal-comparison, comparison-of-constants] +if 2.0 is 3.0: # [literal-comparison, comparison-of-constants] pass if () is (1, 2, 3): pass -if () is {1:2, 2:3}: # [literal-comparison] +if () is {1:2, 2:3}: # [literal-comparison] pass -if [] is [4, 5, 6]: # [literal-comparison] +if [] is [4, 5, 6]: # [literal-comparison] pass -if () is {1, 2, 3}: # [literal-comparison] +if () is {1, 2, 3}: # [literal-comparison] pass if () is not {1:2, 2:3}: # [literal-comparison] diff --git a/tests/functional/l/literal_comparison.txt b/tests/functional/l/literal_comparison.txt index c1a679c76..f14c73219 100644 --- a/tests/functional/l/literal_comparison.txt +++ b/tests/functional/l/literal_comparison.txt @@ -1,15 +1,15 @@ comparison-of-constants:4:3:4:9::"Comparison between constants: '2 is 2' has a constant value":HIGH -literal-comparison:4:3:4:9::Comparison to literal:UNDEFINED +literal-comparison:4:3:4:9::In '2 is 2', use '==' when comparing constant literals not 'is' ('2 == 2'):HIGH comparison-of-constants:7:3:7:14::"Comparison between constants: 'a is b'a'' has a constant value":HIGH -literal-comparison:7:3:7:14::Comparison to literal:UNDEFINED +literal-comparison:7:3:7:14::In ''a' is b'a'', use '==' when comparing constant literals not 'is' (''a' == b'a''):HIGH comparison-of-constants:10:3:10:13::"Comparison between constants: '2.0 is 3.0' has a constant value":HIGH -literal-comparison:10:3:10:13::Comparison to literal:UNDEFINED -literal-comparison:16:3:16:19::Comparison to literal:UNDEFINED -literal-comparison:19:3:19:18::Comparison to literal:UNDEFINED -literal-comparison:22:3:22:18::Comparison to literal:UNDEFINED -literal-comparison:25:3:25:23::Comparison to literal:UNDEFINED -literal-comparison:28:3:28:22::Comparison to literal:UNDEFINED -literal-comparison:31:3:31:22::Comparison to literal:UNDEFINED -literal-comparison:38:3:38:13::Comparison to literal:UNDEFINED -literal-comparison:41:3:41:13::Comparison to literal:UNDEFINED -literal-comparison:44:3:44:14::Comparison to literal:UNDEFINED +literal-comparison:10:3:10:13::In '2.0 is 3.0', use '==' when comparing constant literals not 'is' ('2.0 == 3.0'):HIGH +literal-comparison:16:3:16:19::"In '() is {1: 2, 2: 3}', use '==' when comparing constant literals not 'is' ('() == {1: 2, 2: 3}')":HIGH +literal-comparison:19:3:19:18::In '[] is [4, 5, 6]', use '==' when comparing constant literals not 'is' ('[] == [4, 5, 6]'):HIGH +literal-comparison:22:3:22:18::In '() is {1, 2, 3}', use '==' when comparing constant literals not 'is' ('() == {1, 2, 3}'):HIGH +literal-comparison:25:3:25:23::"In '() is not {1: 2, 2: 3}', use '!=' when comparing constant literals not 'is not' ('() != {1: 2, 2: 3}')":HIGH +literal-comparison:28:3:28:22::In '[] is not [4, 5, 6]', use '!=' when comparing constant literals not 'is not' ('[] != [4, 5, 6]'):HIGH +literal-comparison:31:3:31:22::In '() is not {1, 2, 3}', use '!=' when comparing constant literals not 'is not' ('() != {1, 2, 3}'):HIGH +literal-comparison:38:3:38:13::In 'CONST is 0', use '==' when comparing constant literals not 'is' ('CONST == 0'):HIGH +literal-comparison:41:3:41:13::In 'CONST is 1', use '==' when comparing constant literals not 'is' ('CONST == 1'):HIGH +literal-comparison:44:3:44:14::In 'CONST is 42', use '==' when comparing constant literals not 'is' ('CONST == 42'):HIGH -- cgit v1.2.1