Skip to content

Commit

Permalink
Supress some protected-access warnings when django-simple-history is …
Browse files Browse the repository at this point in the history
…installed.

django-simple-hisory is a fairly popular [1] package for keeping track
of changes in django objects. Setting the `_change_reason` property of
an object is the officially documented way to provide a value for the
`history_change_reason` field of historical objects [2].

When django-simple-hisory is installed, protected-access warnings for
setting `_change_reason` is most likely a false positive, and should be
supressed.

Because of inherent limitations of pylint, this may lead to some false
negatives if `_change_reason` is used elsewhere.

[1] https://pypistats.org/packages/django-simple-history
[2] https://django-simple-history.readthedocs.io/en/latest/historical_model.html#change-reason
  • Loading branch information
m000 committed Dec 12, 2023
1 parent 955f779 commit 655ebf2
Showing 1 changed file with 26 additions and 0 deletions.
26 changes: 26 additions & 0 deletions pylint_django/augmentations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from astroid.nodes.scoped_nodes import Module
from astroid.objects import Super
from django import VERSION as django_version
from django.conf import settings
from django.utils import termcolors
from django.views.generic.base import ContextMixin, RedirectView, View
from django.views.generic.dates import DateMixin, DayMixin, MonthMixin, WeekMixin, YearMixin
Expand Down Expand Up @@ -752,6 +753,23 @@ def allow_meta_protected_access(node):
return False


def allow_simple_history_protected_access(assign_node):
# NOTE: Only consider the first assignment target, mirroring current pylint behavior.
# See pylint ClassChecker::visit_assign().
# Because the type of assign_target typically cannot be inferred, this will suppress
# the warning even in assignments on objects not related to simple_history.
assign_target = assign_node.targets[0]
assign_target_attrname = getattr(assign_target, "attrname", None)

if assign_target_attrname is None or assign_target_attrname not in ("_change_reason",):
return False

if "simple_history" not in settings.INSTALLED_APPS:
return False

return True


class IsClass: # pylint: disable=too-few-public-methods
def __init__(self, class_name):
self.class_name = class_name
Expand Down Expand Up @@ -968,6 +986,14 @@ def apply_augmentations(linter):
is_model_mpttmeta_subclass,
)

# django-simple-history
suppress_message(
linter,
ClassChecker.visit_assign,
"protected-access",
allow_simple_history_protected_access,
)

# factory_boy's DjangoModelFactory
suppress_message(linter, TypeChecker.visit_attribute, "no-member", is_model_factory)
suppress_message(
Expand Down

0 comments on commit 655ebf2

Please sign in to comment.