From bda60a1f62a8c318571ee339feb937213adced7d Mon Sep 17 00:00:00 2001 From: James Sinclair Date: Tue, 30 Mar 2021 11:23:51 +1100 Subject: [PATCH 1/2] Add failing tests for protected member access in class methods. --- .../a/access/access_to_protected_members.py | 112 ++++++++++++++++++ .../a/access/access_to_protected_members.txt | 7 ++ 2 files changed, 119 insertions(+) diff --git a/tests/functional/a/access/access_to_protected_members.py b/tests/functional/a/access/access_to_protected_members.py index 777cd5ccde..06438400a5 100644 --- a/tests/functional/a/access/access_to_protected_members.py +++ b/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] diff --git a/tests/functional/a/access/access_to_protected_members.txt b/tests/functional/a/access/access_to_protected_members.txt index f1a545b27a..1b93683db1 100644 --- a/tests/functional/a/access/access_to_protected_members.txt +++ b/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 From 013835c6e6b27f4f08998e0f6500a10e8c4703bd Mon Sep 17 00:00:00 2001 From: James Sinclair Date: Tue, 30 Mar 2021 14:34:17 +1100 Subject: [PATCH 2/2] Allow protected-access to class/instance members inside class methods. --- CONTRIBUTORS.txt | 2 ++ ChangeLog | 5 +++++ doc/whatsnew/2.7.rst | 2 ++ pylint/checkers/classes.py | 46 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 1924b31e1f..8ace48a72d 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -464,3 +464,5 @@ contributors: * Konstantina Saketou: contributor * Andrew Howe: contributor + +* James Sinclair (irgeek): contributor diff --git a/ChangeLog b/ChangeLog index 29a4233d6a..6826c837ad 100644 --- a/ChangeLog +++ b/ChangeLog @@ -18,6 +18,11 @@ 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 + + What's New in Pylint 2.7.3? diff --git a/doc/whatsnew/2.7.rst b/doc/whatsnew/2.7.rst index 6395ffd2bb..35bdb9348b 100644 --- a/doc/whatsnew/2.7.rst +++ b/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 diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 633048d045..afdbaab92d 100644 --- a/pylint/checkers/classes.py +++ b/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