Anonymous avatar Anonymous committed 40de65b

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.

Comments (0)

Files changed (4)

 
 	--
 
-    * #18772 no prototype consistency check for mangled methods (patch by
+    * #7394: W0212 (access to protected member) not emited on assigments
+    (patch by lothiraldan@gmail.com)
+
+    * #18772; no prototype consistency check for mangled methods (patch by
     lothiraldan@gmail.com)
 
     * #92911: emit W0102 when sets are used as default arguments in functions

checkers/classes.py

 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
         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
         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

test/input/func_class_access_protected_members.py

-# pylint: disable=R0903
+# pylint: disable=R0903, C0111, W0231
 """test external access to protected class members"""
 
 __revision__ = ''
 
 class MyClass:
-    """class docstring"""
     _cls_protected = 5
-    
+
     def __init__(self, other):
-        """init"""
+        MyClass._cls_protected = 6
         self._protected = 1
         self.public = other
-        
-        
+        self.attr = 0
+
     def test(self):
-        """test"""
         self._protected += self._cls_protected
         print self.public._haha
-        
+
     def clsmeth(cls):
-        """this is ok"""
+        cls._cls_protected += 1
         print cls._cls_protected
     clsmeth = classmethod(clsmeth)
-    
+
+class Subclass(MyClass):
+
+    def __init__(self):
+        MyClass._protected = 5
+
 INST = MyClass()
-print INST.public
+INST.attr = 1
+print INST.attr
+INST._protected = 2
 print INST._protected
+INST._cls_protected = 3
 print INST._cls_protected
 

test/messages/func_class_access_protected_members.txt

-W: 19:MyClass.test: Access to a protected member _haha of a client class
-W: 28: Access to a protected member _protected of a client class
-W: 29: Access to a protected member _cls_protected of a client class
+W: 17:MyClass.test: Access to a protected member _haha of a client class
+W: 32: Access to a protected member _protected of a client class
+W: 33: Access to a protected member _protected of a client class
+W: 34: Access to a protected member _cls_protected of a client class
+W: 35: Access to a protected member _cls_protected of a client class
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.