diff options
author | Claudiu Popa <pcmanticore@gmail.com> | 2015-10-30 14:24:23 +0200 |
---|---|---|
committer | Claudiu Popa <pcmanticore@gmail.com> | 2015-10-30 14:24:23 +0200 |
commit | d00bce5417db3baf7adbef12bcbbb909d2d4c372 (patch) | |
tree | 3a9e519e7e6a9931f8eed542c79fec2ce8616229 | |
parent | a32219e4e1559845fbc4bf831a4efce3a2d2195c (diff) | |
parent | 70ad9f916e3534288573a5bbf1741a4abb73b238 (diff) | |
download | pylint-d00bce5417db3baf7adbef12bcbbb909d2d4c372.tar.gz |
Merged in dmand/pylint/enhance-unpacking-checker (pull request #297)
Enhance tuple unpacking checker
-rw-r--r-- | pylint/checkers/typecheck.py | 170 | ||||
-rw-r--r-- | pylint/checkers/utils.py | 103 | ||||
-rw-r--r-- | pylint/checkers/variables.py | 20 | ||||
-rw-r--r-- | pylint/test/functional/unpacking_non_sequence.py | 25 | ||||
-rw-r--r-- | pylint/test/functional/unpacking_non_sequence.txt | 24 |
5 files changed, 187 insertions, 155 deletions
diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 7baaaf6..046c3b9 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -35,7 +35,9 @@ from pylint.interfaces import IAstroidChecker, INFERENCE, INFERENCE_FAILURE from pylint.checkers import BaseChecker from pylint.checkers.utils import ( is_super, check_messages, decorated_with_property, - decorated_with, node_ignores_exception, class_is_abstract) + decorated_with, node_ignores_exception, class_is_abstract, + is_iterable, is_mapping, supports_membership_test, + is_comprehension, is_inside_abstract_class) from pylint import utils @@ -44,11 +46,6 @@ _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__' -CONTAINS_METHOD = '__contains__' -KEYS_METHOD = 'keys' def _unflatten(iterable): @@ -89,102 +86,6 @@ 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, - astroid.GeneratorExp) - return isinstance(node, comprehensions) - -def _supports_mapping_protocol(value): - return _hasattr(value, GETITEM_METHOD) and _hasattr(value, KEYS_METHOD) - -def _supports_membership_test_protocol(value): - return _hasattr(value, CONTAINS_METHOD) - -def _supports_iteration_protocol(value): - return _hasattr(value, ITER_METHOD) or _hasattr(value, GETITEM_METHOD) - -def _is_abstract_class_name(name): - lname = name.lower() - is_mixin = lname.endswith('mixin') - is_abstract = lname.startswith('abstract') - is_base = lname.startswith('base') or lname.endswith('base') - return is_mixin or is_abstract or is_base - -def _is_inside_abstract_class(node): - while node is not None: - if isinstance(node, astroid.ClassDef): - if class_is_abstract(node): - return True - name = getattr(node, 'name', None) - if name is not None and _is_abstract_class_name(name): - return True - node = node.parent - return False - -def _should_skip_iterable_check(node): - if _is_inside_abstract_class(node): - return True - infered = helpers.safe_infer(node) - if infered is None or infered is astroid.YES: - return True - if isinstance(infered, (astroid.ClassDef, astroid.Instance)): - if not helpers.has_known_bases(infered): - return True - return False - -def _is_iterable(node): - # for/set/dict-comprehensions can't be infered with astroid - # so we have to check for them explicitly - if _is_comprehension(node): - return True - infered = helpers.safe_infer(node) - if isinstance(infered, astroid.ClassDef): - # classobj can only be iterable if it has an iterable metaclass - meta = infered.metaclass() - if meta is not None: - if _supports_iteration_protocol(meta): - return True - if isinstance(infered, astroid.Instance): - if _supports_iteration_protocol(infered): - return True - return False - -def _is_mapping(node): - if isinstance(node, astroid.DictComp): - return True - infered = helpers.safe_infer(node) - if isinstance(infered, astroid.ClassDef): - # classobj can only be iterable if it has an iterable metaclass - meta = infered.metaclass() - if meta is not None: - if _supports_mapping_protocol(meta): - return True - if isinstance(infered, astroid.Instance): - if _supports_mapping_protocol(infered): - return True - return False - -def _supports_membership_test(node): - infered = helpers.safe_infer(node) - if isinstance(infered, astroid.ClassDef): - meta = infered.metaclass() - if meta is not None and _supports_membership_test_protocol(meta): - return True - if isinstance(infered, astroid.Instance): - if _supports_membership_test_protocol(infered): - return True - return _is_iterable(node) - - MSGS = { 'E1101': ('%s %r has no %r member', 'no-member', @@ -896,16 +797,17 @@ accessed. Python regular expressions are accepted.'} args=str(error), node=node) def _check_membership_test(self, node): - # value supports membership test in either of those cases: - # 1. value defines __contains__ method - # 2. value is iterable (defines __iter__ or __getitem__) - if _should_skip_iterable_check(node): + if is_inside_abstract_class(node): return - if _supports_membership_test(node): + if is_comprehension(node): return - self.add_message('unsupported-membership-test', - args=node.as_string(), - node=node) + infered = helpers.safe_infer(node) + if infered is None or infered is astroid.YES: + return + if not supports_membership_test(infered): + self.add_message('unsupported-membership-test', + args=node.as_string(), + node=node) @check_messages('unsupported-membership-test') def visit_compare(self, node): @@ -941,58 +843,66 @@ class IterableChecker(BaseChecker): 'mapping is expected'), } - def _check_iterable(self, node, root_node): - if _should_skip_iterable_check(node): + def _check_iterable(self, node): + if is_inside_abstract_class(node): + return + if is_comprehension(node): return - if _is_iterable(node): + infered = helpers.safe_infer(node) + if infered is None or infered is astroid.YES: return - self.add_message('not-an-iterable', - args=node.as_string(), - node=root_node) + if not is_iterable(infered): + self.add_message('not-an-iterable', + args=node.as_string(), + node=node) - def _check_mapping(self, node, root_node): - if _should_skip_iterable_check(node): + def _check_mapping(self, node): + if is_inside_abstract_class(node): + return + if isinstance(node, astroid.DictComp): return - if _is_mapping(node): + infered = helpers.safe_infer(node) + if infered is None or infered is astroid.YES: return - self.add_message('not-a-mapping', - args=node.as_string(), - node=root_node) + if not is_mapping(infered): + self.add_message('not-a-mapping', + args=node.as_string(), + node=node) @check_messages('not-an-iterable') def visit_for(self, node): - self._check_iterable(node.iter, node) + self._check_iterable(node.iter) @check_messages('not-an-iterable') def visit_yieldfrom(self, node): - self._check_iterable(node.value, node) + self._check_iterable(node.value) @check_messages('not-an-iterable', 'not-a-mapping') def visit_call(self, node): for stararg in node.starargs: - self._check_iterable(stararg.value, node) + self._check_iterable(stararg.value) for kwarg in node.kwargs: - self._check_mapping(kwarg.value, node) + self._check_mapping(kwarg.value) @check_messages('not-an-iterable') def visit_listcomp(self, node): for gen in node.generators: - self._check_iterable(gen.iter, node) + self._check_iterable(gen.iter) @check_messages('not-an-iterable') def visit_dictcomp(self, node): for gen in node.generators: - self._check_iterable(gen.iter, node) + self._check_iterable(gen.iter) @check_messages('not-an-iterable') def visit_setcomp(self, node): for gen in node.generators: - self._check_iterable(gen.iter, node) + self._check_iterable(gen.iter) @check_messages('not-an-iterable') def visit_generatorexp(self, node): for gen in node.generators: - self._check_iterable(gen.iter, node) + self._check_iterable(gen.iter) def register(linter): diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 8873d0e..5f9c227 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -26,6 +26,7 @@ import warnings import astroid from astroid import helpers from astroid import scoped_nodes +import six from six.moves import map, builtins # pylint: disable=redefined-builtin BUILTINS_NAME = builtins.__name__ @@ -39,6 +40,11 @@ else: EXCEPTIONS_MODULE = "builtins" ABC_METHODS = set(('abc.abstractproperty', 'abc.abstractmethod', 'abc.abstractclassmethod', 'abc.abstractstaticmethod')) +ITER_METHOD = '__iter__' +NEXT_METHOD = 'next' if six.PY2 else '__next__' +GETITEM_METHOD = '__getitem__' +CONTAINS_METHOD = '__contains__' +KEYS_METHOD = 'keys' # Dictionary which maps the number of expected parameters a # special method can have to a set of special methods. @@ -584,6 +590,103 @@ def class_is_abstract(node): return False +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, + astroid.GeneratorExp) + return isinstance(node, comprehensions) + + +def _supports_mapping_protocol(value): + return _hasattr(value, GETITEM_METHOD) and _hasattr(value, KEYS_METHOD) + + +def _supports_membership_test_protocol(value): + return _hasattr(value, CONTAINS_METHOD) + + +def _supports_iteration_protocol(value): + return _hasattr(value, ITER_METHOD) or _hasattr(value, GETITEM_METHOD) + + +def _is_abstract_class_name(name): + lname = name.lower() + is_mixin = lname.endswith('mixin') + is_abstract = lname.startswith('abstract') + is_base = lname.startswith('base') or lname.endswith('base') + return is_mixin or is_abstract or is_base + + +def is_inside_abstract_class(node): + while node is not None: + if isinstance(node, astroid.ClassDef): + if class_is_abstract(node): + return True + name = getattr(node, 'name', None) + if name is not None and _is_abstract_class_name(name): + return True + node = node.parent + return False + + +def is_iterable(value): + if isinstance(value, astroid.ClassDef): + if not helpers.has_known_bases(value): + return True + # classobj can only be iterable if it has an iterable metaclass + meta = value.metaclass() + if meta is not None: + if _supports_iteration_protocol(meta): + return True + if isinstance(value, astroid.Instance): + if not helpers.has_known_bases(value): + return True + if _supports_iteration_protocol(value): + return True + return False + + +def is_mapping(value): + if isinstance(value, astroid.ClassDef): + if not helpers.has_known_bases(value): + return True + # classobj can only be a mapping if it has a metaclass is mapping + meta = value.metaclass() + if meta is not None: + if _supports_mapping_protocol(meta): + return True + if isinstance(value, astroid.Instance): + if not helpers.has_known_bases(value): + return True + if _supports_mapping_protocol(value): + return True + return False + + +def supports_membership_test(value): + if isinstance(value, astroid.ClassDef): + if not helpers.has_known_bases(value): + return True + meta = value.metaclass() + if meta is not None and _supports_membership_test_protocol(meta): + return True + if isinstance(value, astroid.Instance): + if not helpers.has_known_bases(value): + return True + if _supports_membership_test_protocol(value): + return True + return is_iterable(value) + + # TODO(cpopa): deprecate these or leave them as aliases? safe_infer = astroid.helpers.safe_infer has_known_bases = astroid.helpers.has_known_bases diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 79f6d05..b30aaa6 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -30,7 +30,8 @@ from pylint.checkers.utils import ( PYMETHODS, is_ancestor_name, is_builtin, is_defined_before, is_error, is_func_default, is_func_decorator, assign_parent, check_messages, is_inside_except, clobber_in_except, - get_all_elements, has_known_bases, node_ignores_exception) + get_all_elements, has_known_bases, node_ignores_exception, + is_inside_abstract_class, is_comprehension, is_iterable) import six SPECIAL_OBJ = re.compile("^_{2}[a-z]+_{2}$") @@ -1039,6 +1040,10 @@ builtins. Remember that you should avoid to define new builtins when possible.' """ Check for unbalanced tuple unpacking and unpacking non sequences. """ + if is_inside_abstract_class(node): + return + if is_comprehension(node): + return if infered is astroid.YES: return if (isinstance(infered.parent, astroid.Arguments) and @@ -1059,19 +1064,10 @@ builtins. Remember that you should avoid to define new builtins when possible.' len(targets), len(values))) # attempt to check unpacking may be possible (ie RHS is iterable) - elif isinstance(infered, astroid.Instance): - for meth in ('__iter__', '__getitem__'): - try: - infered.getattr(meth) - break - except astroid.NotFoundError: - continue - else: + else: + if not is_iterable(infered): self.add_message('unpacking-non-sequence', node=node, args=(_get_unpacking_extra_info(node, infered),)) - else: - self.add_message('unpacking-non-sequence', node=node, - args=(_get_unpacking_extra_info(node, infered),)) def _check_module_attrs(self, node, module, module_names): diff --git a/pylint/test/functional/unpacking_non_sequence.py b/pylint/test/functional/unpacking_non_sequence.py index 93b1bbd..f449d5b 100644 --- a/pylint/test/functional/unpacking_non_sequence.py +++ b/pylint/test/functional/unpacking_non_sequence.py @@ -1,9 +1,10 @@ """Check unpacking non-sequences in assignments. """ # pylint: disable=too-few-public-methods, invalid-name, attribute-defined-outside-init, unused-variable, no-absolute-import -# pylint: disable=using-constant-test +# pylint: disable=using-constant-test, no-init from os import rename as nonseq_func from functional.unpacking import nonseq +from six import with_metaclass __revision__ = 0 @@ -37,6 +38,27 @@ def good_unpacking2(): """ returns should be unpackable """ return good_unpacking() +class MetaIter(type): + "metaclass that makes classes that use it iterables" + def __iter__(cls): + return iter((1, 2)) + +class IterClass(with_metaclass(MetaIter)): + "class that is iterable (and unpackable)" + +class AbstrClass(object): + "abstract class" + pair = None + + def setup_pair(self): + "abstract method" + raise NotImplementedError + + def __init__(self): + "error should not be emitted because setup_pair is abstract" + self.setup_pair() + x, y = self.pair + a, b = [1, 2] a, b = (1, 2) a, b = set([1, 2]) @@ -47,6 +69,7 @@ a, b = Iter() a, b = (number for number in range(2)) a, b = good_unpacking() a, b = good_unpacking2() +a, b = IterClass # Not working class NonSeq(object): diff --git a/pylint/test/functional/unpacking_non_sequence.txt b/pylint/test/functional/unpacking_non_sequence.txt index af58686..8d13c44 100644 --- a/pylint/test/functional/unpacking_non_sequence.txt +++ b/pylint/test/functional/unpacking_non_sequence.txt @@ -1,12 +1,12 @@ -unpacking-non-sequence:61::Attempting to unpack a non-sequence defined at line 52 -unpacking-non-sequence:62::Attempting to unpack a non-sequence -unpacking-non-sequence:63::Attempting to unpack a non-sequence None -unpacking-non-sequence:64::Attempting to unpack a non-sequence 1 -unpacking-non-sequence:65::Attempting to unpack a non-sequence defined at line 9 of functional.unpacking -unpacking-non-sequence:66::Attempting to unpack a non-sequence defined at line 11 of functional.unpacking -unpacking-non-sequence:67::Attempting to unpack a non-sequence defined at line 58 -unpacking-non-sequence:68::Attempting to unpack a non-sequence -unpacking-non-sequence:83:ClassUnpacking.test:Attempting to unpack a non-sequence defined at line 52 -unpacking-non-sequence:84:ClassUnpacking.test:Attempting to unpack a non-sequence -unpacking-non-sequence:85:ClassUnpacking.test:Attempting to unpack a non-sequence defined at line 58 -unpacking-non-sequence:86:ClassUnpacking.test:Attempting to unpack a non-sequence +unpacking-non-sequence:84::Attempting to unpack a non-sequence defined at line 75 +unpacking-non-sequence:85::Attempting to unpack a non-sequence +unpacking-non-sequence:86::Attempting to unpack a non-sequence None +unpacking-non-sequence:87::Attempting to unpack a non-sequence 1 +unpacking-non-sequence:88::Attempting to unpack a non-sequence defined at line 9 of functional.unpacking +unpacking-non-sequence:89::Attempting to unpack a non-sequence defined at line 11 of functional.unpacking +unpacking-non-sequence:90::Attempting to unpack a non-sequence defined at line 81 +unpacking-non-sequence:91::Attempting to unpack a non-sequence +unpacking-non-sequence:106:ClassUnpacking.test:Attempting to unpack a non-sequence defined at line 75 +unpacking-non-sequence:107:ClassUnpacking.test:Attempting to unpack a non-sequence +unpacking-non-sequence:108:ClassUnpacking.test:Attempting to unpack a non-sequence defined at line 81 +unpacking-non-sequence:109:ClassUnpacking.test:Attempting to unpack a non-sequence |