From 662421f1f74303b547e501269a30020045a6ed3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sylvain=20Th=C3=A9nault?= Date: Thu, 7 Jun 2012 14:54:03 +0200 Subject: Add checking for protected attribute assignement, closes #7394. Remove checking for protected attribute access via super(). Move common code for checking protected attribute accessing in a new function _check_protected_attribute_access. --- checkers/classes.py | 60 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 10 deletions(-) (limited to 'checkers') diff --git a/checkers/classes.py b/checkers/classes.py index 74f2e459a..8001821e6 100644 --- a/checkers/classes.py +++ b/checkers/classes.py @@ -18,7 +18,7 @@ from __future__ import generators from logilab import astng -from logilab.astng import YES, Instance, are_exclusive +from logilab.astng import YES, Instance, are_exclusive, AssAttr from pylint.interfaces import IASTNGChecker from pylint.checkers import BaseChecker @@ -300,32 +300,66 @@ a class method.'} methods) """ attrname = node.attrname - if self._first_attrs and isinstance(node.expr, astng.Name) and \ - node.expr.name == self._first_attrs[-1]: + # Check self + if self.is_first_attr(node): self._accessed[-1].setdefault(attrname, []).append(node) return if 'W0212' not in self.active_msgs: return + + self._check_protected_attribute_access(node) + + def visit_assign(self, assign_node): + if 'W0212' not in self.active_msgs: + return + + node = assign_node.targets[0] + if not isinstance(node, AssAttr): + return + + if self.is_first_attr(node): + return + + self._check_protected_attribute_access(node) + + def _check_protected_attribute_access(self, node): + '''Given an attribute access node (set or get), check if attribute + access is legitimate. Call _check_first_attr with node before calling + this method. Valid cases are: + * self._attr in a method or cls._attr in a classmethod. Checked by + _check_first_attr. + * Klass._attr inside "Klass" class. + * Klass2._attr inside "Klass" class when Klass2 is a base class of + Klass. + ''' + attrname = node.attrname + if attrname[0] == '_' and not attrname == '_' and not ( - attrname.startswith('__') and attrname.endswith('__')): - # XXX move this in a reusable function + attrname.startswith('__') and attrname.endswith('__')): + klass = node.frame() + while klass is not None and not isinstance(klass, astng.Class): if klass.parent is None: klass = None else: klass = klass.parent.frame() + # XXX infer to be more safe and less dirty ?? # in classes, check we are not getting a parent method # through the class object or through super callee = node.expr.as_string() - if klass is None or not (callee == klass.name or - callee in klass.basenames - or (isinstance(node.expr, astng.CallFunc) - and isinstance(node.expr.func, astng.Name) - and node.expr.func.name == 'super')): + + # We are not in a class, no remaining valid case + if klass is None: self.add_message('W0212', node=node, args=attrname) + return + + # We are in a class, one remaining valid cases, Klass._attr inside + # Klass + if not (callee == klass.name or callee in klass.basenames): + self.add_message('W0212', node=node, args=attrname) def visit_name(self, node): """check if the name handle an access to a class member @@ -537,6 +571,12 @@ a class method.'} elif len(method1.args.defaults) < len(refmethod.args.defaults): self.add_message('W0222', args=class_type, node=method1) + def is_first_attr(self, node): + """Check that attribute lookup name use first attribute variable name + (self for method, cls for classmethod and mcs for metaclass). + """ + return self._first_attrs and isinstance(node.expr, astng.Name) and \ + node.expr.name == self._first_attrs[-1] def _ancestors_to_call(klass_node, method='__init__'): """return a dictionary where keys are the list of base classes providing -- cgit v1.2.1