diff options
| -rw-r--r-- | ChangeLog | 4 | ||||
| -rw-r--r-- | pylint/checkers/base.py | 121 | ||||
| -rw-r--r-- | pylint/checkers/classes.py | 11 | ||||
| -rw-r--r-- | pylint/checkers/stdlib.py | 45 | ||||
| -rw-r--r-- | pylint/checkers/typecheck.py | 161 | ||||
| -rw-r--r-- | pylint/checkers/utils.py | 114 | ||||
| -rw-r--r-- | pylint/checkers/variables.py | 20 | ||||
| -rw-r--r-- | pylint/test/functional/consider_using_enumerate.py | 43 | ||||
| -rw-r--r-- | pylint/test/functional/consider_using_enumerate.txt | 1 | ||||
| -rw-r--r-- | pylint/test/functional/iterable_context.py | 40 | ||||
| -rw-r--r-- | pylint/test/functional/mapping_context.py | 40 | ||||
| -rw-r--r-- | pylint/test/functional/membership_protocol.py | 46 | ||||
| -rw-r--r-- | pylint/test/functional/membership_protocol.txt | 14 | ||||
| -rw-r--r-- | pylint/test/functional/unpacking_non_sequence.py | 25 | ||||
| -rw-r--r-- | pylint/test/functional/unpacking_non_sequence.txt | 24 | ||||
| -rw-r--r-- | tox.ini | 6 |
16 files changed, 491 insertions, 224 deletions
@@ -365,6 +365,10 @@ ChangeLog for Pylint encountered in languages with variabile assignments in conditional statements. + * Add a new convention message, 'consider-using-enumerate', which is + emitted when code that uses `range` and `len` for iterating is encountered. + Closes issue #684. + 2015-03-14 -- 1.4.3 diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index c9e8b75..8f4eca5 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -62,6 +62,14 @@ REVERSED_PROTOCOL_METHOD = '__reversed__' SEQUENCE_PROTOCOL_METHODS = ('__getitem__', '__len__') REVERSED_METHODS = (SEQUENCE_PROTOCOL_METHODS, (REVERSED_PROTOCOL_METHOD, )) +TYPECHECK_COMPARISON_OPERATORS = frozenset(('is', 'is not', '==', + '!=', 'in', 'not in')) +LITERAL_NODE_TYPES = (astroid.Const, astroid.Dict, astroid.List, astroid.Set) +UNITTEST_CASE = 'unittest.case' +if sys.version_info >= (3, 0): + TYPE_QNAME = 'builtins.type' +else: + TYPE_QNAME = '__builtin__.type' PY33 = sys.version_info >= (3, 3) PY3K = sys.version_info >= (3, 0) @@ -1455,6 +1463,82 @@ class LambdaForComprehensionChecker(_BasicChecker): self.add_message('deprecated-lambda', node=node) +class RecommandationChecker(_BasicChecker): + msgs = {'C0200': ('Consider using enumerate instead of iterating with range and len', + 'consider-using-enumerate', + 'Emitted when code that iterates with range and len is ' + 'encountered. Such code can be simplified by using the ' + 'enumerate builtin.') + } + + @staticmethod + def _is_builtin(node, function): + inferred = helpers.safe_infer(node) + if not inferred: + return False + return is_builtin_object(inferred) and inferred.name == function + + @check_messages('consider-using-enumerate') + def visit_for(self, node): + """Emit a convention whenever range and len are used for indexing.""" + # Verify that we have a `range(len(...))` call and that the object + # which is iterated is used as a subscript in the body of the for. + + # Is it a proper range call? + if not isinstance(node.iter, astroid.Call): + return + if not self._is_builtin(node.iter.func, 'range'): + return + if len(node.iter.args) != 1: + return + + # Is it a proper len call? + if not isinstance(node.iter.args[0], astroid.Call): + return + second_func = node.iter.args[0].func + if not self._is_builtin(second_func, 'len'): + return + len_args = node.iter.args[0].args + if not len_args or len(len_args) != 1: + return + iterating_object = len_args[0] + if not isinstance(iterating_object, astroid.Name): + return + + # Verify that the body of the for loop uses a subscript + # with the object that was iterated. This uses some heuristics + # in order to make sure that the same object is used in the + # for body. + for child in node.body: + for subscript in child.nodes_of_class(astroid.Subscript): + if not isinstance(subscript.value, astroid.Name): + continue + if not isinstance(subscript.slice, astroid.Index): + continue + if not isinstance(subscript.slice.value, astroid.Name): + continue + if subscript.slice.value.name != node.target.name: + continue + if iterating_object.name != subscript.value.name: + continue + if subscript.value.scope() != node.scope(): + # Ignore this subscript if it's not in the same + # scope. This means that in the body of the for + # loop, another scope was created, where the same + # name for the iterating object was used. + continue + self.add_message('consider-using-enumerate', node=node) + return + + +def _is_one_arg_pos_call(call): + """Is this a call with exactly 1 argument, + where that argument is positional? + """ + return (isinstance(call, astroid.Call) + and len(call.args) == 1 and not call.keywords) + + class ComparisonChecker(_BasicChecker): """Checks for comparisons @@ -1472,6 +1556,13 @@ class ComparisonChecker(_BasicChecker): 'Used when the constant is placed on the left side' 'of a comparison. It is usually clearer in intent to ' 'place it in the right hand side of the comparison.'), + 'C0123': ('Using type() instead of isinstance() for a typecheck.', + 'unidiomatic-typecheck', + 'The idiomatic way to perform an explicit typecheck in ' + 'Python is to use isinstance(x, Y) rather than ' + 'type(x) == Y, type(x) is Y. Though there are unusual ' + 'situations where these give different results.', + {'old_names': [('W0154', 'unidiomatic-typecheck')]}), } def _check_singleton_comparison(self, singleton, root_node): @@ -1498,8 +1589,10 @@ class ComparisonChecker(_BasicChecker): self.add_message('misplaced-comparison-constant', node=node, args=(suggestion,)) - @check_messages('singleton-comparison', 'misplaced-comparison-constant') + @check_messages('singleton-comparison', 'misplaced-comparison-constant', + 'unidiomatic-typecheck') def visit_compare(self, node): + self._check_unidiomatic_typecheck(node) # NOTE: this checker only works with binary comparisons like 'x == 42' # but not 'x == y == 42' if len(node.ops) != 1: @@ -1516,6 +1609,31 @@ class ComparisonChecker(_BasicChecker): elif isinstance(right, astroid.Const): self._check_singleton_comparison(right, node) + def _check_unidiomatic_typecheck(self, node): + operator, right = node.ops[0] + if operator in TYPECHECK_COMPARISON_OPERATORS: + left = node.left + if _is_one_arg_pos_call(left): + self._check_type_x_is_y(node, left, operator, right) + + def _check_type_x_is_y(self, node, left, operator, right): + """Check for expressions like type(x) == Y.""" + left_func = helpers.safe_infer(left.func) + if not (isinstance(left_func, astroid.ClassDef) + and left_func.qname() == TYPE_QNAME): + return + + if operator in ('is', 'is not') and _is_one_arg_pos_call(right): + right_func = helpers.safe_infer(right.func) + if (isinstance(right_func, astroid.ClassDef) + and right_func.qname() == TYPE_QNAME): + # type(x) == type(a) + right_arg = helpers.safe_infer(right.args[0]) + if not isinstance(right_arg, LITERAL_NODE_TYPES): + # not e.g. type(x) == type([]) + return + self.add_message('unidiomatic-typecheck', node=node) + def register(linter): """required method to auto register this checker""" @@ -1526,3 +1644,4 @@ def register(linter): linter.register_checker(PassChecker(linter)) linter.register_checker(LambdaForComprehensionChecker(linter)) linter.register_checker(ComparisonChecker(linter)) + linter.register_checker(RecommandationChecker(linter)) diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 014dd4a..ae9c727 100644 --- a/pylint/checkers/classes.py +++ b/pylint/checkers/classes.py @@ -34,7 +34,7 @@ from pylint.checkers.utils import ( overrides_a_method, check_messages, is_attr_private, is_attr_protected, node_frame_class, is_builtin_object, decorated_with_property, unimplemented_abstract_methods, - decorated_with) + decorated_with, class_is_abstract) from pylint.utils import deprecated_option, get_global_option import six @@ -102,15 +102,6 @@ def _called_in_methods(func, klass, methods): return True return False -def class_is_abstract(node): - """return true if the given class node should be considered as an abstract - class - """ - for method in node.methods(): - if method.parent.frame() is node: - if method.is_abstract(pass_is_abstract=False): - return True - return False def _is_attribute_property(name, klass): """ Check if the given attribute *name* is a property diff --git a/pylint/checkers/stdlib.py b/pylint/checkers/stdlib.py index 7df71b2..f583215 100644 --- a/pylint/checkers/stdlib.py +++ b/pylint/checkers/stdlib.py @@ -27,17 +27,12 @@ from pylint.checkers import BaseChecker from pylint.checkers import utils -TYPECHECK_COMPARISON_OPERATORS = frozenset(('is', 'is not', '==', - '!=', 'in', 'not in')) -LITERAL_NODE_TYPES = (astroid.Const, astroid.Dict, astroid.List, astroid.Set) OPEN_FILES = {'open', 'file'} UNITTEST_CASE = 'unittest.case' if sys.version_info >= (3, 0): OPEN_MODULE = '_io' - TYPE_QNAME = 'builtins.type' else: OPEN_MODULE = '__builtin__' - TYPE_QNAME = '__builtin__.type' def _check_mode_str(mode): @@ -82,14 +77,6 @@ def _check_mode_str(mode): return True -def _is_one_arg_pos_call(call): - """Is this a call with exactly 1 argument, - where that argument is positional? - """ - return (isinstance(call, astroid.Call) - and len(call.args) == 1 and not call.keywords) - - class StdlibChecker(BaseChecker): __implements__ = (IAstroidChecker,) name = 'stdlib' @@ -114,12 +101,6 @@ class StdlibChecker(BaseChecker): 'a condition. If a constant is passed as parameter, that ' 'condition will be always true. In this case a warning ' 'should be emitted.'), - 'W1504': ('Using type() instead of isinstance() for a typecheck.', - 'unidiomatic-typecheck', - 'The idiomatic way to perform an explicit typecheck in ' - 'Python is to use isinstance(x, Y) rather than ' - 'type(x) == Y, type(x) is Y. Though there are unusual ' - 'situations where these give different results.'), 'W1505': ('Using deprecated method %s()', 'deprecated-method', 'The method is marked as deprecated and will be removed in ' @@ -231,14 +212,6 @@ class StdlibChecker(BaseChecker): for value in node.values: self._check_datetime(value) - @utils.check_messages('unidiomatic-typecheck') - def visit_compare(self, node): - operator, right = node.ops[0] - if operator in TYPECHECK_COMPARISON_OPERATORS: - left = node.left - if _is_one_arg_pos_call(left): - self._check_type_x_is_y(node, left, operator, right) - def _check_deprecated_method(self, node, infer): py_vers = sys.version_info[0] @@ -296,24 +269,6 @@ class StdlibChecker(BaseChecker): self.add_message('bad-open-mode', node=node, args=mode_arg.value) - def _check_type_x_is_y(self, node, left, operator, right): - """Check for expressions like type(x) == Y.""" - left_func = helpers.safe_infer(left.func) - if not (isinstance(left_func, astroid.ClassDef) - and left_func.qname() == TYPE_QNAME): - return - - if operator in ('is', 'is not') and _is_one_arg_pos_call(right): - right_func = helpers.safe_infer(right.func) - if (isinstance(right_func, astroid.ClassDef) - and right_func.qname() == TYPE_QNAME): - # type(x) == type(a) - right_arg = helpers.safe_infer(right.args[0]) - if not isinstance(right_arg, LITERAL_NODE_TYPES): - # not e.g. type(x) == type([]) - return - self.add_message('unidiomatic-typecheck', node=node) - def register(linter): """required method to auto register this checker """ diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index e9fddbd..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) + 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,47 +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) - 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 _supports_membership_test(value): - return _hasattr(value, CONTAINS_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', @@ -841,36 +797,17 @@ accessed. Python regular expressions are accepted.'} args=str(error), node=node) def _check_membership_test(self, node): - # instance supports membership test in either of those cases: - # 1. instance defines __contains__ method - # 2. instance is iterable (defines __iter__ or __getitem__) - if _is_comprehension(node) or _is_inside_mixin_declaration(node): + if is_inside_abstract_class(node): + return + if is_comprehension(node): return - infered = helpers.safe_infer(node) if infered is None or infered is astroid.YES: return - - # classes can be iterables/containers too - if isinstance(infered, astroid.ClassDef): - if not helpers.has_known_bases(infered): - return - meta = infered.metaclass() - if meta is not None: - if _supports_membership_test(meta): - return - if _is_iterable(meta): - return - - if isinstance(infered, astroid.Instance): - if not helpers.has_known_bases(infered): - return - if _supports_membership_test(infered) or _is_iterable(infered): - return - - self.add_message('unsupported-membership-test', - args=node.as_string(), - node=node) + 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): @@ -906,96 +843,66 @@ class IterableChecker(BaseChecker): '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): + def _check_iterable(self, node): + if is_inside_abstract_class(node): + return + if is_comprehension(node): return - infered = helpers.safe_infer(node) if infered is None or infered is astroid.YES: return + if not is_iterable(infered): + self.add_message('not-an-iterable', + args=node.as_string(), + node=node) - 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): + def _check_mapping(self, node): + if is_inside_abstract_class(node): + return + if isinstance(node, astroid.DictComp): 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) + 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 443e32d..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. @@ -573,6 +579,114 @@ def node_ignores_exception(node, exception): return False +def class_is_abstract(node): + """return true if the given class node should be considered as an abstract + class + """ + for method in node.methods(): + if method.parent.frame() is node: + if method.is_abstract(pass_is_abstract=False): + return True + 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/consider_using_enumerate.py b/pylint/test/functional/consider_using_enumerate.py new file mode 100644 index 0000000..f906da0 --- /dev/null +++ b/pylint/test/functional/consider_using_enumerate.py @@ -0,0 +1,43 @@ +"""Emit a message for iteration through range and len is encountered."""
+
+# pylint: disable=missing-docstring, import-error
+
+def bad():
+ iterable = [1, 2, 3]
+ for obj in range(len(iterable)): # [consider-using-enumerate]
+ yield iterable[obj]
+
+
+def good():
+ iterable = other_obj = [1, 2, 3]
+ total = 0
+ for obj in range(len(iterable)):
+ total += obj
+ yield total
+ yield iterable[obj + 1: 2]
+ yield iterable[len(obj)]
+ for obj in iterable:
+ yield iterable[obj - 1]
+
+ for index, obj in enumerate(iterable):
+ yield iterable[index]
+ for index in range(0, 10):
+ yield iterable[index + 1]
+ for index in range(10):
+ yield iterable[index]
+ for index in range(len([1, 2, 3, 4])):
+ yield index
+ for index in range(len(iterable)):
+ yield [1, 2, 3][index]
+ yield len([1, 2, 3])
+ for index in range(len(iterable)):
+ yield other_obj[index]
+
+ from unknown import unknown
+ for index in range(unknown(iterable)):
+ yield iterable[index]
+
+ for index in range(len(iterable)):
+ def test(iterable):
+ return iterable[index]
+ yield test([1, 2, 3])
diff --git a/pylint/test/functional/consider_using_enumerate.txt b/pylint/test/functional/consider_using_enumerate.txt new file mode 100644 index 0000000..36cd55d --- /dev/null +++ b/pylint/test/functional/consider_using_enumerate.txt @@ -0,0 +1 @@ +consider-using-enumerate:7:bad:Consider using enumerate instead of iterating with range and len diff --git a/pylint/test/functional/iterable_context.py b/pylint/test/functional/iterable_context.py index 8dfcbbe..fa4e617 100644 --- a/pylint/test/functional/iterable_context.py +++ b/pylint/test/functional/iterable_context.py @@ -127,7 +127,7 @@ m = MyClass() for i in m: print(i) -# skip checks if statement is inside mixin class +# skip checks if statement is inside mixin/base/abstract class class ManagedAccessViewMixin(object): access_requirements = None @@ -137,5 +137,43 @@ class ManagedAccessViewMixin(object): def dispatch(self, *_args, **_kwargs): klasses = self.get_access_requirements() + # no error should be emitted here for requirement in klasses: print(requirement) + +class BaseType(object): + valid_values = None + + def validate(self, value): + if self.valid_values is None: + return True + else: + # error should not be emitted here + for v in self.valid_values: + if value == v: + return True + return False + +class AbstractUrlMarkManager(object): + def __init__(self): + self._lineparser = None + self._init_lineparser() + # error should not be emitted here + for line in self._lineparser: + print(line) + + def _init_lineparser(self): + raise NotImplementedError + +# class is not named as abstract +# but still is deduceably abstract +class UrlMarkManager(object): + def __init__(self): + self._lineparser = None + self._init_lineparser() + # error should not be emitted here + for line in self._lineparser: + print(line) + + def _init_lineparser(self): + raise NotImplementedError diff --git a/pylint/test/functional/mapping_context.py b/pylint/test/functional/mapping_context.py index cfab8dc..72457b8 100644 --- a/pylint/test/functional/mapping_context.py +++ b/pylint/test/functional/mapping_context.py @@ -36,7 +36,7 @@ class NotMapping(object): test(**NotMapping()) # [not-a-mapping] -# skip checks if statement is inside mixin class +# skip checks if statement is inside mixin/base/abstract class class SomeMixin(object): kwargs = None @@ -50,6 +50,44 @@ class SomeMixin(object): kws = self.get_kwargs() self.run(**kws) +class AbstractThing(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) + +class BaseThing(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) + +# abstract class +class Thing(object): + def get_kwargs(self): + raise NotImplementedError + + def run(self, **kwargs): + print(kwargs) + + def dispatch(self): + kwargs = self.get_kwargs() + self.run(**kwargs) + # skip uninferable instances from some_missing_module import Mapping diff --git a/pylint/test/functional/membership_protocol.py b/pylint/test/functional/membership_protocol.py index 7b3a46f..0fd4886 100644 --- a/pylint/test/functional/membership_protocol.py +++ b/pylint/test/functional/membership_protocol.py @@ -5,9 +5,9 @@ 1 in {'a': 1, 'b': 2} 1 in {1, 2, 3} 1 in (1, 2, 3) -1 in "123" -1 in u"123" -1 in bytearray(b"123") +'1' in "123" +'1' in u"123" +'1' in bytearray(b"123") 1 in frozenset([1, 2, 3]) # comprehensions @@ -59,7 +59,7 @@ class MaybeIterable(ImportedClass): 10 in MaybeIterable() -# do not emit warning inside mixins +# do not emit warning inside mixins/abstract/base classes class UsefulMixin(object): stuff = None @@ -71,6 +71,44 @@ class UsefulMixin(object): if thing in stuff: pass +class BaseThing(object): + valid_values = None + + def validate(self, value): + if self.valid_values is None: + return True + else: + # error should not be emitted here + return value in self.valid_values + +class AbstractThing(object): + valid_values = None + + def validate(self, value): + if self.valid_values is None: + return True + else: + # error should not be emitted here + return value in self.valid_values + +# class is not named as abstract +# but still is deduceably abstract +class Thing(object): + valid_values = None + + def __init__(self): + self._init_values() + + def validate(self, value): + if self.valid_values is None: + return True + else: + # error should not be emitted here + return value in self.valid_values + + def _init_values(self): + raise NotImplementedError + # error cases 42 in 42 # [unsupported-membership-test] 42 not in None # [unsupported-membership-test] diff --git a/pylint/test/functional/membership_protocol.txt b/pylint/test/functional/membership_protocol.txt index 6e9bd8e..edb2227 100644 --- a/pylint/test/functional/membership_protocol.txt +++ b/pylint/test/functional/membership_protocol.txt @@ -1,7 +1,7 @@ -unsupported-membership-test:75::Value '42' doesn't support membership test -unsupported-membership-test:76::Value 'None' doesn't support membership test -unsupported-membership-test:77::Value '8.5' doesn't support membership test -unsupported-membership-test:82::Value 'EmptyClass()' doesn't support membership test -unsupported-membership-test:83::Value 'EmptyClass' doesn't support membership test -unsupported-membership-test:84::Value 'count' doesn't support membership test -unsupported-membership-test:85::Value 'range' doesn't support membership test +unsupported-membership-test:113::Value '42' doesn't support membership test +unsupported-membership-test:114::Value 'None' doesn't support membership test +unsupported-membership-test:115::Value '8.5' doesn't support membership test +unsupported-membership-test:120::Value 'EmptyClass()' doesn't support membership test +unsupported-membership-test:121::Value 'EmptyClass' doesn't support membership test +unsupported-membership-test:122::Value 'count' doesn't support membership test +unsupported-membership-test:123::Value 'range' doesn't support membership test 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 @@ -5,14 +5,14 @@ envlist = py27, py33, pylint [testenv:pylint] deps = - hg+https://bitbucket.org/logilab/astroid/ + hg+https://bitbucket.org/logilab/astroid@master six hg+https://bitbucket.org/logilab/pylint commands = pylint -rn --rcfile={toxinidir}/pylintrc {envsitepackagesdir}/pylint [testenv] deps = - hg+https://bitbucket.org/logilab/astroid/ + hg+https://bitbucket.org/logilab/astroid@master six commands = python -Wi -m unittest discover -s {envsitepackagesdir}/pylint/test/ -p {posargs:*test_*}.py -changedir = {toxworkdir} +changedir = {toxworkdir}
\ No newline at end of file |