From 6c6ec4a713118136ccdd9117693078e15fcccbbe Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Thu, 15 Oct 2015 21:45:00 +0300 Subject: Make iterable/mapping checker more smart and refactor it. Now it ignores errors inside mixins declarations and is able to recognize iterable/mapping metaclasses. --- pylint/checkers/typecheck.py | 132 ++++++++++++------------ pylint/test/functional/iterable_context.py | 26 ++++- pylint/test/functional/iterable_context_py2.py | 18 ++++ pylint/test/functional/iterable_context_py2.rc | 3 + pylint/test/functional/iterable_context_py2.txt | 1 + pylint/test/functional/iterable_context_py3.py | 18 ++++ pylint/test/functional/iterable_context_py3.rc | 3 + pylint/test/functional/iterable_context_py3.txt | 1 + pylint/test/functional/mapping_context.py | 33 +++++- pylint/test/functional/mapping_context.txt | 3 +- pylint/test/functional/mapping_context_py2.py | 19 ++++ pylint/test/functional/mapping_context_py2.rc | 3 + pylint/test/functional/mapping_context_py2.txt | 1 + pylint/test/functional/mapping_context_py3.py | 19 ++++ pylint/test/functional/mapping_context_py3.rc | 3 + pylint/test/functional/mapping_context_py3.txt | 1 + pylint/test/unittest_checker_typecheck.py | 8 -- 17 files changed, 211 insertions(+), 81 deletions(-) create mode 100644 pylint/test/functional/iterable_context_py2.py create mode 100644 pylint/test/functional/iterable_context_py2.rc create mode 100644 pylint/test/functional/iterable_context_py2.txt create mode 100644 pylint/test/functional/iterable_context_py3.py create mode 100644 pylint/test/functional/iterable_context_py3.rc create mode 100644 pylint/test/functional/iterable_context_py3.txt create mode 100644 pylint/test/functional/mapping_context_py2.py create mode 100644 pylint/test/functional/mapping_context_py2.rc create mode 100644 pylint/test/functional/mapping_context_py2.txt create mode 100644 pylint/test/functional/mapping_context_py3.py create mode 100644 pylint/test/functional/mapping_context_py3.rc create mode 100644 pylint/test/functional/mapping_context_py3.txt diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 83d9742..5095d72 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -43,6 +43,10 @@ _ZOPE_DEPRECATED = ( ) BUILTINS = six.moves.builtins.__name__ STR_FORMAT = "%s.str.format" % BUILTINS +ITER_METHOD = '__iter__' +NEXT_METHOD = 'next' if six.PY2 else '__next__' +GETITEM_METHOD = '__getitem__' +KEYS_METHOD = 'keys' def _unflatten(iterable): @@ -83,6 +87,13 @@ def _is_owner_ignored(owner, name, ignored_classes, ignored_modules): return any(name == ignore or qname == ignore for ignore in ignored_classes) +def _hasattr(value, attr): + try: + value.getattr(attr) + return True + except astroid.NotFoundError: + return False + def _is_comprehension(node): comprehensions = (astroid.ListComp, astroid.SetComp, @@ -90,62 +101,29 @@ def _is_comprehension(node): return isinstance(node, comprehensions) -def _is_class_def(node): - return isinstance(node, astroid.ClassDef) - - def _is_iterable(value): - if _is_class_def(value): - return False - try: - value.getattr('__iter__') - return True - except astroid.NotFoundError: - pass - try: - # this checks works for string types - value.getattr('__getitem__') - return True - except astroid.NotFoundError: - pass - return False + # '__iter__' is for standard iterables + # '__getitem__' is for strings and other old-style iterables + return _hasattr(value, ITER_METHOD) or _hasattr(value, GETITEM_METHOD) def _is_iterator(value): - if _is_class_def(value): - return False - try: - value.getattr('__iter__') - if six.PY2: - value.getattr('next') - elif six.PY3: - value.getattr('__next__') - return True - except astroid.NotFoundError: - return False + return _hasattr(value, NEXT_METHOD) and _hasattr(value, ITER_METHOD) -def _is_old_style_iterator(value): - if _is_class_def(value): - return False - try: - value.getattr('__getitem__') - value.getattr('__len__') - return True - except astroid.NotFoundError: - return False +def _is_mapping(value): + return _hasattr(value, GETITEM_METHOD) and _hasattr(value, KEYS_METHOD) -def _is_mapping(value): - if _is_class_def(value): - return False - try: - value.getattr('__getitem__') - value.getattr('__iter__') - value.getattr('keys') - return True - except astroid.NotFoundError: - return False +def _is_inside_mixin_declaration(node): + while node is not None: + if isinstance(node, astroid.ClassDef): + name = getattr(node, 'name', None) + if name is not None and name.lower().endswith("mixin"): + return True + node = node.parent + return False + MSGS = { 'E1101': ('%s %r has no %r member', @@ -884,35 +862,57 @@ class IterableChecker(BaseChecker): } def _check_iterable(self, node, root_node): - # for/set/dict-comprehensions can't be infered with astroid, - # so we check for them before checking infered value - if _is_comprehension(node): + # for/set/dict-comprehensions can't be infered with astroid + # so we have to check for them explicitly + if _is_comprehension(node) or _is_inside_mixin_declaration(node): return + infered = helpers.safe_infer(node) - if infered is None: - return - if infered is astroid.YES: - return - if _is_iterable(infered): - return - if _is_iterator(infered): - return - if _is_old_style_iterator(infered): + if infered is None or infered is astroid.YES: return + + if isinstance(infered, astroid.ClassDef): + if not helpers.has_known_bases(infered): + return + # classobj can only be iterable if it has an iterable metaclass + meta = infered.metaclass() + if meta is not None: + if _is_iterable(meta): + return + if _is_iterator(meta): + return + + if isinstance(infered, astroid.Instance): + if not helpers.has_known_bases(infered): + return + if _is_iterable(infered) or _is_iterator(infered): + return + self.add_message('not-an-iterable', args=node.as_string(), node=root_node) def _check_mapping(self, node, root_node): - if isinstance(node, astroid.DictComp): + if isinstance(node, astroid.DictComp) or _is_inside_mixin_declaration(node): return + infered = helpers.safe_infer(node) - if infered is None: - return - if infered is astroid.YES: - return - if _is_mapping(infered): + if infered is None or infered is astroid.YES: return + + if isinstance(infered, astroid.ClassDef): + if not helpers.has_known_bases(infered): + return + meta = infered.metaclass() + if meta is not None and _is_mapping(meta): + return + + if isinstance(infered, astroid.Instance): + if not helpers.has_known_bases(infered): + return + if _is_mapping(infered): + return + self.add_message('not-a-mapping', args=node.as_string(), node=root_node) diff --git a/pylint/test/functional/iterable_context.py b/pylint/test/functional/iterable_context.py index 47400f9..8dfcbbe 100644 --- a/pylint/test/functional/iterable_context.py +++ b/pylint/test/functional/iterable_context.py @@ -2,7 +2,7 @@ Checks that primitive values are not used in an iterating/mapping context. """ -# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-init,no-self-use +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-init,no-self-use,import-error,unused-argument,bad-mcs-method-argument from __future__ import print_function # primitives @@ -115,3 +115,27 @@ for i in 8.5: # [not-an-iterable] for i in 10: # [not-an-iterable] pass + + +# skip uninferable instances +from some_missing_module import Iterable + +class MyClass(Iterable): + pass + +m = MyClass() +for i in m: + print(i) + +# skip checks if statement is inside mixin class +class ManagedAccessViewMixin(object): + access_requirements = None + + def get_access_requirements(self): + return self.access_requirements + + def dispatch(self, *_args, **_kwargs): + klasses = self.get_access_requirements() + + for requirement in klasses: + print(requirement) diff --git a/pylint/test/functional/iterable_context_py2.py b/pylint/test/functional/iterable_context_py2.py new file mode 100644 index 0000000..8687f84 --- /dev/null +++ b/pylint/test/functional/iterable_context_py2.py @@ -0,0 +1,18 @@ +""" +Checks that iterable metaclasses are recognized by pylint. +""" +# pylint: disable=missing-docstring,too-few-public-methods,no-init,no-self-use,unused-argument,bad-mcs-method-argument + +# metaclasses as iterables +class Meta(type): + def __iter__(self): + return iter((1, 2, 3)) + +class SomeClass(object): + __metaclass__ = Meta + + +for i in SomeClass: + print i +for i in SomeClass(): # [not-an-iterable] + print i diff --git a/pylint/test/functional/iterable_context_py2.rc b/pylint/test/functional/iterable_context_py2.rc new file mode 100644 index 0000000..61e01ea --- /dev/null +++ b/pylint/test/functional/iterable_context_py2.rc @@ -0,0 +1,3 @@ +[testoptions] +max_pyver=2.7 + diff --git a/pylint/test/functional/iterable_context_py2.txt b/pylint/test/functional/iterable_context_py2.txt new file mode 100644 index 0000000..8de579a --- /dev/null +++ b/pylint/test/functional/iterable_context_py2.txt @@ -0,0 +1 @@ +not-an-iterable:17::Non-iterable value SomeClass() is used in an iterating context diff --git a/pylint/test/functional/iterable_context_py3.py b/pylint/test/functional/iterable_context_py3.py new file mode 100644 index 0000000..cb2a505 --- /dev/null +++ b/pylint/test/functional/iterable_context_py3.py @@ -0,0 +1,18 @@ +""" +Checks that iterable metaclasses are recognized by pylint. +""" +# pylint: disable=missing-docstring,too-few-public-methods,no-init,no-self-use,unused-argument,bad-mcs-method-argument + +# metaclasses as iterables +class Meta(type): + def __iter__(self): + return iter((1, 2, 3)) + +class SomeClass(metaclass=Meta): + pass + + +for i in SomeClass: + print(i) +for i in SomeClass(): # [not-an-iterable] + print(i) diff --git a/pylint/test/functional/iterable_context_py3.rc b/pylint/test/functional/iterable_context_py3.rc new file mode 100644 index 0000000..9bf6df0 --- /dev/null +++ b/pylint/test/functional/iterable_context_py3.rc @@ -0,0 +1,3 @@ +[testoptions] +min_pyver=3.0 + diff --git a/pylint/test/functional/iterable_context_py3.txt b/pylint/test/functional/iterable_context_py3.txt new file mode 100644 index 0000000..8de579a --- /dev/null +++ b/pylint/test/functional/iterable_context_py3.txt @@ -0,0 +1 @@ +not-an-iterable:17::Non-iterable value SomeClass() is used in an iterating context diff --git a/pylint/test/functional/mapping_context.py b/pylint/test/functional/mapping_context.py index d9e0d2c..cfab8dc 100644 --- a/pylint/test/functional/mapping_context.py +++ b/pylint/test/functional/mapping_context.py @@ -1,7 +1,7 @@ """ Checks that only valid values are used in a mapping context. """ -# pylint: disable=missing-docstring,invalid-name,too-few-public-methods +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-self-use,import-error from __future__ import print_function @@ -22,9 +22,6 @@ class CustomMapping(object): def __init__(self): self.data = dict(a=1, b=2, c=3, d=4, e=5) - def __iter__(self): - return iter(self.data) - def __getitem__(self, key): return self.data[key] @@ -32,5 +29,31 @@ class CustomMapping(object): return self.data.keys() test(**CustomMapping()) - test(**CustomMapping) # [not-a-mapping] + +class NotMapping(object): + pass + +test(**NotMapping()) # [not-a-mapping] + +# skip checks if statement is inside mixin class +class SomeMixin(object): + kwargs = None + + def get_kwargs(self): + return self.kwargs + + def run(self, **kwargs): + print(kwargs) + + def dispatch(self): + kws = self.get_kwargs() + self.run(**kws) + +# skip uninferable instances +from some_missing_module import Mapping + +class MyClass(Mapping): + pass + +test(**MyClass()) diff --git a/pylint/test/functional/mapping_context.txt b/pylint/test/functional/mapping_context.txt index f27ca97..201da1a 100644 --- a/pylint/test/functional/mapping_context.txt +++ b/pylint/test/functional/mapping_context.txt @@ -1 +1,2 @@ -not-a-mapping:36::Non-mapping value CustomMapping is used in a mapping context +not-a-mapping:32::Non-mapping value CustomMapping is used in a mapping context +not-a-mapping:37::Non-mapping value NotMapping() is used in a mapping context diff --git a/pylint/test/functional/mapping_context_py2.py b/pylint/test/functional/mapping_context_py2.py new file mode 100644 index 0000000..afe4400 --- /dev/null +++ b/pylint/test/functional/mapping_context_py2.py @@ -0,0 +1,19 @@ +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods +from __future__ import print_function + + +def test(**kwargs): + print(kwargs) + +# metaclasses as mappings +class Meta(type): + def __getitem__(self, key): + return ord(key) + def keys(self): + return ['a', 'b', 'c'] + +class SomeClass(object): + __metaclass__ = Meta + +test(**SomeClass) +test(**SomeClass()) # [not-a-mapping] diff --git a/pylint/test/functional/mapping_context_py2.rc b/pylint/test/functional/mapping_context_py2.rc new file mode 100644 index 0000000..61e01ea --- /dev/null +++ b/pylint/test/functional/mapping_context_py2.rc @@ -0,0 +1,3 @@ +[testoptions] +max_pyver=2.7 + diff --git a/pylint/test/functional/mapping_context_py2.txt b/pylint/test/functional/mapping_context_py2.txt new file mode 100644 index 0000000..59cca6c --- /dev/null +++ b/pylint/test/functional/mapping_context_py2.txt @@ -0,0 +1 @@ +not-a-mapping:19::Non-mapping value SomeClass() is used in a mapping context diff --git a/pylint/test/functional/mapping_context_py3.py b/pylint/test/functional/mapping_context_py3.py new file mode 100644 index 0000000..042d4d0 --- /dev/null +++ b/pylint/test/functional/mapping_context_py3.py @@ -0,0 +1,19 @@ +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-self-use +from __future__ import print_function + +def test(**kwargs): + print(kwargs) + +# metaclasses as mappings +class Meta(type): + def __getitem__(cls, key): + return ord(key) + + def keys(cls): + return ['a', 'b', 'c'] + +class SomeClass(metaclass=Meta): + pass + +test(**SomeClass) +test(**SomeClass()) # [not-a-mapping] diff --git a/pylint/test/functional/mapping_context_py3.rc b/pylint/test/functional/mapping_context_py3.rc new file mode 100644 index 0000000..9bf6df0 --- /dev/null +++ b/pylint/test/functional/mapping_context_py3.rc @@ -0,0 +1,3 @@ +[testoptions] +min_pyver=3.0 + diff --git a/pylint/test/functional/mapping_context_py3.txt b/pylint/test/functional/mapping_context_py3.txt new file mode 100644 index 0000000..59cca6c --- /dev/null +++ b/pylint/test/functional/mapping_context_py3.txt @@ -0,0 +1 @@ +not-a-mapping:19::Non-mapping value SomeClass() is used in a mapping context diff --git a/pylint/test/unittest_checker_typecheck.py b/pylint/test/unittest_checker_typecheck.py index b89a81b..0aaa8a5 100644 --- a/pylint/test/unittest_checker_typecheck.py +++ b/pylint/test/unittest_checker_typecheck.py @@ -1,19 +1,11 @@ """Unittest for the type checker.""" import unittest -import sys from astroid import test_utils from pylint.checkers import typecheck from pylint.testutils import CheckerTestCase, Message, set_config -def python33_and_newer(test): - """ - Decorator for any tests that will fail if launched not with Python 3.3+. - """ - return unittest.skipIf(sys.version_info < (3, 3), - 'Python 3.2 and older')(test) - class TypeCheckerTest(CheckerTestCase): "Tests for pylint.checkers.typecheck" CHECKER_CLASS = typecheck.TypeChecker -- cgit v1.2.1