From dbced8d8a694a3bd3c2e17a8e1ed42188afc02d8 Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sun, 17 Oct 2021 09:01:43 +0100 Subject: 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 --- ChangeLog | 5 +++++ pylint/checkers/base.py | 10 +++++++--- tests/functional/b/bad_reversed_sequence.py | 4 ---- tests/functional/b/bad_reversed_sequence.txt | 7 +++---- tests/functional/b/bad_reversed_sequence_py37.py | 10 ++++++++++ tests/functional/b/bad_reversed_sequence_py37.txt | 3 ++- tests/functional/b/bad_reversed_sequence_py38.py | 10 ++++++++++ 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})) -- cgit v1.2.1