From aba38cd9c0cd8be923a7aa8e0d1712c53d3e1e6c Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Mon, 5 Oct 2015 17:15:14 +0300 Subject: Add initial version of checker for iterables/mappings. It checks for the following things: - for-statement should contain an iterable value - `yield from`-statement should contain an iterable value - function call with star args should contain iterable value (e.g. in `foo(*bar)` bar should be an iterable) - function call with kwargs should contain a mapping (e.g. in `foo(**bar)` bar should be a dict) Idea came from issue #563. --- pylint/checkers/base.py | 58 +++++++++++++++++++ pylint/test/unittest_checker_base.py | 107 +++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+) diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index 30e337b..84f5a8d 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -1476,6 +1476,64 @@ class ComparisonChecker(_BasicChecker): self.check_singleton_comparison(right, node) +class IterableChecker(_BasicChecker): + """ + TODO: + """ + msgs = {'E0121': ('Non-iterable value %s is used in an iterating context', + 'not-an-iterable', + 'YOLO'), + 'E0122': ('Non-mapping value %s is used in an mapping context', + 'not-a-mapping', + 'YOLO'), + } + + def _is_string(self, node): + return isinstance(node, astroid.Const) and \ + isinstance(node.value, six.string_types) + + def _is_iterable(self, value): + is_iterable_value = isinstance(value, (astroid.List, astroid.Tuple, + astroid.Set, astroid.Dict, + astroid.bases.Generator)) + is_string = self._is_string(value) + return is_iterable_value or is_string + + def _is_mapping(self, value): + return isinstance(value, astroid.Dict) + + def _check_iterable(self, element, root_node): + infered = helpers.safe_infer(element) + if infered is not None: + if not self._is_iterable(infered): + self.add_message('not-an-iterable', + args=infered.as_string(), + node=root_node) + + def _check_mapping(self, element, root_node): + infered = helpers.safe_infer(element) + if infered is not None: + if not self._is_mapping(infered): + self.add_message('not-a-mapping', + args=infered.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) + + def register(linter): """required method to auto register this checker""" linter.register_checker(BasicErrorChecker(linter)) diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py index c68f379..fb595e3 100644 --- a/pylint/test/unittest_checker_base.py +++ b/pylint/test/unittest_checker_base.py @@ -10,6 +10,13 @@ from pylint.checkers import base 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 DocstringTest(CheckerTestCase): CHECKER_CLASS = base.DocStringChecker @@ -293,5 +300,105 @@ class ComparisonTest(CheckerTestCase): self.checker.visit_compare(node) +class IterableTest(CheckerTestCase): + CHECKER_CLASS = base.IterableChecker + + def test_non_iterable_in_for(self): + node = test_utils.extract_node(""" + for i in 42: + print(i) + """) + message = Message('not-an-iterable', node=node, args='42') + with self.assertAddsMessages(message): + self.checker.visit_for(node) + + node = test_utils.extract_node(""" + for i in [1,2,3]: + print(i) + """) + with self.assertNoMessages(): + self.checker.visit_for(node) + + node = test_utils.extract_node(""" + def count(): + i = 0 + while True: + yield i + i += 1 + + for i in count(): + print(i) + """) + with self.assertNoMessages(): + self.walk(node.root()) + + node = test_utils.extract_node(""" + for i in "aeiou": + print(i) + """) + with self.assertNoMessages(): + self.checker.visit_for(node) + + @python33_and_newer + def test_non_iterable_in_yield_from(self): + node = test_utils.extract_node(""" + yield from 42 + """) + message = Message('not-an-iterable', node=node, args='42') + with self.assertAddsMessages(message): + self.checker.visit_yieldfrom(node) + + node = test_utils.extract_node(""" + yield from [1,2,3] + """) + with self.assertNoMessages(): + self.checker.visit_yieldfrom(node) + + def test_non_iterable_in_starargs(self): + node = test_utils.extract_node(""" + foo(*123) + """) + message = Message('not-an-iterable', node=node, args='123') + with self.assertAddsMessages(message): + self.checker.visit_call(node) + + node = test_utils.extract_node(""" + stuff = [1,2,3] + foo(*stuff) + """) + with self.assertNoMessages(): + self.walk(node.root()) + + def test_non_mapping_in_kwargs(self): + node = test_utils.extract_node(""" + foo(**123) + """) + message = Message('not-a-mapping', node=node, args='123') + with self.assertAddsMessages(message): + self.checker.visit_call(node) + + node = test_utils.extract_node(""" + args = [1, 2, 3] + foo(**args) + """) + message = Message('not-a-mapping', node=node, args='[1, 2, 3]') + with self.assertAddsMessages(message): + self.walk(node.root()) + + node = test_utils.extract_node(""" + kwargs = {'answer': 42} + foo(**kwargs) + """) + with self.assertNoMessages(): + self.walk(node.root()) + + node = test_utils.extract_node(""" + foo(**dict(a=1, b=2)) + """) + with self.assertNoMessages(): + self.checker.visit_call(node) + + + if __name__ == '__main__': unittest.main() -- cgit v1.2.1 From c47b60020a7e592373deaba7d1fe6f78592c534b Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Mon, 5 Oct 2015 20:04:01 +0300 Subject: Add iterable checking for all types of comprehensions. Specifically, list, dict and set comprehensions along with generator expressions. --- pylint/checkers/base.py | 21 +++++++++++++++++++++ pylint/test/unittest_checker_base.py | 15 +++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index 84f5a8d..7f6f44c 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -1533,6 +1533,27 @@ class IterableChecker(_BasicChecker): 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): + import code; code.interact(local=locals()) + for gen in node.generators: + self._check_iterable(gen.iter, node) + def register(linter): """required method to auto register this checker""" diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py index fb595e3..b58d8d9 100644 --- a/pylint/test/unittest_checker_base.py +++ b/pylint/test/unittest_checker_base.py @@ -398,6 +398,21 @@ class IterableTest(CheckerTestCase): with self.assertNoMessages(): self.checker.visit_call(node) + def test_non_iterable_in_listcomp(self): + node = test_utils.extract_node(""" + [x for x in 123] + """) + message = Message('not-an-iterable', node=node, args='123') + with self.assertAddsMessages(message): + self.checker.visit_listcomp(node) + # TODO: more tests + + def test_non_iterable_in_generator(self): + node = test_utils.extract_node("__(x for x in 123)") + message = Message('not-an-iterable', node=node, args='123') + with self.assertAddsMessages(message): + self.walk(node.root()) + # TODO: more tests if __name__ == '__main__': -- cgit v1.2.1 From 7666e2d0f386b8f4758dd8b221b9380edaf8fe8f Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Tue, 6 Oct 2015 12:45:34 +0300 Subject: Improve iterable inference and add more unit tests for iterable checker --- pylint/checkers/base.py | 19 +++++++++---- pylint/test/unittest_checker_base.py | 54 ++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index 7f6f44c..904cf9f 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -1499,15 +1499,25 @@ class IterableChecker(_BasicChecker): is_string = self._is_string(value) return is_iterable_value or is_string + def _not_an_iterable(self, value): + is_iterable = self._is_iterable(value) + is_const = isinstance(value, astroid.Const) + return not is_iterable and is_const + def _is_mapping(self, value): - return isinstance(value, astroid.Dict) + return isinstance(value, (astroid.Dict, astroid.DictComp)) + + def _not_a_mapping(self, value): + is_mapping = self._is_mapping(value) + is_const = isinstance(value, astroid.Const) + return not is_mapping and is_const def _check_iterable(self, element, root_node): infered = helpers.safe_infer(element) if infered is not None: - if not self._is_iterable(infered): + if self._not_an_iterable(infered): self.add_message('not-an-iterable', - args=infered.as_string(), + args=element.as_string(), node=root_node) def _check_mapping(self, element, root_node): @@ -1515,7 +1525,7 @@ class IterableChecker(_BasicChecker): if infered is not None: if not self._is_mapping(infered): self.add_message('not-a-mapping', - args=infered.as_string(), + args=element.as_string(), node=root_node) @check_messages('not-an-iterable') @@ -1550,7 +1560,6 @@ class IterableChecker(_BasicChecker): @check_messages('not-an-iterable') def visit_generatorexp(self, node): - import code; code.interact(local=locals()) for gen in node.generators: self._check_iterable(gen.iter, node) diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py index b58d8d9..538cf94 100644 --- a/pylint/test/unittest_checker_base.py +++ b/pylint/test/unittest_checker_base.py @@ -369,7 +369,7 @@ class IterableTest(CheckerTestCase): with self.assertNoMessages(): self.walk(node.root()) - def test_non_mapping_in_kwargs(self): + def test_non_mapping_in_funcall_kwargs(self): node = test_utils.extract_node(""" foo(**123) """) @@ -381,7 +381,7 @@ class IterableTest(CheckerTestCase): args = [1, 2, 3] foo(**args) """) - message = Message('not-a-mapping', node=node, args='[1, 2, 3]') + message = Message('not-a-mapping', node=node, args='args') with self.assertAddsMessages(message): self.walk(node.root()) @@ -398,21 +398,63 @@ class IterableTest(CheckerTestCase): with self.assertNoMessages(): self.checker.visit_call(node) + node = test_utils.extract_node(""" + foo(**{str(x): x for x in range(5)}) + """) + with self.assertNoMessages(): + self.checker.visit_call(node) + def test_non_iterable_in_listcomp(self): node = test_utils.extract_node(""" - [x for x in 123] + [x ** 2 for x in 10] """) - message = Message('not-an-iterable', node=node, args='123') + message = Message('not-an-iterable', node=node, args='10') with self.assertAddsMessages(message): self.checker.visit_listcomp(node) - # TODO: more tests + + node = test_utils.extract_node(""" + [x ** 2 for x in range(10)] + """) + with self.assertNoMessages(): + self.checker.visit_listcomp(node) + + def test_non_iterable_in_dictcomp(self): + node = test_utils.extract_node(""" + {x: chr(x) for x in 256} + """) + message = Message('not-an-iterable', node=node, args='256') + with self.assertAddsMessages(message): + self.checker.visit_dictcomp(node) + + node = test_utils.extract_node(""" + {ord(x): x for x in "aoeui"} + """) + with self.assertNoMessages(): + self.checker.visit_dictcomp(node) + + def test_non_iterable_in_setcomp(self): + node = test_utils.extract_node(""" + {2 ** x for x in 10} + """) + message = Message('not-an-iterable', node=node, args='10') + with self.assertAddsMessages(message): + self.checker.visit_setcomp(node) + + node = test_utils.extract_node(""" + {2 ** x for x in range(10)} + """) + with self.assertNoMessages(): + self.checker.visit_setcomp(node) def test_non_iterable_in_generator(self): node = test_utils.extract_node("__(x for x in 123)") message = Message('not-an-iterable', node=node, args='123') with self.assertAddsMessages(message): self.walk(node.root()) - # TODO: more tests + + node = test_utils.extract_node("__(chr(x) for x in range(25))") + with self.assertNoMessages(): + self.walk(node.root()) if __name__ == '__main__': -- cgit v1.2.1 From aa68ce1b13aed050373c1953e6b92987633d4777 Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Wed, 7 Oct 2015 02:39:08 +0300 Subject: Alter inference strategy in iterable/mapping checker --- pylint/checkers/base.py | 49 +++++++++++++++--------------------- pylint/test/unittest_checker_base.py | 25 ++---------------- 2 files changed, 22 insertions(+), 52 deletions(-) diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index 904cf9f..b68b082 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -1478,39 +1478,29 @@ class ComparisonChecker(_BasicChecker): class IterableChecker(_BasicChecker): """ - TODO: + 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. """ - msgs = {'E0121': ('Non-iterable value %s is used in an iterating context', + msgs = {'E0118': ('Non-iterable value %s is used in an iterating context', 'not-an-iterable', - 'YOLO'), - 'E0122': ('Non-mapping value %s is used in an mapping context', + 'Used when a non-iterable value is used in place where' + 'iterable is expected'), + 'E0119': ('Non-mapping value %s is used in a mapping context', 'not-a-mapping', - 'YOLO'), + 'Used when a non-mapping value is used in place where' + 'mapping is expected'), } - def _is_string(self, node): - return isinstance(node, astroid.Const) and \ - isinstance(node.value, six.string_types) - - def _is_iterable(self, value): - is_iterable_value = isinstance(value, (astroid.List, astroid.Tuple, - astroid.Set, astroid.Dict, - astroid.bases.Generator)) - is_string = self._is_string(value) - return is_iterable_value or is_string - - def _not_an_iterable(self, value): - is_iterable = self._is_iterable(value) - is_const = isinstance(value, astroid.Const) - return not is_iterable and is_const - - def _is_mapping(self, value): - return isinstance(value, (astroid.Dict, astroid.DictComp)) - - def _not_a_mapping(self, value): - is_mapping = self._is_mapping(value) - is_const = isinstance(value, astroid.Const) - return not is_mapping and is_const + def _not_an_iterable(self, node): + is_const = isinstance(node, astroid.Const) + is_string = is_const and isinstance(node.value, six.string_types) + return is_const and not is_string def _check_iterable(self, element, root_node): infered = helpers.safe_infer(element) @@ -1523,7 +1513,7 @@ class IterableChecker(_BasicChecker): def _check_mapping(self, element, root_node): infered = helpers.safe_infer(element) if infered is not None: - if not self._is_mapping(infered): + if isinstance(infered, astroid.Const): self.add_message('not-a-mapping', args=element.as_string(), node=root_node) @@ -1573,3 +1563,4 @@ def register(linter): linter.register_checker(PassChecker(linter)) linter.register_checker(LambdaForComprehensionChecker(linter)) linter.register_checker(ComparisonChecker(linter)) + linter.register_checker(IterableChecker(linter)) diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py index 538cf94..0b1cddd 100644 --- a/pylint/test/unittest_checker_base.py +++ b/pylint/test/unittest_checker_base.py @@ -354,7 +354,7 @@ class IterableTest(CheckerTestCase): with self.assertNoMessages(): self.checker.visit_yieldfrom(node) - def test_non_iterable_in_starargs(self): + def test_non_iterable_in_funcall_starargs(self): node = test_utils.extract_node(""" foo(*123) """) @@ -378,32 +378,11 @@ class IterableTest(CheckerTestCase): self.checker.visit_call(node) node = test_utils.extract_node(""" - args = [1, 2, 3] - foo(**args) - """) - message = Message('not-a-mapping', node=node, args='args') - with self.assertAddsMessages(message): - self.walk(node.root()) - - node = test_utils.extract_node(""" - kwargs = {'answer': 42} - foo(**kwargs) + foo(**retdict()) """) with self.assertNoMessages(): self.walk(node.root()) - node = test_utils.extract_node(""" - foo(**dict(a=1, b=2)) - """) - with self.assertNoMessages(): - self.checker.visit_call(node) - - node = test_utils.extract_node(""" - foo(**{str(x): x for x in range(5)}) - """) - with self.assertNoMessages(): - self.checker.visit_call(node) - def test_non_iterable_in_listcomp(self): node = test_utils.extract_node(""" [x ** 2 for x in 10] -- cgit v1.2.1 From 335e147d7d47d61740a40641a62014b5aed4b59b Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Wed, 7 Oct 2015 02:39:38 +0300 Subject: Add functional tests for iterable hecker --- pylint/test/functional/iterable_context.py | 36 ++++++++++++++++++++++ pylint/test/functional/iterable_context.txt | 10 ++++++ pylint/test/functional/yield_from_iterable_py33.py | 7 +++++ pylint/test/functional/yield_from_iterable_py33.rc | 2 ++ .../test/functional/yield_from_iterable_py33.txt | 1 + 5 files changed, 56 insertions(+) create mode 100644 pylint/test/functional/iterable_context.py create mode 100644 pylint/test/functional/iterable_context.txt create mode 100644 pylint/test/functional/yield_from_iterable_py33.py create mode 100644 pylint/test/functional/yield_from_iterable_py33.rc create mode 100644 pylint/test/functional/yield_from_iterable_py33.txt diff --git a/pylint/test/functional/iterable_context.py b/pylint/test/functional/iterable_context.py new file mode 100644 index 0000000..8282683 --- /dev/null +++ b/pylint/test/functional/iterable_context.py @@ -0,0 +1,36 @@ +""" +Checks that primitive values are not used in an +iterating/mapping context. +""" +# pylint: disable=missing-docstring,invalid-name +from __future__ import print_function + +# for-statement +for i in 42: # [not-an-iterable] + pass + +for i in True: # [not-an-iterable] + pass + +# funcall-starargs +def test(*args, **kwargs): + print(args, kwargs) + +test(*1) # [not-an-iterable] +test(*False) # [not-an-iterable] + +# funcall-kwargs +test(**1) # [not-a-mapping] +test(**None) # [not-a-mapping] + +# list-comprehension +test([3 ** x for x in 10]) # [not-an-iterable] + +# dict-comprehension +test({k: chr(k) for k in 128}) # [not-an-iterable] + +# set-comprehension +test({x for x in 32}) # [not-an-iterable] + +# generator-expression +test(str(x) for x in 10) # [not-an-iterable] diff --git a/pylint/test/functional/iterable_context.txt b/pylint/test/functional/iterable_context.txt new file mode 100644 index 0000000..8c9f098 --- /dev/null +++ b/pylint/test/functional/iterable_context.txt @@ -0,0 +1,10 @@ +not-an-iterable:9::Non-iterable value 42 is used in an iterating context +not-an-iterable:12::Non-iterable value True is used in an iterating context +not-an-iterable:19::Non-iterable value 1 is used in an iterating context +not-an-iterable:20::Non-iterable value False is used in an iterating context +not-a-mapping:23::Non-mapping value 1 is used in a mapping context +not-a-mapping:24::Non-mapping value None is used in a mapping context +not-an-iterable:27::Non-iterable value 10 is used in an iterating context +not-an-iterable:30::Non-iterable value 128 is used in an iterating context +not-an-iterable:33::Non-iterable value 32 is used in an iterating context +not-an-iterable:36::Non-iterable value 10 is used in an iterating 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 -- cgit v1.2.1 From 5c1cbb60dc9612663bcc4bd57e84192643154f76 Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Wed, 7 Oct 2015 02:44:53 +0300 Subject: Update changelog --- ChangeLog | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index e1e71c0..9eb0c9b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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. + * Add checker to identify multiple imports on one line. Closes issue #598. -- cgit v1.2.1 From 573d4dd7b0ef6aa492713ec39711cc87195d638a Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Thu, 8 Oct 2015 00:35:55 +0300 Subject: Move iterable checker to typecheck module and alter inference strategy --- pylint/checkers/base.py | 79 ---------------- pylint/checkers/typecheck.py | 120 +++++++++++++++++++++++++ pylint/test/unittest_checker_base.py | 144 ------------------------------ pylint/test/unittest_checker_typecheck.py | 144 ++++++++++++++++++++++++++++++ 4 files changed, 264 insertions(+), 223 deletions(-) diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index b68b082..30e337b 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -1476,84 +1476,6 @@ class ComparisonChecker(_BasicChecker): self.check_singleton_comparison(right, node) -class IterableChecker(_BasicChecker): - """ - 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. - """ - msgs = {'E0118': ('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'), - 'E0119': ('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 _not_an_iterable(self, node): - is_const = isinstance(node, astroid.Const) - is_string = is_const and isinstance(node.value, six.string_types) - return is_const and not is_string - - def _check_iterable(self, element, root_node): - infered = helpers.safe_infer(element) - if infered is not None: - if self._not_an_iterable(infered): - self.add_message('not-an-iterable', - args=element.as_string(), - node=root_node) - - def _check_mapping(self, element, root_node): - infered = helpers.safe_infer(element) - if infered is not None: - if isinstance(infered, astroid.Const): - self.add_message('not-a-mapping', - args=element.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(BasicErrorChecker(linter)) @@ -1563,4 +1485,3 @@ def register(linter): linter.register_checker(PassChecker(linter)) linter.register_checker(LambdaForComprehensionChecker(linter)) linter.register_checker(ComparisonChecker(linter)) - linter.register_checker(IterableChecker(linter)) diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 5f2bb23..f7df240 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -794,6 +794,126 @@ 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 _is_comprehension(self, node): + comprehensions = (astroid.ListComp, + astroid.SetComp, + astroid.DictComp) + return isinstance(node, comprehensions) + + def _is_string_value(self, node): + return isinstance(node, astroid.Const) and isinstance(node.value, six.string_types) + + def _is_iterable_value(self, value): + try: + value.getattr('__iter__') + return True + except astroid.NotFoundError: + return False + + def _is_old_style_iterator(self, value): + try: + value.getattr('__next__') + value.getattr('__len__') + return True + except astroid.NotFoundError: + return False + + def _check_iterable(self, node, root_node): + # for/set/dict-comprehensions can't be infered with astroid, + # so we check for them before the inference + if self._is_comprehension(node): + return + infered = helpers.safe_infer(node) + if infered is None: + return + if infered is astroid.YES: + return + if self._is_iterable_value(infered): + return + if self._is_old_style_iterator(infered): + return + if self._is_string_value(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): + return + infered = helpers.safe_infer(node) + if infered is None: + return + if infered is astroid.YES: + return + if isinstance(infered, astroid.Dict): + 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/unittest_checker_base.py b/pylint/test/unittest_checker_base.py index 0b1cddd..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 @@ -10,13 +9,6 @@ from pylint.checkers import base 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 DocstringTest(CheckerTestCase): CHECKER_CLASS = base.DocStringChecker @@ -300,141 +292,5 @@ class ComparisonTest(CheckerTestCase): self.checker.visit_compare(node) -class IterableTest(CheckerTestCase): - CHECKER_CLASS = base.IterableChecker - - def test_non_iterable_in_for(self): - node = test_utils.extract_node(""" - for i in 42: - print(i) - """) - message = Message('not-an-iterable', node=node, args='42') - with self.assertAddsMessages(message): - self.checker.visit_for(node) - - node = test_utils.extract_node(""" - for i in [1,2,3]: - print(i) - """) - with self.assertNoMessages(): - self.checker.visit_for(node) - - node = test_utils.extract_node(""" - def count(): - i = 0 - while True: - yield i - i += 1 - - for i in count(): - print(i) - """) - with self.assertNoMessages(): - self.walk(node.root()) - - node = test_utils.extract_node(""" - for i in "aeiou": - print(i) - """) - with self.assertNoMessages(): - self.checker.visit_for(node) - - @python33_and_newer - def test_non_iterable_in_yield_from(self): - node = test_utils.extract_node(""" - yield from 42 - """) - message = Message('not-an-iterable', node=node, args='42') - with self.assertAddsMessages(message): - self.checker.visit_yieldfrom(node) - - node = test_utils.extract_node(""" - yield from [1,2,3] - """) - with self.assertNoMessages(): - self.checker.visit_yieldfrom(node) - - def test_non_iterable_in_funcall_starargs(self): - node = test_utils.extract_node(""" - foo(*123) - """) - message = Message('not-an-iterable', node=node, args='123') - with self.assertAddsMessages(message): - self.checker.visit_call(node) - - node = test_utils.extract_node(""" - stuff = [1,2,3] - foo(*stuff) - """) - with self.assertNoMessages(): - self.walk(node.root()) - - def test_non_mapping_in_funcall_kwargs(self): - node = test_utils.extract_node(""" - foo(**123) - """) - message = Message('not-a-mapping', node=node, args='123') - with self.assertAddsMessages(message): - self.checker.visit_call(node) - - node = test_utils.extract_node(""" - foo(**retdict()) - """) - with self.assertNoMessages(): - self.walk(node.root()) - - def test_non_iterable_in_listcomp(self): - node = test_utils.extract_node(""" - [x ** 2 for x in 10] - """) - message = Message('not-an-iterable', node=node, args='10') - with self.assertAddsMessages(message): - self.checker.visit_listcomp(node) - - node = test_utils.extract_node(""" - [x ** 2 for x in range(10)] - """) - with self.assertNoMessages(): - self.checker.visit_listcomp(node) - - def test_non_iterable_in_dictcomp(self): - node = test_utils.extract_node(""" - {x: chr(x) for x in 256} - """) - message = Message('not-an-iterable', node=node, args='256') - with self.assertAddsMessages(message): - self.checker.visit_dictcomp(node) - - node = test_utils.extract_node(""" - {ord(x): x for x in "aoeui"} - """) - with self.assertNoMessages(): - self.checker.visit_dictcomp(node) - - def test_non_iterable_in_setcomp(self): - node = test_utils.extract_node(""" - {2 ** x for x in 10} - """) - message = Message('not-an-iterable', node=node, args='10') - with self.assertAddsMessages(message): - self.checker.visit_setcomp(node) - - node = test_utils.extract_node(""" - {2 ** x for x in range(10)} - """) - with self.assertNoMessages(): - self.checker.visit_setcomp(node) - - def test_non_iterable_in_generator(self): - node = test_utils.extract_node("__(x for x in 123)") - message = Message('not-an-iterable', node=node, args='123') - with self.assertAddsMessages(message): - self.walk(node.root()) - - node = test_utils.extract_node("__(chr(x) for x in range(25))") - with self.assertNoMessages(): - self.walk(node.root()) - - if __name__ == '__main__': unittest.main() diff --git a/pylint/test/unittest_checker_typecheck.py b/pylint/test/unittest_checker_typecheck.py index b7135ea..a2f3598 100644 --- a/pylint/test/unittest_checker_typecheck.py +++ b/pylint/test/unittest_checker_typecheck.py @@ -1,10 +1,19 @@ """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 @@ -88,6 +97,141 @@ class TypeCheckerTest(CheckerTestCase): with self.assertNoMessages(): self.checker.visit_attribute(node) +class IterableTest(CheckerTestCase): + CHECKER_CLASS = typecheck.IterableChecker + + def test_non_iterable_in_for(self): + node = test_utils.extract_node(""" + for i in 42: + print(i) + """) + message = Message('not-an-iterable', node=node, args='42') + with self.assertAddsMessages(message): + self.checker.visit_for(node) + + node = test_utils.extract_node(""" + for i in [1,2,3]: + print(i) + """) + with self.assertNoMessages(): + self.checker.visit_for(node) + + node = test_utils.extract_node(""" + def count(): + i = 0 + while True: + yield i + i += 1 + + for i in count(): + print(i) + """) + with self.assertNoMessages(): + self.walk(node.root()) + + node = test_utils.extract_node(""" + for i in "aeiou": + print(i) + """) + with self.assertNoMessages(): + self.checker.visit_for(node) + + @python33_and_newer + def test_non_iterable_in_yield_from(self): + node = test_utils.extract_node(""" + yield from 42 + """) + message = Message('not-an-iterable', node=node, args='42') + with self.assertAddsMessages(message): + self.checker.visit_yieldfrom(node) + + node = test_utils.extract_node(""" + yield from [1,2,3] + """) + with self.assertNoMessages(): + self.checker.visit_yieldfrom(node) + + def test_non_iterable_in_funcall_starargs(self): + node = test_utils.extract_node(""" + foo(*123) + """) + message = Message('not-an-iterable', node=node, args='123') + with self.assertAddsMessages(message): + self.checker.visit_call(node) + + node = test_utils.extract_node(""" + stuff = [1,2,3] + foo(*stuff) + """) + with self.assertNoMessages(): + self.walk(node.root()) + + def test_non_mapping_in_funcall_kwargs(self): + node = test_utils.extract_node(""" + foo(**123) + """) + message = Message('not-a-mapping', node=node, args='123') + with self.assertAddsMessages(message): + self.checker.visit_call(node) + + node = test_utils.extract_node(""" + foo(**retdict()) + """) + with self.assertNoMessages(): + self.walk(node.root()) + + def test_non_iterable_in_listcomp(self): + node = test_utils.extract_node(""" + [x ** 2 for x in 10] + """) + message = Message('not-an-iterable', node=node, args='10') + with self.assertAddsMessages(message): + self.checker.visit_listcomp(node) + + node = test_utils.extract_node(""" + [x ** 2 for x in range(10)] + """) + with self.assertNoMessages(): + self.checker.visit_listcomp(node) + + def test_non_iterable_in_dictcomp(self): + node = test_utils.extract_node(""" + {x: chr(x) for x in 256} + """) + message = Message('not-an-iterable', node=node, args='256') + with self.assertAddsMessages(message): + self.checker.visit_dictcomp(node) + + node = test_utils.extract_node(""" + {ord(x): x for x in "aoeui"} + """) + with self.assertNoMessages(): + self.checker.visit_dictcomp(node) + + def test_non_iterable_in_setcomp(self): + node = test_utils.extract_node(""" + {2 ** x for x in 10} + """) + message = Message('not-an-iterable', node=node, args='10') + with self.assertAddsMessages(message): + self.checker.visit_setcomp(node) + + node = test_utils.extract_node(""" + {2 ** x for x in range(10)} + """) + with self.assertNoMessages(): + self.checker.visit_setcomp(node) + + def test_non_iterable_in_generator(self): + node = test_utils.extract_node("__(x for x in 123)") + message = Message('not-an-iterable', node=node, args='123') + with self.assertAddsMessages(message): + self.walk(node.root()) + + node = test_utils.extract_node("__(chr(x) for x in range(25))") + with self.assertNoMessages(): + self.walk(node.root()) + if __name__ == '__main__': unittest.main() -- cgit v1.2.1 From 675f4a4b7325224b714c321e3f545c1c1ef1a760 Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Thu, 8 Oct 2015 02:53:11 +0300 Subject: Refactor iterable context checker --- pylint/checkers/typecheck.py | 86 +++++++++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index f7df240..d54c5d3 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -83,6 +83,58 @@ def _is_owner_ignored(owner, name, ignored_classes, ignored_modules): return any(name == ignore or qname == ignore for ignore in ignored_classes) +def _is_comprehension(node): + comprehensions = (astroid.ListComp, + astroid.SetComp, + astroid.DictComp) + return isinstance(node, comprehensions) + + +def _is_iterable(value): + 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 + + +def _is_iterator(value): + try: + value.getattr('__iter__') + if six.PY2: + value.getattr('next') + elif six.PY3: + value.getattr('__next__') + return True + except astroid.NotFoundError: + return False + + +def _is_old_style_iterator(value): + try: + value.getattr('__getitem__') + value.getattr('__len__') + return True + except astroid.NotFoundError: + return False + + +def _is_mapping(value): + try: + value.getattr('__getitem__') + value.getattr('__iter__') + value.getattr('__len__') + return True + except astroid.NotFoundError: + return False + MSGS = { 'E1101': ('%s %r has no %r member', 'no-member', @@ -819,45 +871,21 @@ class IterableChecker(BaseChecker): 'mapping is expected'), } - def _is_comprehension(self, node): - comprehensions = (astroid.ListComp, - astroid.SetComp, - astroid.DictComp) - return isinstance(node, comprehensions) - - def _is_string_value(self, node): - return isinstance(node, astroid.Const) and isinstance(node.value, six.string_types) - - def _is_iterable_value(self, value): - try: - value.getattr('__iter__') - return True - except astroid.NotFoundError: - return False - - def _is_old_style_iterator(self, value): - try: - value.getattr('__next__') - value.getattr('__len__') - return True - except astroid.NotFoundError: - return False - def _check_iterable(self, node, root_node): # for/set/dict-comprehensions can't be infered with astroid, # so we check for them before the inference - if self._is_comprehension(node): + if _is_comprehension(node): return infered = helpers.safe_infer(node) if infered is None: return if infered is astroid.YES: return - if self._is_iterable_value(infered): + if _is_iterable(infered): return - if self._is_old_style_iterator(infered): + if _is_iterator(infered): return - if self._is_string_value(infered): + if _is_old_style_iterator(infered): return self.add_message('not-an-iterable', args=node.as_string(), @@ -871,7 +899,7 @@ class IterableChecker(BaseChecker): return if infered is astroid.YES: return - if isinstance(infered, astroid.Dict): + if _is_mapping(infered): return self.add_message('not-a-mapping', args=node.as_string(), -- cgit v1.2.1 From 892108e5313120e0040cb1f260612030e9ed61d6 Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Thu, 8 Oct 2015 02:53:46 +0300 Subject: Add more positive functional tests for the iterable context checker --- pylint/test/functional/iterable_context.py | 96 ++++++++++++++++++++++++----- pylint/test/functional/iterable_context.txt | 8 --- 2 files changed, 79 insertions(+), 25 deletions(-) diff --git a/pylint/test/functional/iterable_context.py b/pylint/test/functional/iterable_context.py index 8282683..0571583 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 +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-init from __future__ import print_function # for-statement @@ -12,25 +12,87 @@ for i in 42: # [not-an-iterable] for i in True: # [not-an-iterable] pass -# funcall-starargs -def test(*args, **kwargs): - print(args, kwargs) +for i in range(10): + pass + +for i in ''.join(x ** 2 for x in range(10)): + pass + +numbers = [1, 2, 3] +inumbers = iter(numbers) + +for i in inumbers: + pass + +for i in "123": + pass + +for i in u"123": + pass + +for i in set(numbers): + pass + +for i in frozenset(numbers): + pass + +for i in dict(a=1, b=2): + pass + +for i in [x for x in range(10)]: + pass -test(*1) # [not-an-iterable] -test(*False) # [not-an-iterable] +for i in {x for x in range(1, 100, 2)}: + pass + +for i in {x: 10 - x for x in range(10)}: + pass + +def powers_of_two(): + k = 0 + while k < 10: + yield 2 ** k + k += 1 + +for i in powers_of_two(): + pass + + +# check for old-style iterator +class C(object): + def __getitem__(self, k): + if k > 10: + raise IndexError + return k + 1 + + def __len__(self): + return 10 -# funcall-kwargs -test(**1) # [not-a-mapping] -test(**None) # [not-a-mapping] +for i in C(): + print(i) -# list-comprehension -test([3 ** x for x in 10]) # [not-an-iterable] -# dict-comprehension -test({k: chr(k) for k in 128}) # [not-an-iterable] +# check for custom iterators +class A: + pass + + +class B: + def __iter__(self): + return self + + def __next__(self): + return 1 + + def next(self): + return 1 -# set-comprehension -test({x for x in 32}) # [not-an-iterable] -# generator-expression -test(str(x) for x in 10) # [not-an-iterable] +def test(*args): + pass + + +test(*A()) # [not-an-iterable] +test(*B()) +for i in A(): # [not-an-iterable] + pass diff --git a/pylint/test/functional/iterable_context.txt b/pylint/test/functional/iterable_context.txt index 8c9f098..b618ffb 100644 --- a/pylint/test/functional/iterable_context.txt +++ b/pylint/test/functional/iterable_context.txt @@ -1,10 +1,2 @@ not-an-iterable:9::Non-iterable value 42 is used in an iterating context not-an-iterable:12::Non-iterable value True is used in an iterating context -not-an-iterable:19::Non-iterable value 1 is used in an iterating context -not-an-iterable:20::Non-iterable value False is used in an iterating context -not-a-mapping:23::Non-mapping value 1 is used in a mapping context -not-a-mapping:24::Non-mapping value None is used in a mapping context -not-an-iterable:27::Non-iterable value 10 is used in an iterating context -not-an-iterable:30::Non-iterable value 128 is used in an iterating context -not-an-iterable:33::Non-iterable value 32 is used in an iterating context -not-an-iterable:36::Non-iterable value 10 is used in an iterating context -- cgit v1.2.1 From f9550ad341a5155f32f895d068c8bbcd29e07606 Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Thu, 8 Oct 2015 16:22:42 +0300 Subject: Handle more corner cases in iterable/mapping checker --- pylint/checkers/typecheck.py | 16 +++++- pylint/test/functional/iterable_context.py | 77 ++++++++++++++++++----------- pylint/test/functional/iterable_context.txt | 12 ++++- pylint/test/functional/mapping_context.py | 36 ++++++++++++++ pylint/test/functional/mapping_context.txt | 1 + 5 files changed, 109 insertions(+), 33 deletions(-) create mode 100644 pylint/test/functional/mapping_context.py create mode 100644 pylint/test/functional/mapping_context.txt diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index d54c5d3..83d9742 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -90,7 +90,13 @@ 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 @@ -106,6 +112,8 @@ def _is_iterable(value): def _is_iterator(value): + if _is_class_def(value): + return False try: value.getattr('__iter__') if six.PY2: @@ -118,6 +126,8 @@ def _is_iterator(value): def _is_old_style_iterator(value): + if _is_class_def(value): + return False try: value.getattr('__getitem__') value.getattr('__len__') @@ -127,10 +137,12 @@ def _is_old_style_iterator(value): def _is_mapping(value): + if _is_class_def(value): + return False try: value.getattr('__getitem__') value.getattr('__iter__') - value.getattr('__len__') + value.getattr('keys') return True except astroid.NotFoundError: return False @@ -873,7 +885,7 @@ 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 the inference + # so we check for them before checking infered value if _is_comprehension(node): return infered = helpers.safe_infer(node) diff --git a/pylint/test/functional/iterable_context.py b/pylint/test/functional/iterable_context.py index 0571583..47400f9 100644 --- a/pylint/test/functional/iterable_context.py +++ b/pylint/test/functional/iterable_context.py @@ -2,32 +2,28 @@ Checks that primitive values are not used in an iterating/mapping context. """ -# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-init +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-init,no-self-use from __future__ import print_function -# for-statement -for i in 42: # [not-an-iterable] - pass +# primitives +numbers = [1, 2, 3] -for i in True: # [not-an-iterable] +for i in numbers: pass -for i in range(10): +for i in iter(numbers): pass -for i in ''.join(x ** 2 for x in range(10)): +for i in "123": pass -numbers = [1, 2, 3] -inumbers = iter(numbers) - -for i in inumbers: +for i in u"123": pass -for i in "123": +for i in b"123": pass -for i in u"123": +for i in bytearray(b"123"): pass for i in set(numbers): @@ -39,6 +35,7 @@ for i in frozenset(numbers): for i in dict(a=1, b=2): pass +# comprehensions for i in [x for x in range(10)]: pass @@ -48,6 +45,7 @@ for i in {x for x in range(1, 100, 2)}: for i in {x: 10 - x for x in range(10)}: pass +# generators def powers_of_two(): k = 0 while k < 10: @@ -57,9 +55,25 @@ def powers_of_two(): 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 -# check for old-style iterator class C(object): + "old-style iterator" def __getitem__(self, k): if k > 10: raise IndexError @@ -72,27 +86,32 @@ for i in C(): print(i) -# check for custom iterators -class A: - pass - +def test(*args): + print(args) -class B: - def __iter__(self): - return self - def __next__(self): - return 1 +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 - def next(self): - return 1 +for i in range: # [not-an-iterable] + pass +# check that primitive non-iterable types are catched +for i in True: # [not-an-iterable] + pass -def test(*args): +for i in None: # [not-an-iterable] pass +for i in 8.5: # [not-an-iterable] + pass -test(*A()) # [not-an-iterable] -test(*B()) -for i in A(): # [not-an-iterable] +for i in 10: # [not-an-iterable] pass diff --git a/pylint/test/functional/iterable_context.txt b/pylint/test/functional/iterable_context.txt index b618ffb..fbe1433 100644 --- a/pylint/test/functional/iterable_context.txt +++ b/pylint/test/functional/iterable_context.txt @@ -1,2 +1,10 @@ -not-an-iterable:9::Non-iterable value 42 is used in an iterating context -not-an-iterable:12::Non-iterable value True is used in an iterating context +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/mapping_context.py b/pylint/test/functional/mapping_context.py new file mode 100644 index 0000000..d9e0d2c --- /dev/null +++ b/pylint/test/functional/mapping_context.py @@ -0,0 +1,36 @@ +""" +Checks that only valid values are used in a mapping context. +""" +# pylint: disable=missing-docstring,invalid-name,too-few-public-methods +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 __iter__(self): + return iter(self.data) + + def __getitem__(self, key): + return self.data[key] + + def keys(self): + return self.data.keys() + +test(**CustomMapping()) + +test(**CustomMapping) # [not-a-mapping] diff --git a/pylint/test/functional/mapping_context.txt b/pylint/test/functional/mapping_context.txt new file mode 100644 index 0000000..f27ca97 --- /dev/null +++ b/pylint/test/functional/mapping_context.txt @@ -0,0 +1 @@ +not-a-mapping:36::Non-mapping value CustomMapping is used in a mapping context -- cgit v1.2.1 From af2d53301e296856bba66e04b06d7544e5a6343d Mon Sep 17 00:00:00 2001 From: Dmitry Pribysh Date: Thu, 8 Oct 2015 16:23:04 +0300 Subject: Remove redundant unittests for iterable checker --- pylint/test/unittest_checker_typecheck.py | 135 ------------------------------ 1 file changed, 135 deletions(-) diff --git a/pylint/test/unittest_checker_typecheck.py b/pylint/test/unittest_checker_typecheck.py index a2f3598..b89a81b 100644 --- a/pylint/test/unittest_checker_typecheck.py +++ b/pylint/test/unittest_checker_typecheck.py @@ -97,141 +97,6 @@ class TypeCheckerTest(CheckerTestCase): with self.assertNoMessages(): self.checker.visit_attribute(node) -class IterableTest(CheckerTestCase): - CHECKER_CLASS = typecheck.IterableChecker - - def test_non_iterable_in_for(self): - node = test_utils.extract_node(""" - for i in 42: - print(i) - """) - message = Message('not-an-iterable', node=node, args='42') - with self.assertAddsMessages(message): - self.checker.visit_for(node) - - node = test_utils.extract_node(""" - for i in [1,2,3]: - print(i) - """) - with self.assertNoMessages(): - self.checker.visit_for(node) - - node = test_utils.extract_node(""" - def count(): - i = 0 - while True: - yield i - i += 1 - - for i in count(): - print(i) - """) - with self.assertNoMessages(): - self.walk(node.root()) - - node = test_utils.extract_node(""" - for i in "aeiou": - print(i) - """) - with self.assertNoMessages(): - self.checker.visit_for(node) - - @python33_and_newer - def test_non_iterable_in_yield_from(self): - node = test_utils.extract_node(""" - yield from 42 - """) - message = Message('not-an-iterable', node=node, args='42') - with self.assertAddsMessages(message): - self.checker.visit_yieldfrom(node) - - node = test_utils.extract_node(""" - yield from [1,2,3] - """) - with self.assertNoMessages(): - self.checker.visit_yieldfrom(node) - - def test_non_iterable_in_funcall_starargs(self): - node = test_utils.extract_node(""" - foo(*123) - """) - message = Message('not-an-iterable', node=node, args='123') - with self.assertAddsMessages(message): - self.checker.visit_call(node) - - node = test_utils.extract_node(""" - stuff = [1,2,3] - foo(*stuff) - """) - with self.assertNoMessages(): - self.walk(node.root()) - - def test_non_mapping_in_funcall_kwargs(self): - node = test_utils.extract_node(""" - foo(**123) - """) - message = Message('not-a-mapping', node=node, args='123') - with self.assertAddsMessages(message): - self.checker.visit_call(node) - - node = test_utils.extract_node(""" - foo(**retdict()) - """) - with self.assertNoMessages(): - self.walk(node.root()) - - def test_non_iterable_in_listcomp(self): - node = test_utils.extract_node(""" - [x ** 2 for x in 10] - """) - message = Message('not-an-iterable', node=node, args='10') - with self.assertAddsMessages(message): - self.checker.visit_listcomp(node) - - node = test_utils.extract_node(""" - [x ** 2 for x in range(10)] - """) - with self.assertNoMessages(): - self.checker.visit_listcomp(node) - - def test_non_iterable_in_dictcomp(self): - node = test_utils.extract_node(""" - {x: chr(x) for x in 256} - """) - message = Message('not-an-iterable', node=node, args='256') - with self.assertAddsMessages(message): - self.checker.visit_dictcomp(node) - - node = test_utils.extract_node(""" - {ord(x): x for x in "aoeui"} - """) - with self.assertNoMessages(): - self.checker.visit_dictcomp(node) - - def test_non_iterable_in_setcomp(self): - node = test_utils.extract_node(""" - {2 ** x for x in 10} - """) - message = Message('not-an-iterable', node=node, args='10') - with self.assertAddsMessages(message): - self.checker.visit_setcomp(node) - - node = test_utils.extract_node(""" - {2 ** x for x in range(10)} - """) - with self.assertNoMessages(): - self.checker.visit_setcomp(node) - - def test_non_iterable_in_generator(self): - node = test_utils.extract_node("__(x for x in 123)") - message = Message('not-an-iterable', node=node, args='123') - with self.assertAddsMessages(message): - self.walk(node.root()) - - node = test_utils.extract_node("__(chr(x) for x in range(25))") - with self.assertNoMessages(): - self.walk(node.root()) - if __name__ == '__main__': unittest.main() -- cgit v1.2.1 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