Add comparison-of-constants checker

ChangeLog

@@ -9,6 +9,10 @@ Release date: TBA
 ..
   Put new features here and also in 'doc/whatsnew/2.14.rst'
 
+* Added new checker ``comparison-of-constants``.
+
+  Closes #6076
+
 * Started ignoring underscore as a local variable for ``too-many-locals``.
 
   Closes #6488

doc/data/messages/c/comparison-of-constants/bad.py

@@ -0,0 +1,2 @@
+def is_the_answer() -> bool:
+    return 42 == 42 # [comparison-of-constants]

doc/data/messages/c/comparison-of-constants/good.py

@@ -0,0 +1,2 @@
+def is_the_answer(meaning_of_life: int) -> bool:
+    return meaning_of_life == 42

doc/whatsnew/2.14.rst

@@ -12,6 +12,10 @@ Summary -- Release highlights
 New checkers
 ============
 
+* Added new checker ``comparison-of-constants``.
+
+  Closes #6076
+
 * Added new checker ``typevar-name-mismatch``: TypeVar must be assigned to a variable with the same name as its name argument.
 
   Closes #5224

pylint/checkers/base/comparison_checker.py

@@ -9,6 +9,7 @@
 
 from pylint.checkers import utils
 from pylint.checkers.base.basic_checker import _BasicChecker
+from pylint.interfaces import HIGH
 
 LITERAL_NODE_TYPES = (nodes.Const, nodes.Dict, nodes.List, nodes.Set)
 COMPARISON_OPERATORS = frozenset(("==", "!=", "<", ">", "<=", ">="))
@@ -58,6 +59,13 @@ class ComparisonChecker(_BasicChecker):
             "comparison-with-itself",
             "Used when something is compared against itself.",
         ),
+        "R0133": (
+            "Comparison between constants: '%s %s %s' is also a constant, remove the comparison and consider a refactor",
+            "comparison-of-constants",
+            "When two literals are compared with each other the result is a constant, "
+            "and 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",
+        ),
         "W0143": (
             "Comparing against a callable, did you omit the parenthesis?",
             "comparison-with-callable",
@@ -212,6 +220,24 @@ def _check_logical_tautology(self, node: nodes.Compare):
             suggestion = f"{left_operand} {operator} {right_operand}"
             self.add_message("comparison-with-itself", node=node, args=(suggestion,))
 
+    def _check_two_literals_being_compared(self, node: nodes.Compare) -> None:
+        """Check if two literals are being compared; this is always a logical tautology."""
+        left_operand = node.left
+        if not isinstance(left_operand, nodes.Const):
+            return
+
+        right_operand = node.ops[0][1]
+        if not isinstance(right_operand, nodes.Const):
+            return
+
+        operator = node.ops[0][0]
+        self.add_message(
+            "comparison-of-constants",
+            node=node,
+            args=(left_operand.value, operator, right_operand.value),
+            confidence=HIGH,
+        )
+
     def _check_callable_comparison(self, node):
         operator = node.ops[0][0]
         if operator not in COMPARISON_OPERATORS:
@@ -240,13 +266,15 @@ def _check_callable_comparison(self, node):
         "unidiomatic-typecheck",
         "literal-comparison",
         "comparison-with-itself",
+        "comparison-of-constants",
         "comparison-with-callable",
         "nan-comparison",
     )
     def visit_compare(self, node: nodes.Compare) -> None:
         self._check_callable_comparison(node)
         self._check_logical_tautology(node)
         self._check_unidiomatic_typecheck(node)
+        self._check_two_literals_being_compared(node)
         # NOTE: this checker only works with binary comparisons like 'x == 42'
         # but not 'x == y == 42'
         if len(node.ops) != 1:

tests/functional/a/access/access_to_protected_members.py

