From 438151f56329c016f0c329c6ef9f6023da1753cc Mon Sep 17 00:00:00 2001 From: Manolis Stamatogiannakis Date: Tue, 12 Dec 2023 17:46:14 +0100 Subject: [PATCH] Supress some protected-access warnings when django-simple-history is 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 --- pylint_django/augmentations/__init__.py | 26 +++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pylint_django/augmentations/__init__.py b/pylint_django/augmentations/__init__.py index 89f454f4..c6d76c7f 100644 --- a/pylint_django/augmentations/__init__.py +++ b/pylint_django/augmentations/__init__.py @@ -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 @@ -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 for 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 @@ -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(