Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow protected-access in class methods #4267

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -464,3 +464,5 @@ contributors:
* Konstantina Saketou: contributor

* Andrew Howe: contributor

* James Sinclair (irgeek): contributor
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -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``


Expand Down
2 changes: 2 additions & 0 deletions doc/whatsnew/2.7.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
46 changes: 46 additions & 0 deletions pylint/checkers/classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
112 changes: 112 additions & 0 deletions tests/functional/a/access/access_to_protected_members.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
7 changes: 7 additions & 0 deletions tests/functional/a/access/access_to_protected_members.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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