@@ -85,9 +85,9 @@ def __le__(self, other):
         Test a correct access as the access to protected member
         is inside a special method even if it is deeply nested
         """
-        if 2 > 1:
+        if 2 > 1:  # pylint: disable=comparison-of-constants
             if isinstance(other, self.__class__):
-                if "answer" == "42":
+                if "answer" == "42":  # pylint: disable=comparison-of-constants
                     return self._foo == other._foo
         return False
 

tests/functional/a/assert_on_tuple.py

@@ -1,6 +1,6 @@
 '''Assert check example'''
 
-# pylint: disable=comparison-with-itself
+# pylint: disable=comparison-with-itself comparison-of-constants
 assert (1 == 1, 2 == 2), "no error"
 assert (1 == 1, 2 == 2) # [assert-on-tuple]
 assert 1 == 1, "no error"

tests/functional/c/comparison_of_constants.py

@@ -0,0 +1,51 @@
+# pylint: disable=missing-docstring, comparison-with-itself, invalid-name
+
+
+if 2 is 2: # [literal-comparison, comparison-of-constants]
+    pass
+
+while 2 == 2: # [comparison-of-constants]
+    pass
+
+while 2 > 2: # [comparison-of-constants]
+    pass
+
+n = 2
+if 2 != n:
+    pass
+
+if n != 1 + 1:
+    pass
+
+if True == True: # [comparison-of-constants, singleton-comparison]
+    pass
+
+CONST = 24
+
+if CONST is 0: # [literal-comparison]
+    pass
+
+if CONST is 1: # [literal-comparison]
+    pass
+
+if CONST is 42: # [literal-comparison]
+    pass
+
+if  0 < CONST < 42:
+    pass
+
+if  0 < n < 42:
+    pass
+
+if  True == n != 42:
+    pass
+
+if  0 == n != 42:
+    pass
+
+
+print(0 < n < 42)
+print(0 <= n < 42 )
+print(n < 1 < n*42 < 42)
+print(42> n <= 0)
+print(0 == n > 42)

tests/functional/c/comparison_of_constants.txt

@@ -0,0 +1,9 @@
+comparison-of-constants:4:3:4:9::"Comparison between constants: '2 is 2' is also a constant, remove the comparison and consider a refactor":HIGH
+literal-comparison:4:3:4:9::Comparison to literal:UNDEFINED
+comparison-of-constants:7:6:7:12::"Comparison between constants: '2 == 2' is also a constant, remove the comparison and consider a refactor":HIGH
+comparison-of-constants:10:6:10:11::"Comparison between constants: '2 > 2' is also a constant, remove the comparison and consider a refactor":HIGH
+comparison-of-constants:20:3:20:15::"Comparison between constants: 'True == True' is also a constant, remove the comparison and consider a refactor":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

tests/functional/c/comparison_with_callable.py

@@ -1,4 +1,4 @@
-# pylint: disable = disallowed-name, missing-docstring, useless-return, invalid-name, no-self-use, line-too-long, useless-object-inheritance
+# pylint: disable = disallowed-name, missing-docstring, useless-return, invalid-name, no-self-use, line-too-long, useless-object-inheritance, comparison-of-constants
 def foo():
     return None
 

tests/functional/c/consider/consider_using_get.py

@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring,invalid-name,using-constant-test,invalid-sequence-index,undefined-variable
+# pylint: disable=missing-docstring,invalid-name,using-constant-test,invalid-sequence-index,undefined-variable, comparison-of-constants
 dictionary = {}
 key = 'key'
 

tests/functional/c/consider/consider_using_in.py

@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring, invalid-name, pointless-statement, undefined-variable, literal-comparison, line-too-long, unneeded-not, too-few-public-methods, use-implicit-booleaness-not-comparison
+# pylint: disable=missing-docstring, invalid-name, pointless-statement, undefined-variable, literal-comparison, line-too-long, unneeded-not, too-few-public-methods, use-implicit-booleaness-not-comparison, comparison-of-constants
 
 value = value1 = 1
 value2 = 2

tests/functional/d/deprecated/deprecated_methods_py38.py

@@ -1,5 +1,5 @@
 """ Functional tests for method deprecation. """
-# pylint: disable=missing-docstring, super-init-not-called, not-callable
+# pylint: disable=missing-docstring, super-init-not-called, not-callable, comparison-of-constants
 import base64
 import inspect
 import logging

tests/functional/e/excess_escapes.py

@@ -1,4 +1,4 @@
-# pylint:disable=pointless-string-statement, fixme, comparison-with-itself
+# pylint:disable=pointless-string-statement, fixme, comparison-with-itself, comparison-of-constants
 """Stray backslash escapes may be missing a raw-string prefix."""
 # pylint: disable=redundant-u-string-prefix
 

tests/functional/ext/check_elif/check_elif.py

@@ -1,4 +1,4 @@
-# pylint: disable=no-else-raise,unsupported-membership-test,using-constant-test
+# pylint: disable=no-else-raise,unsupported-membership-test,using-constant-test, comparison-of-constants
 
 """Checks use of "else if" triggers a refactor message"""
 from typing import Union, Sequence, Any, Mapping

tests/functional/ext/comparison_placement/misplaced_comparison_constant.py

@@ -1,6 +1,7 @@
 """Check that the constants are on the right side of the comparisons"""
 
 # pylint: disable=singleton-comparison, missing-docstring, too-few-public-methods, useless-object-inheritance
+# pylint: disable=comparison-of-constants
 
 class MyClass(object):
     def __init__(self):

tests/functional/ext/comparison_placement/misplaced_comparison_constant.txt

@@ -1,6 +1,6 @@
-misplaced-comparison-constant:19:11:19:17:bad_comparisons:Comparison should be i >= 5:UNDEFINED
-misplaced-comparison-constant:21:11:21:17:bad_comparisons:Comparison should be i == 1:UNDEFINED
-misplaced-comparison-constant:23:11:23:29:bad_comparisons:Comparison should be dummy_return() > 3:UNDEFINED
-misplaced-comparison-constant:25:11:25:39:bad_comparisons:Comparison should be instance.dummy_return() != 4:UNDEFINED
-misplaced-comparison-constant:27:11:27:29:bad_comparisons:Comparison should be instance.attr == 1:UNDEFINED
-misplaced-comparison-constant:29:11:29:33:bad_comparisons:Comparison should be instance.attr == 'aaa':UNDEFINED
+misplaced-comparison-constant:20:11:20:17:bad_comparisons:Comparison should be i >= 5:UNDEFINED
+misplaced-comparison-constant:22:11:22:17:bad_comparisons:Comparison should be i == 1:UNDEFINED
+misplaced-comparison-constant:24:11:24:29:bad_comparisons:Comparison should be dummy_return() > 3:UNDEFINED
+misplaced-comparison-constant:26:11:26:39:bad_comparisons:Comparison should be instance.dummy_return() != 4:UNDEFINED
+misplaced-comparison-constant:28:11:28:29:bad_comparisons:Comparison should be instance.attr == 1:UNDEFINED
+misplaced-comparison-constant:30:11:30:33:bad_comparisons:Comparison should be instance.attr == 'aaa':UNDEFINED

tests/functional/i/import_outside_toplevel.py

@@ -1,4 +1,5 @@
 # pylint: disable=unused-import,multiple-imports,no-self-use,missing-docstring,invalid-name,too-few-public-methods
+# pylint: disable=comparison-of-constants
 
 import abc
 

tests/functional/i/import_outside_toplevel.txt

@@ -1,11 +1,11 @@
-import-outside-toplevel:10:4:10:19:f:Import outside toplevel (symtable):UNDEFINED
-import-outside-toplevel:14:4:14:18:g:Import outside toplevel (os, sys):UNDEFINED
-import-outside-toplevel:18:4:18:24:h:Import outside toplevel (time):UNDEFINED
-import-outside-toplevel:22:4:22:41:i:Import outside toplevel (random, socket):UNDEFINED
-import-outside-toplevel:26:4:26:19:C:Import outside toplevel (tokenize):UNDEFINED
-import-outside-toplevel:29:8:29:21:C.j:Import outside toplevel (turtle):UNDEFINED
-import-outside-toplevel:34:8:34:23:k:Import outside toplevel (tabnanny):UNDEFINED
-import-outside-toplevel:38:4:38:39:j:Import outside toplevel (collections.defaultdict):UNDEFINED
-import-outside-toplevel:42:4:42:48:m:Import outside toplevel (math.sin, math.cos):UNDEFINED
-import-error:50:4:50:21:o:Unable to import 'notastroid':UNDEFINED
-import-outside-toplevel:50:4:50:21:o:Import outside toplevel (notastroid):UNDEFINED
+import-outside-toplevel:11:4:11:19:f:Import outside toplevel (symtable):UNDEFINED
+import-outside-toplevel:15:4:15:18:g:Import outside toplevel (os, sys):UNDEFINED
+import-outside-toplevel:19:4:19:24:h:Import outside toplevel (time):UNDEFINED
+import-outside-toplevel:23:4:23:41:i:Import outside toplevel (random, socket):UNDEFINED
+import-outside-toplevel:27:4:27:19:C:Import outside toplevel (tokenize):UNDEFINED
+import-outside-toplevel:30:8:30:21:C.j:Import outside toplevel (turtle):UNDEFINED
+import-outside-toplevel:35:8:35:23:k:Import outside toplevel (tabnanny):UNDEFINED
+import-outside-toplevel:39:4:39:39:j:Import outside toplevel (collections.defaultdict):UNDEFINED
+import-outside-toplevel:43:4:43:48:m:Import outside toplevel (math.sin, math.cos):UNDEFINED
+import-error:51:4:51:21:o:Unable to import 'notastroid':UNDEFINED
+import-outside-toplevel:51:4:51:21:o:Import outside toplevel (notastroid):UNDEFINED

tests/functional/l/literal_comparison.py

@@ -1,13 +1,13 @@
 # pylint: disable=missing-docstring, comparison-with-itself
 
 
-if 2 is 2: # [literal-comparison]
+if 2 is 2: # [literal-comparison, comparison-of-constants]
     pass
 
-if "a" is b"a": # [literal-comparison]
+if "a" is b"a": # [literal-comparison, comparison-of-constants]
     pass
 
-if 2.0 is 3.0: # [literal-comparison]
+if 2.0 is 3.0: # [literal-comparison, comparison-of-constants]
     pass
 
 if () is (1, 2, 3):

tests/functional/l/literal_comparison.txt

@@ -1,5 +1,8 @@
+comparison-of-constants:4:3:4:9::"Comparison between constants: '2 is 2' is also a constant, remove the comparison and consider a refactor":HIGH
 literal-comparison:4:3:4:9::Comparison to literal:UNDEFINED
+comparison-of-constants:7:3:7:14::"Comparison between constants: 'a is b'a'' is also a constant, remove the comparison and consider a refactor":HIGH
 literal-comparison:7:3:7:14::Comparison to literal:UNDEFINED
+comparison-of-constants:10:3:10:13::"Comparison between constants: '2.0 is 3.0' is also a constant, remove the comparison and consider a refactor":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

tests/functional/l/logical_tautology.py

@@ -11,21 +11,21 @@ def foo():
         return True
     elif arg <= arg: # [comparison-with-itself]
         return True
-    elif None == None: # [comparison-with-itself]
+    elif None == None: # [comparison-of-constants, comparison-with-itself]
         return None
-    elif 786 == 786: # [comparison-with-itself]
+    elif 786 == 786: # [comparison-of-constants, comparison-with-itself]
         return True
-    elif 786 is 786: # [comparison-with-itself]
+    elif 786 is 786: # [comparison-of-constants, comparison-with-itself]
         return True
-    elif 786 is not 786: # [comparison-with-itself]
+    elif 786 is not 786: # [comparison-of-constants, comparison-with-itself]
         return True
     elif arg is arg: # [comparison-with-itself]
         return True
     elif arg is not arg: # [comparison-with-itself]
         return True
-    elif True is True: # [comparison-with-itself]
+    elif True is True: # [comparison-of-constants, comparison-with-itself]
         return True
-    elif 666 == 786:
+    elif 666 == 786: # [comparison-of-constants]
         return False
     else:
         return None

tests/functional/l/logical_tautology.txt

@@ -2,11 +2,17 @@ comparison-with-itself:6:7:6:17:foo:Redundant comparison - arg == arg:UNDEFINED
 comparison-with-itself:8:9:8:19:foo:Redundant comparison - arg != arg:UNDEFINED
 comparison-with-itself:10:9:10:18:foo:Redundant comparison - arg > arg:UNDEFINED
 comparison-with-itself:12:9:12:19:foo:Redundant comparison - arg <= arg:UNDEFINED
+comparison-of-constants:14:9:14:21:foo:"Comparison between constants: 'None == None' is also a constant, remove the comparison and consider a refactor":HIGH
 comparison-with-itself:14:9:14:21:foo:Redundant comparison - None == None:UNDEFINED
+comparison-of-constants:16:9:16:19:foo:"Comparison between constants: '786 == 786' is also a constant, remove the comparison and consider a refactor":HIGH
 comparison-with-itself:16:9:16:19:foo:Redundant comparison - 786 == 786:UNDEFINED
+comparison-of-constants:18:9:18:19:foo:"Comparison between constants: '786 is 786' is also a constant, remove the comparison and consider a refactor":HIGH
 comparison-with-itself:18:9:18:19:foo:Redundant comparison - 786 is 786:UNDEFINED
+comparison-of-constants:20:9:20:23:foo:"Comparison between constants: '786 is not 786' is also a constant, remove the comparison and consider a refactor":HIGH
 comparison-with-itself:20:9:20:23:foo:Redundant comparison - 786 is not 786:UNDEFINED
 comparison-with-itself:22:9:22:19:foo:Redundant comparison - arg is arg:UNDEFINED
 comparison-with-itself:24:9:24:23:foo:Redundant comparison - arg is not arg:UNDEFINED
+comparison-of-constants:26:9:26:21:foo:"Comparison between constants: 'True is True' is also a constant, remove the comparison and consider a refactor":HIGH
 comparison-with-itself:26:9:26:21:foo:Redundant comparison - True is True:UNDEFINED
+comparison-of-constants:28:9:28:19:foo:"Comparison between constants: '666 == 786' is also a constant, remove the comparison and consider a refactor":HIGH
 comparison-with-itself:36:18:36:28:bar:Redundant comparison - arg != arg:UNDEFINED

tests/functional/m/membership_protocol.py

@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring,pointless-statement,expression-not-assigned,too-few-public-methods,import-error,wrong-import-position,no-else-return, comparison-with-itself, useless-object-inheritance, redundant-u-string-prefix
+# pylint: disable=missing-docstring,pointless-statement,expression-not-assigned,too-few-public-methods,import-error,wrong-import-position,no-else-return, comparison-with-itself, useless-object-inheritance, redundant-u-string-prefix comparison-of-constants
 
 # standard types
 1 in [1, 2, 3]

tests/functional/m/misplaced_bare_raise.py

@@ -1,5 +1,6 @@
 # pylint: disable=missing-docstring, broad-except, unreachable, try-except-raise, raise-missing-from
 # pylint: disable=unused-variable, too-few-public-methods, invalid-name, useless-object-inheritance
+# pylint: disable=comparison-of-constants
 
 try:
     raise # [misplaced-bare-raise]

tests/functional/m/misplaced_bare_raise.txt

@@ -1,7 +1,7 @@
-misplaced-bare-raise:5:4:5:9::The raise statement is not inside an except clause:UNDEFINED
-misplaced-bare-raise:35:16:35:21:test1.best:The raise statement is not inside an except clause:UNDEFINED
-misplaced-bare-raise:38:4:38:9:test1:The raise statement is not inside an except clause:UNDEFINED
-misplaced-bare-raise:39:0:39:5::The raise statement is not inside an except clause:UNDEFINED
-misplaced-bare-raise:48:4:48:9::The raise statement is not inside an except clause:UNDEFINED
-misplaced-bare-raise:56:4:56:9:A:The raise statement is not inside an except clause:UNDEFINED
-misplaced-bare-raise:67:4:67:9::The raise statement is not inside an except clause:UNDEFINED
+misplaced-bare-raise:6:4:6:9::The raise statement is not inside an except clause:UNDEFINED
+misplaced-bare-raise:36:16:36:21:test1.best:The raise statement is not inside an except clause:UNDEFINED
+misplaced-bare-raise:39:4:39:9:test1:The raise statement is not inside an except clause:UNDEFINED
+misplaced-bare-raise:40:0:40:5::The raise statement is not inside an except clause:UNDEFINED
+misplaced-bare-raise:49:4:49:9::The raise statement is not inside an except clause:UNDEFINED
+misplaced-bare-raise:57:4:57:9:A:The raise statement is not inside an except clause:UNDEFINED
+misplaced-bare-raise:68:4:68:9::The raise statement is not inside an except clause:UNDEFINED

tests/functional/n/nan_comparison_check.py

@@ -1,5 +1,5 @@
 # pylint: disable=missing-docstring, invalid-name
-# pylint: disable=literal-comparison,comparison-with-itself, import-error
+# pylint: disable=literal-comparison,comparison-with-itself, import-error, comparison-of-constants
 """Test detection of NaN value comparison."""
 import numpy
 

tests/functional/s/singleton_comparison.py

@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring, invalid-name, literal-comparison, comparison-with-itself
+# pylint: disable=missing-docstring, invalid-name, literal-comparison, comparison-with-itself, comparison-of-constants
 x = 42
 a = x is None
 b = x == None  # [singleton-comparison]

tests/functional/s/superfluous_parens.py

@@ -20,9 +20,9 @@
 del DICT['a']
 
 B = [x for x in ((3, 4))]
-C = [x for x in ((3, 4) if 1 > 0 else (5, 6))]
-D = [x for x in ((3, 4) if 1 > 0 else ((5, 6)))]
-E = [x for x in ((3, 4) if 1 > 0 else ((((5, 6)))))]
+C = [x for x in ((3, 4) if 1 > 0 else (5, 6))]  # pylint: disable=comparison-of-constants
+D = [x for x in ((3, 4) if 1 > 0 else ((5, 6)))]  # pylint: disable=comparison-of-constants
+E = [x for x in ((3, 4) if 1 > 0 else ((((5, 6)))))]  # pylint: disable=comparison-of-constants
 
 # Test assertions
 F = "Version 1.0"
@@ -74,4 +74,4 @@ def __iter__(self):
 
 M = A is not (A <= H)
 M = True is not (M == K)
-M = True is not (True is not False)
+M = True is not (True is not False)  # pylint: disable=comparison-of-constants

tests/functional/t/too/too_many_boolean_expressions.py

@@ -1,6 +1,6 @@
 """Checks for if statements containing too many boolean expressions"""
 
-# pylint: disable=invalid-name, comparison-with-itself, chained-comparison, condition-evals-to-constant
+# pylint: disable=invalid-name, comparison-with-itself, chained-comparison, condition-evals-to-constant, comparison-of-constants
 
 x = y = z = 5
 if x > -5 and x < 5 and y > -5 and y < 5 and z > -5 and z < 5:  # [too-many-boolean-expressions]