Allow protected-access in class methods (#4267)

* Add failing tests for protected member access in class methods.
* Allow protected-access to class/instance members inside class methods.

Co-authored-by: Pierre Sassoulas <[email protected]>

CONTRIBUTORS.txt

@@ -464,3 +464,5 @@ contributors:
 * Konstantina Saketou: contributor
 
 * Andrew Howe: contributor
+
+* James Sinclair (irgeek): contributor

ChangeLog

@@ -17,6 +17,10 @@ Release date: Undefined
 ..
   Put bug fixes that will be cherry-picked to latest major version here
 
+* Improved protected access checks to allow access inside class methods
+
+  Closes #1159
+
 * Fix issue with PEP 585 syntax and the use of ``collections.abc.Set``
 
 * Fix issue that caused class variables annotated with ``typing.ClassVar`` to be

doc/whatsnew/2.7.rst

@@ -56,3 +56,5 @@ Other Changes
 * Fixes duplicate code detection for --jobs=2+
 
 * New option ``allowed-redefined-builtins`` defines variable names allowed to shadow builtins.
+
+* Improved protected access checks to allow access inside class methods

pylint/checkers/classes.py

@@ -1426,6 +1426,14 @@ def _check_protected_attribute_access(self, node):
                     name = stmt.targets[0].name
                     if _is_attribute_property(name, klass):
                         return
+
+                if (
+                    self._is_classmethod(node.frame())
+                    and self._is_inferred_instance(node.expr, klass)
+                    and self._is_class_attribute(attrname, klass)
+                ):
+                    return
+
                 licit_protected_member = not attrname.startswith("__")
                 if (
                     not self.config.check_protected_access_in_special_methods
@@ -1456,6 +1464,44 @@ def _is_type_self_call(self, expr):
             and self._is_mandatory_method_param(expr.args[0])
         )
 
+    @staticmethod
+    def _is_classmethod(func):
+        """Check if the given *func* node is a class method."""
+
+        return isinstance(func, astroid.FunctionDef) and (
+            func.type == "classmethod" or func.name == "__class_getitem__"
+        )
+
+    @staticmethod
+    def _is_inferred_instance(expr, klass):
+        """Check if the inferred value of the given *expr* is an instance of *klass*."""
+
+        inferred = safe_infer(expr)
+        if not isinstance(inferred, astroid.Instance):
+            return False
+
+        return inferred._proxied is klass
+
+    @staticmethod
+    def _is_class_attribute(name, klass):
+        """Check if the given attribute *name* is a class or instance member of the given *klass*.
+
+        Returns ``True`` if the name is a property in the given klass,
+        ``False`` otherwise.
+        """
+
+        try:
+            klass.getattr(name)
+            return True
+        except astroid.NotFoundError:
+            pass
+
+        try:
+            klass.instance_attr(name)
+            return True
+        except astroid.NotFoundError:
+            return False
+
     def visit_name(self, node):
         """check if the name handle an access to a class member
         if so, register it

tests/functional/a/access/access_to_protected_members.py

@@ -99,3 +99,115 @@ def __fake_special__(self, other):
         if isinstance(other, self.__class__):
             return self._foo == other._foo  # [protected-access]
         return False
+
+
+class Issue1159OtherClass(object):
+    """Test for GitHub issue 1159"""
+
+    _foo = 0
+
+    def __init__(self):
+        self._bar = 0
+
+
+class Issue1159(object):
+    """Test for GitHub issue 1159"""
+
+    _foo = 0
+
+    def __init__(self):
+        self._bar = 0
+
+    @classmethod
+    def access_cls_attr(cls):
+        """
+        Access to protected class members inside class methods is OK.
+        """
+
+        _ = cls._foo
+
+    @classmethod
+    def assign_cls_attr(cls):
+        """
+        Assignment to protected class members inside class methods is OK.
+        """
+
+        cls._foo = 1
+
+    @classmethod
+    def access_inst_attr(cls):
+        """
+        Access to protected instance members inside class methods is OK.
+        """
+
+        instance = cls()
+        _ = instance._bar
+
+    @classmethod
+    def assign_inst_attr(cls):
+        """
+        Assignment to protected members inside class methods is OK.
+        """
+
+        instance = cls()
+        instance._bar = 1
+
+    @classmethod
+    def access_other_attr(cls):
+        """
+        Access to protected instance members of other classes is not OK.
+        """
+
+        instance = Issue1159OtherClass()
+        instance._bar = 3  # [protected-access]
+        _ = instance._foo  # [protected-access]
+
+
+class Issue1159Subclass(Issue1159):
+    """Test for GitHub issue 1159"""
+
+    @classmethod
+    def access_inst_attr(cls):
+        """
+        Access to protected instance members inside class methods is OK.
+        """
+
+        instance = cls()
+        _ = instance._bar
+
+    @classmethod
+    def assign_inst_attr(cls):
+        """
+        Assignment to protected instance members inside class methods is OK.
+        """
+
+        instance = cls()
+        instance._bar = 1
+
+    @classmethod
+    def access_missing_member(cls):
+        """
+        Access to unassigned members inside class methods is not OK.
+        """
+
+        instance = cls()
+        _ = instance._baz  # [no-member,protected-access]
+
+    @classmethod
+    def assign_missing_member(cls):
+        """
+        Defining attributes outside init is still not OK.
+        """
+
+        instance = cls()
+        instance._qux = 1  # [attribute-defined-outside-init]
+
+    @classmethod
+    def access_other_attr(cls):
+        """
+        Access to protected instance members of other classes is not OK.
+        """
+
+        instance = Issue1159OtherClass()
+        instance._bar = 3  # [protected-access]
+        _ = instance._foo  # [protected-access]

tests/functional/a/access/access_to_protected_members.txt

@@ -7,3 +7,10 @@ protected-access:58:19:Issue1031.incorrect_access:Access to a protected member _
 protected-access:72:48:Issue1802.__eq__:Access to a protected member __private of a client class
 protected-access:80:32:Issue1802.not_in_special:Access to a protected member _foo of a client class
 protected-access:100:32:Issue1802.__fake_special__:Access to a protected member _foo of a client class
+protected-access:162:8:Issue1159.access_other_attr:Access to a protected member _bar of a client class
+protected-access:163:12:Issue1159.access_other_attr:Access to a protected member _foo of a client class
+no-member:194:12:Issue1159Subclass.access_missing_member:Instance of 'Issue1159Subclass' has no '_baz' member; maybe '_bar'?:INFERENCE
+protected-access:194:12:Issue1159Subclass.access_missing_member:Access to a protected member _baz of a client class
+attribute-defined-outside-init:203:8:Issue1159Subclass.assign_missing_member:Attribute '_qux' defined outside __init__
+protected-access:212:8:Issue1159Subclass.access_other_attr:Access to a protected member _bar of a client class
+protected-access:213:12:Issue1159Subclass.access_other_attr:Access to a protected member _foo of a client class