diff options
author | Claudiu Popa <pcmanticore@gmail.com> | 2015-10-18 18:41:13 +0300 |
---|---|---|
committer | Claudiu Popa <pcmanticore@gmail.com> | 2015-10-18 18:41:13 +0300 |
commit | 0e9b80974435c9c9cdeb6165de64dc24986c8897 (patch) | |
tree | cec7e8070513571385aefff8c0a8c8fecd449710 | |
parent | 5c8335754a164bc8073bc8623a887894fe4e8c24 (diff) | |
parent | b12c2803500c0b883b3a12b4cfc0b174ac52c4f8 (diff) | |
download | pylint-0e9b80974435c9c9cdeb6165de64dc24986c8897.tar.gz |
Merged in dmand/pylint/iterable-checker (pull request #282)
Implement checker for values in iterable/mapping context
23 files changed, 478 insertions, 1 deletions
@@ -2,6 +2,11 @@ ChangeLog for Pylint -------------------- -- + * Add new errors, 'not-an-iterable', emitted when non-iterable value + is used in an iterating context (starargs, for-statement, + comprehensions, etc), and 'not-a-mapping', emitted when non-mapping + value is used in a mapping context. Closes issue #563. + * Make 'no-self-use' checker not emit a warning if there is a 'super()' call inside the method. Closes issue #667. diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 66ac05b..80b51b5 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -44,6 +44,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): @@ -84,6 +88,44 @@ 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, + astroid.DictComp) + return isinstance(node, comprehensions) + + +def _is_iterable(value): + # '__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): + return _hasattr(value, NEXT_METHOD) and _hasattr(value, ITER_METHOD) + + +def _is_mapping(value): + return _hasattr(value, GETITEM_METHOD) and _hasattr(value, KEYS_METHOD) + + +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', 'no-member', @@ -791,6 +833,124 @@ accessed. Python regular expressions are accepted.'} args=str(error), node=node) +class IterableChecker(BaseChecker): + """ + Checks for non-iterables used in an iterable context. + Contexts include: + - for-statement + - starargs in function call + - `yield from`-statement + - list, dict and set comprehensions + - generator expressions + Also checks for non-mappings in function call kwargs. + """ + + __implements__ = (IAstroidChecker,) + name = 'iterable_check' + + msgs = {'E1132': ('Non-iterable value %s is used in an iterating context', + 'not-an-iterable', + 'Used when a non-iterable value is used in place where' + 'iterable is expected'), + 'E1133': ('Non-mapping value %s is used in a mapping context', + 'not-a-mapping', + 'Used when a non-mapping value is used in place where' + 'mapping is expected'), + } + + def _check_iterable(self, node, root_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 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) or _is_inside_mixin_declaration(node): + return + + infered = helpers.safe_infer(node) + 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) + + @check_messages('not-an-iterable') + def visit_for(self, node): + self._check_iterable(node.iter, node) + + @check_messages('not-an-iterable') + def visit_yieldfrom(self, node): + self._check_iterable(node.value, node) + + @check_messages('not-an-iterable', 'not-a-mapping') + def visit_call(self, node): + for stararg in node.starargs: + self._check_iterable(stararg.value, node) + for kwarg in node.kwargs: + self._check_mapping(kwarg.value, node) + + @check_messages('not-an-iterable') + def visit_listcomp(self, node): + for gen in node.generators: + self._check_iterable(gen.iter, node) + + @check_messages('not-an-iterable') + def visit_dictcomp(self, node): + for gen in node.generators: + self._check_iterable(gen.iter, node) + + @check_messages('not-an-iterable') + def visit_setcomp(self, node): + for gen in node.generators: + self._check_iterable(gen.iter, node) + + @check_messages('not-an-iterable') + def visit_generatorexp(self, node): + for gen in node.generators: + self._check_iterable(gen.iter, node) + + def register(linter): """required method to auto register this checker """ linter.register_checker(TypeChecker(linter)) + linter.register_checker(IterableChecker(linter)) diff --git a/pylint/test/functional/iterable_context.py b/pylint/test/functional/iterable_context.py new file mode 100644 index 0000000..8dfcbbe --- /dev/null +++ b/pylint/test/functional/iterable_context.py @@ -0,0 +1,141 @@ +""" +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,import-error,unused-argument,bad-mcs-method-argument +from __future__ import print_function + +# primitives +numbers = [1, 2, 3] + +for i in numbers: + pass + +for i in iter(numbers): + pass + +for i in "123": + pass + +for i in u"123": + pass + +for i in b"123": + pass + +for i in bytearray(b"123"): + pass + +for i in set(numbers): + pass + +for i in frozenset(numbers): + pass + +for i in dict(a=1, b=2): + pass + +# comprehensions +for i in [x for x in range(10)]: + pass + +for i in {x for x in range(1, 100, 2)}: + pass + +for i in {x: 10 - x for x in range(10)}: + pass + +# generators +def powers_of_two(): + k = 0 + while k < 10: + yield 2 ** k + k += 1 + +for i in powers_of_two(): + pass + +for i in powers_of_two: # [not-an-iterable] + pass + +# check for custom iterators +class A(object): + pass + +class B(object): + def __iter__(self): + return self + + def __next__(self): + return 1 + + def next(self): + return 1 + +class C(object): + "old-style iterator" + def __getitem__(self, k): + if k > 10: + raise IndexError + return k + 1 + + def __len__(self): + return 10 + +for i in C(): + print(i) + + +def test(*args): + print(args) + + +test(*A()) # [not-an-iterable] +test(*B()) +test(*B) # [not-an-iterable] +for i in A(): # [not-an-iterable] + pass +for i in B(): + pass +for i in B: # [not-an-iterable] + pass + +for i in range: # [not-an-iterable] + pass + +# check that primitive non-iterable types are catched +for i in True: # [not-an-iterable] + pass + +for i in None: # [not-an-iterable] + pass + +for i in 8.5: # [not-an-iterable] + pass + +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.txt b/pylint/test/functional/iterable_context.txt new file mode 100644 index 0000000..fbe1433 --- /dev/null +++ b/pylint/test/functional/iterable_context.txt @@ -0,0 +1,10 @@ +not-an-iterable:58::Non-iterable value powers_of_two is used in an iterating context +not-an-iterable:93::Non-iterable value A() is used in an iterating context +not-an-iterable:95::Non-iterable value B is used in an iterating context +not-an-iterable:96::Non-iterable value A() is used in an iterating context +not-an-iterable:100::Non-iterable value B is used in an iterating context +not-an-iterable:103::Non-iterable value range is used in an iterating context +not-an-iterable:107::Non-iterable value True is used in an iterating context +not-an-iterable:110::Non-iterable value None is used in an iterating context +not-an-iterable:113::Non-iterable value 8.5 is used in an iterating context +not-an-iterable:116::Non-iterable value 10 is used in an iterating context 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 new file mode 100644 index 0000000..cfab8dc --- /dev/null +++ b/pylint/test/functional/mapping_context.py @@ -0,0 +1,59 @@ +""" +Checks that only valid values are used in a mapping context. +""" +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-self-use,import-error +from __future__ import print_function + + +def test(**kwargs): + print(kwargs) + + +# dictionary value/comprehension +dict_value = dict(a=1, b=2, c=3) +dict_comp = {chr(x): x for x in range(256)} +test(**dict_value) +test(**dict_comp) + + +# in order to be used in kwargs custom mapping class should define +# __iter__(), __getitem__(key) and keys(). +class CustomMapping(object): + def __init__(self): + self.data = dict(a=1, b=2, c=3, d=4, e=5) + + def __getitem__(self, key): + return self.data[key] + + def keys(self): + 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 new file mode 100644 index 0000000..201da1a --- /dev/null +++ b/pylint/test/functional/mapping_context.txt @@ -0,0 +1,2 @@ +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/functional/yield_from_iterable_py33.py b/pylint/test/functional/yield_from_iterable_py33.py new file mode 100644 index 0000000..7803936 --- /dev/null +++ b/pylint/test/functional/yield_from_iterable_py33.py @@ -0,0 +1,7 @@ +""" +Check that `yield from`-statement takes an iterable. +""" +# pylint: disable=missing-docstring + +def to_ten(): + yield from 10 # [not-an-iterable] diff --git a/pylint/test/functional/yield_from_iterable_py33.rc b/pylint/test/functional/yield_from_iterable_py33.rc new file mode 100644 index 0000000..3330edd --- /dev/null +++ b/pylint/test/functional/yield_from_iterable_py33.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.3
\ No newline at end of file diff --git a/pylint/test/functional/yield_from_iterable_py33.txt b/pylint/test/functional/yield_from_iterable_py33.txt new file mode 100644 index 0000000..906ee93 --- /dev/null +++ b/pylint/test/functional/yield_from_iterable_py33.txt @@ -0,0 +1 @@ +not-an-iterable:7:to_ten:Non-iterable value 10 is used in an iterating context
diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py index c68f379..2094314 100644 --- a/pylint/test/unittest_checker_base.py +++ b/pylint/test/unittest_checker_base.py @@ -1,7 +1,6 @@ """Unittest for the base checker.""" import re -import sys import unittest import astroid diff --git a/pylint/test/unittest_checker_typecheck.py b/pylint/test/unittest_checker_typecheck.py index b7135ea..0aaa8a5 100644 --- a/pylint/test/unittest_checker_typecheck.py +++ b/pylint/test/unittest_checker_typecheck.py @@ -5,6 +5,7 @@ from astroid import test_utils from pylint.checkers import typecheck from pylint.testutils import CheckerTestCase, Message, set_config + class TypeCheckerTest(CheckerTestCase): "Tests for pylint.checkers.typecheck" CHECKER_CLASS = typecheck.TypeChecker |