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.

authorSylvain Th?nault <sylvain.thenault@logilab.fr>
changeset40de65ba89b2
branchdefault
phasepublic
hiddenno
parent revision#085dad997b71 Doesn't check that overriden method signature match if it's a private method, closes #18772
child revision#21a44efb3abf Refactor _check_protected_attribute_access by extracting independent part in utils functions. Improve is_super_call docstring.
files modified by this revision
ChangeLog
checkers/classes.py
test/input/func_class_access_protected_members.py
test/messages/func_class_access_protected_members.txt
# HG changeset patch
# User Sylvain Thénault <sylvain.thenault@logilab.fr>
# Date 1339073643 -7200
# Thu Jun 07 14:54:03 2012 +0200
# Node ID 40de65ba89b2179f6eef826197f4adde6edb69e0
# Parent 085dad997b71fc45e65cd5591d9482d5817e4210
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.

diff --git a/ChangeLog b/ChangeLog
@@ -1,11 +1,14 @@
1  ChangeLog for PyLint
2  ====================
3 
4  	--
5 
6 -    * #18772 no prototype consistency check for mangled methods (patch by
7 +    * #7394: W0212 (access to protected member) not emited on assigments
8 +    (patch by lothiraldan@gmail.com)
9 +
10 +    * #18772; no prototype consistency check for mangled methods (patch by
11      lothiraldan@gmail.com)
12 
13      * #92911: emit W0102 when sets are used as default arguments in functions
14        (patch by tmarek@google.com)
15 
diff --git a/checkers/classes.py b/checkers/classes.py
@@ -16,11 +16,11 @@
16  """classes checker for Python code
17  """
18  from __future__ import generators
19 
20  from logilab import astng
21 -from logilab.astng import YES, Instance, are_exclusive
22 +from logilab.astng import YES, Instance, are_exclusive, AssAttr
23 
24  from pylint.interfaces import IASTNGChecker
25  from pylint.checkers import BaseChecker
26  from pylint.checkers.utils import (PYMETHODS, overrides_a_method,
27      check_messages, is_attr_private)
@@ -298,36 +298,70 @@
28          if so, register it. Also check for access to protected
29          class member from outside its class (but ignore __special__
30          methods)
31          """
32          attrname = node.attrname
33 -        if self._first_attrs and isinstance(node.expr, astng.Name) and \
34 -               node.expr.name == self._first_attrs[-1]:
35 +        # Check self
36 +        if self.is_first_attr(node):
37              self._accessed[-1].setdefault(attrname, []).append(node)
38              return
39          if 'W0212' not in self.active_msgs:
40              return
41 +
42 +        self._check_protected_attribute_access(node)
43 +
44 +    def visit_assign(self, assign_node):
45 +        if 'W0212' not in self.active_msgs:
46 +            return
47 +
48 +        node = assign_node.targets[0]
49 +        if not isinstance(node, AssAttr):
50 +            return
51 +
52 +        if self.is_first_attr(node):
53 +            return
54 +
55 +        self._check_protected_attribute_access(node)
56 +
57 +    def _check_protected_attribute_access(self, node):
58 +        '''Given an attribute access node (set or get), check if attribute
59 +        access is legitimate. Call _check_first_attr with node before calling
60 +        this method. Valid cases are:
61 +        * self._attr in a method or cls._attr in a classmethod. Checked by
62 +        _check_first_attr.
63 +        * Klass._attr inside "Klass" class.
64 +        * Klass2._attr inside "Klass" class when Klass2 is a base class of
65 +            Klass.
66 +        '''
67 +        attrname = node.attrname
68 +
69          if attrname[0] == '_' and not attrname == '_' and not (
70 -             attrname.startswith('__') and attrname.endswith('__')):
71 -            # XXX move this in a reusable function
72 +                attrname.startswith('__') and attrname.endswith('__')):
73 +
74              klass = node.frame()
75 +
76              while klass is not None and not isinstance(klass, astng.Class):
77                  if klass.parent is None:
78                      klass = None
79                  else:
80                      klass = klass.parent.frame()
81 +
82              # XXX infer to be more safe and less dirty ??
83              # in classes, check we are not getting a parent method
84              # through the class object or through super
85              callee = node.expr.as_string()
86 -            if klass is None or not (callee == klass.name or
87 -                callee in klass.basenames
88 -                or (isinstance(node.expr, astng.CallFunc)
89 -                    and isinstance(node.expr.func, astng.Name)
90 -                    and node.expr.func.name == 'super')):
91 +
92 +            # We are not in a class, no remaining valid case
93 +            if klass is None:
94                  self.add_message('W0212', node=node, args=attrname)
95 +                return
96 
97 +            # We are in a class, one remaining valid cases, Klass._attr inside
98 +            # Klass
99 +            if not (callee == klass.name or callee in klass.basenames):
100 +
101 +                self.add_message('W0212', node=node, args=attrname)
102 
103      def visit_name(self, node):
104          """check if the name handle an access to a class member
105          if so, register it
106          """
@@ -535,10 +569,16 @@
107          if len(method1.args.args) != len(refmethod.args.args):
108              self.add_message('W0221', args=class_type, node=method1)
109          elif len(method1.args.defaults) < len(refmethod.args.defaults):
110              self.add_message('W0222', args=class_type, node=method1)
111 
112 +    def is_first_attr(self, node):
113 +        """Check that attribute lookup name use first attribute variable name
114 +        (self for method, cls for classmethod and mcs for metaclass).
115 +        """
116 +        return self._first_attrs and isinstance(node.expr, astng.Name) and \
117 +                   node.expr.name == self._first_attrs[-1]
118 
119  def _ancestors_to_call(klass_node, method='__init__'):
120      """return a dictionary where keys are the list of base classes providing
121      the queried method, and so that should/may be called from the method node
122      """
diff --git a/test/input/func_class_access_protected_members.py b/test/input/func_class_access_protected_members.py
@@ -1,30 +1,36 @@
123 -# pylint: disable=R0903
124 +# pylint: disable=R0903, C0111, W0231
125  """test external access to protected class members"""
126 
127  __revision__ = ''
128 
129  class MyClass:
130 -    """class docstring"""
131      _cls_protected = 5
132 -    
133 +
134      def __init__(self, other):
135 -        """init"""
136 +        MyClass._cls_protected = 6
137          self._protected = 1
138          self.public = other
139 -        
140 -        
141 +        self.attr = 0
142 +
143      def test(self):
144 -        """test"""
145          self._protected += self._cls_protected
146          print self.public._haha
147 -        
148 +
149      def clsmeth(cls):
150 -        """this is ok"""
151 +        cls._cls_protected += 1
152          print cls._cls_protected
153      clsmeth = classmethod(clsmeth)
154 -    
155 +
156 +class Subclass(MyClass):
157 +
158 +    def __init__(self):
159 +        MyClass._protected = 5
160 +
161  INST = MyClass()
162 -print INST.public
163 +INST.attr = 1
164 +print INST.attr
165 +INST._protected = 2
166  print INST._protected
167 +INST._cls_protected = 3
168  print INST._cls_protected
169 
diff --git a/test/messages/func_class_access_protected_members.txt b/test/messages/func_class_access_protected_members.txt
@@ -1,3 +1,5 @@
170 -W: 19:MyClass.test: Access to a protected member _haha of a client class
171 -W: 28: Access to a protected member _protected of a client class
172 -W: 29: Access to a protected member _cls_protected of a client class
173 +W: 17:MyClass.test: Access to a protected member _haha of a client class
174 +W: 32: Access to a protected member _protected of a client class
175 +W: 33: Access to a protected member _protected of a client class
176 +W: 34: Access to a protected member _cls_protected of a client class
177 +W: 35: Access to a protected member _cls_protected of a client class
178 \ No newline at end of file