From Python 3.8 onwards classes inheriting from dict are reversible (#5169)

* From Python 3.8 onwards classes inheriting from dict are reversible

This generalises an earlier change to the bad-reversed-sequence
checker in Python 3.8 onwards: dicts were already being treated as
reversible, but so should any class inheriting from dict.

Fixes #4981

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

7 files changed, 37 insertions, 12 deletions

diff --git a/ChangeLog b/ChangeLog
index 029eee735..6fc6d0f6c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -47,6 +47,11 @@ Release date: TBA
 * Use ``py-version`` setting for alternative union syntax check (PEP 604),
   instead of the Python interpreter version.
 
+* Subclasses of ``dict`` are regarded as reversible by the ``bad-reversed-sequence`` checker
+  (Python 3.8 onwards).
+
+  Closes #4981
+
 * Added new checker ``use-implicit-booleaness-not-comparison``: Emitted when
   collection literal comparison is being used to check for emptiness.
 
diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index 1c67e2fef..5507bde74 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -84,6 +84,7 @@ from pylint.checkers.utils import (
 )
 from pylint.reporters.ureports import nodes as reporter_nodes
 from pylint.utils import LinterStats
+from pylint.utils.utils import get_global_option
 
 if sys.version_info >= (3, 8):
     from typing import Literal
@@ -1085,6 +1086,8 @@ class BasicChecker(_BasicChecker):
 
     def open(self):
         """initialize visit variables and statistics"""
+        py_version = get_global_option(self, "py-version")
+        self._py38_plus = py_version >= (3, 8)
         self._tryfinallys = []
         self.linter.stats.reset_node_count()
 
@@ -1528,15 +1531,16 @@ class BasicChecker(_BasicChecker):
             if isinstance(argument, (nodes.List, nodes.Tuple)):
                 return
 
-            if isinstance(argument, astroid.Instance):
+            # dicts are reversible, but only from Python 3.8 onwards. Prior to
+            # that, any class based on dict must explicitly provide a
+            # __reversed__ method
+            if not self._py38_plus and isinstance(argument, astroid.Instance):
                 if any(
                     ancestor.name == "dict" and utils.is_builtin_object(ancestor)
                     for ancestor in itertools.chain(
                         (argument._proxied,), argument._proxied.ancestors()
                     )
                 ):
-                    # Mappings aren't accepted by reversed(), unless
-                    # they provide explicitly a __reversed__ method.
                     try:
                         argument.locals[REVERSED_PROTOCOL_METHOD]
                     except KeyError:
diff --git a/tests/functional/b/bad_reversed_sequence.py b/tests/functional/b/bad_reversed_sequence.py
index 2450d960b..3abdca25f 100644
--- a/tests/functional/b/bad_reversed_sequence.py
+++ b/tests/functional/b/bad_reversed_sequence.py
@@ -28,9 +28,6 @@ class SecondBadReversed(object):
     def __getitem__(self, index):
         return index
 
-class ThirdBadReversed(dict):
-    """ dict subclass """
-
 def uninferable(seq):
     """ This can't be inferred at this moment,
     make sure we don't have a false positive.
@@ -50,7 +47,6 @@ def test(path):
     seq = reversed(BadReversed()) # [bad-reversed-sequence]
     seq = reversed(SecondBadReversed()) # [bad-reversed-sequence]
     seq = reversed(range(100))
-    seq = reversed(ThirdBadReversed()) # [bad-reversed-sequence]
     seq = reversed(lambda: None) # [bad-reversed-sequence]
     seq = reversed(deque([]))
     seq = reversed("123")
diff --git a/tests/functional/b/bad_reversed_sequence.txt b/tests/functional/b/bad_reversed_sequence.txt
index 051bd11ba..f0597356c 100644
--- a/tests/functional/b/bad_reversed_sequence.txt
+++ b/tests/functional/b/bad_reversed_sequence.txt
@@ -1,7 +1,6 @@
+bad-reversed-sequence:40:10:test:The first reversed() argument is not a sequence
 bad-reversed-sequence:43:10:test:The first reversed() argument is not a sequence
-bad-reversed-sequence:46:10:test:The first reversed() argument is not a sequence
+bad-reversed-sequence:44:10:test:The first reversed() argument is not a sequence
 bad-reversed-sequence:47:10:test:The first reversed() argument is not a sequence
+bad-reversed-sequence:48:10:test:The first reversed() argument is not a sequence
 bad-reversed-sequence:50:10:test:The first reversed() argument is not a sequence
-bad-reversed-sequence:51:10:test:The first reversed() argument is not a sequence
-bad-reversed-sequence:53:10:test:The first reversed() argument is not a sequence
-bad-reversed-sequence:54:10:test:The first reversed() argument is not a sequence
diff --git a/tests/functional/b/bad_reversed_sequence_py37.py b/tests/functional/b/bad_reversed_sequence_py37.py
index a28c84cc0..5a0b2124c 100644
--- a/tests/functional/b/bad_reversed_sequence_py37.py
+++ b/tests/functional/b/bad_reversed_sequence_py37.py
@@ -1,2 +1,12 @@
 """ Dictionaries are reversible starting on python 3.8"""
+
+# pylint: disable=missing-docstring
+
 reversed({'a': 1, 'b': 2}) # [bad-reversed-sequence]
+
+
+class InheritDict(dict):
+    pass
+
+
+reversed(InheritDict({'a': 1, 'b': 2})) # [bad-reversed-sequence]
diff --git a/tests/functional/b/bad_reversed_sequence_py37.txt b/tests/functional/b/bad_reversed_sequence_py37.txt
index 1e1b939ab..d87c84690 100644
--- a/tests/functional/b/bad_reversed_sequence_py37.txt
+++ b/tests/functional/b/bad_reversed_sequence_py37.txt
@@ -1 +1,2 @@
-bad-reversed-sequence:2:::The first reversed() argument is not a sequence
+bad-reversed-sequence:5:::The first reversed() argument is not a sequence
+bad-reversed-sequence:12:::The first reversed() argument is not a sequence
diff --git a/tests/functional/b/bad_reversed_sequence_py38.py b/tests/functional/b/bad_reversed_sequence_py38.py
index bbfdd97c3..4651eb4f8 100644
--- a/tests/functional/b/bad_reversed_sequence_py38.py
+++ b/tests/functional/b/bad_reversed_sequence_py38.py
@@ -1,2 +1,12 @@
 """ Dictionaries are reversible starting on python 3.8"""
+
+# pylint: disable=missing-docstring
+
 reversed({'a': 1, 'b': 2})
+
+
+class InheritDict(dict):
+    """Inherits from dict"""
+
+
+reversed(InheritDict({'a': 1, 'b': 2}))