python: Clean up getattr, setattr, delattr calls with literal names.

These were useful as a transitional workaround to ignore type errors
that only show up with django-stubs, while avoiding errors about
unused type: ignore comments without django-stubs.  Now that the
django-stubs transition is complete, switch to type: ignore comments
so that mypy will tell us if they become unnecessary.  Many already
have.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

zerver/decorator.py

@@ -368,7 +368,7 @@ def webhook_view(
 
         # Store the event types registered for this webhook as an attribute, which can be access
         # later conveniently in zerver.lib.test_classes.WebhookTestCase.
-        setattr(_wrapped_func_arguments, "_all_event_types", all_event_types)
+        _wrapped_func_arguments._all_event_types = all_event_types  # type: ignore[attr-defined] # custom attribute
         return _wrapped_func_arguments
 
     return _wrapped_view_func
@@ -793,7 +793,7 @@ def process_as_post(
         if not request.POST:
             # Only take action if POST is empty.
             if request.content_type == "multipart/form-data":
-                POST, _files = MultiPartParser(
+                POST, files = MultiPartParser(
                     request.META,
                     BytesIO(request.body),
                     request.upload_handlers,
@@ -809,10 +809,7 @@ def process_as_post(
                 # See also: https://github.com/typeddjango/django-stubs/pull/925#issue-1206399444
                 POST._mutable = False
                 request.POST = cast("_ImmutableQueryDict", POST)
-                # Note that request._files is just the private attribute that backs the
+                request.FILES.update(files)
-                # FILES property, so we are essentially setting request.FILES here.  (In
-                # Django 3.2 FILES was still a read-only property.)
-                setattr(request, "_files", _files)
             elif request.content_type == "application/x-www-form-urlencoded":
                 request.POST = QueryDict(request.body, encoding=request.encoding)
 

zerver/lib/create_user.py

@@ -43,8 +43,8 @@ def copy_default_settings(
         target_profile.save()
         return
 
-    setattr(target_profile, "full_name", settings_source.full_name)
+    target_profile.full_name = settings_source.full_name
-    setattr(target_profile, "timezone", canonicalize_timezone(settings_source.timezone))
+    target_profile.timezone = canonicalize_timezone(settings_source.timezone)
     target_profile.save()
 
     if settings_source.avatar_source == UserProfile.AVATAR_FROM_USER:

zerver/lib/import_realm.py

@@ -892,7 +892,7 @@ def import_uploads(
                 process_avatars(record)
         else:
             connection.close()
-            _cache = getattr(cache, "_cache")
+            _cache = cache._cache  # type: ignore[attr-defined] # not in stubs
             assert isinstance(_cache, bmemcached.Client)
             _cache.disconnect_all()
             with ProcessPoolExecutor(max_workers=processes) as executor:

zerver/lib/logging_util.py

@@ -197,12 +197,10 @@ class ZulipFormatter(logging.Formatter):
         return " ".join(pieces)
 
     def format(self, record: logging.LogRecord) -> str:
-        if not getattr(record, "zulip_decorated", False):
+        if not hasattr(record, "zulip_decorated"):
-            # The `setattr` calls put this logic explicitly outside the bounds of the
+            record.zulip_level_abbrev = abbrev_log_levelname(record.levelname)
-            # type system; otherwise mypy would complain LogRecord lacks these attributes.
+            record.zulip_origin = find_log_origin(record)
-            setattr(record, "zulip_level_abbrev", abbrev_log_levelname(record.levelname))
+            record.zulip_decorated = True
-            setattr(record, "zulip_origin", find_log_origin(record))
-            setattr(record, "zulip_decorated", True)
         return super().format(record)
 
 
@@ -227,12 +225,12 @@ class ZulipWebhookFormatter(ZulipFormatter):
 
         request = get_current_request()
         if not request:
-            setattr(record, "user", None)
+            record.user = None
-            setattr(record, "client", None)
+            record.client = None
-            setattr(record, "url", None)
+            record.url = None
-            setattr(record, "content_type", None)
+            record.content_type = None
-            setattr(record, "custom_headers", None)
+            record.custom_headers = None
-            setattr(record, "payload", None)
+            record.payload = None
             return super().format(record)
 
         if request.content_type == "application/json":
@@ -257,12 +255,12 @@ class ZulipWebhookFormatter(ZulipFormatter):
         assert client is not None
 
         assert request.user.is_authenticated
-        setattr(record, "user", f"{request.user.delivery_email} ({request.user.realm.string_id})")
+        record.user = f"{request.user.delivery_email} ({request.user.realm.string_id})"
-        setattr(record, "client", client.name)
+        record.client = client.name
-        setattr(record, "url", request.META.get("PATH_INFO", None))
+        record.url = request.META.get("PATH_INFO", None)
-        setattr(record, "content_type", request.content_type)
+        record.content_type = request.content_type
-        setattr(record, "custom_headers", header_text or None)
+        record.custom_headers = header_text or None
-        setattr(record, "payload", payload)
+        record.payload = payload
         return super().format(record)
 
 

zerver/lib/request.py

@@ -474,9 +474,9 @@ def get_current_request() -> Optional[HttpRequest]:
 
 
 def set_request(req: HttpRequest) -> None:
-    setattr(local, "request", req)
+    local.request = req
 
 
 def unset_request() -> None:
     if hasattr(local, "request"):
-        delattr(local, "request")
+        del local.request

zerver/lib/test_helpers.py

@@ -394,7 +394,7 @@ def instrument_url(f: UrlFuncT) -> UrlFuncT:
                 info = "<bytes>"
             elif isinstance(info, dict):
                 info = {
-                    k: "<file object>" if hasattr(v, "read") and callable(getattr(v, "read")) else v
+                    k: "<file object>" if hasattr(v, "read") and callable(v.read) else v
                     for k, v in info.items()
                 }
 

zerver/lib/transfer.py

@@ -41,7 +41,7 @@ def transfer_avatars_to_s3(processes: int) -> None:
             _transfer_avatar_to_s3(user)
     else:  # nocoverage
         connection.close()
-        _cache = getattr(cache, "_cache")
+        _cache = cache._cache  # type: ignore[attr-defined] # not in stubs
         assert isinstance(_cache, bmemcached.Client)
         _cache.disconnect_all()
         with ProcessPoolExecutor(max_workers=processes) as executor:
@@ -76,7 +76,7 @@ def transfer_message_files_to_s3(processes: int) -> None:
             _transfer_message_files_to_s3(attachment)
     else:  # nocoverage
         connection.close()
-        _cache = getattr(cache, "_cache")
+        _cache = cache._cache  # type: ignore[attr-defined] # not in stubs
         assert isinstance(_cache, bmemcached.Client)
         _cache.disconnect_all()
         with ProcessPoolExecutor(max_workers=processes) as executor:
@@ -111,7 +111,7 @@ def transfer_emoji_to_s3(processes: int) -> None:
             _transfer_emoji_to_s3(realm_emoji)
     else:  # nocoverage
         connection.close()
-        _cache = getattr(cache, "_cache")
+        _cache = cache._cache  # type: ignore[attr-defined] # not in stubs
         assert isinstance(_cache, bmemcached.Client)
         _cache.disconnect_all()
         with ProcessPoolExecutor(max_workers=processes) as executor:

zerver/lib/webhooks/common.py

@@ -192,7 +192,7 @@ def get_fixture_http_headers(integration_name: str, fixture_name: str) -> Dict["
         # TODO: We may want to migrate to a more explicit registration
         # strategy for this behavior rather than a try/except import.
         view_module = importlib.import_module(view_module_name)
-        fixture_to_headers = getattr(view_module, "fixture_to_headers")
+        fixture_to_headers = view_module.fixture_to_headers
     except (ImportError, AttributeError):
         return {}
     return fixture_to_headers(fixture_name)

zerver/management/commands/makemessages.py

@@ -228,9 +228,8 @@ class Command(makemessages.Command):
         process_all = self.frontend_all
 
         # After calling super().handle(), default_locale_path gets set on self
-        # so that we can reuse it here. We have to use getattr it to access it without
+        # so that we can reuse it here.
-        # getting an attribute error from mypy.
+        default_locale_path = self.default_locale_path  # type: ignore[attr-defined] # not in stubs
-        default_locale_path = getattr(self, "default_locale_path")
         paths = glob.glob(f"{default_locale_path}/*")
         all_locales = [os.path.basename(path) for path in paths if os.path.isdir(path)]
 

zerver/middleware.py

@@ -774,7 +774,7 @@ class ZulipSCIMAuthCheckMiddleware(SCIMAuthCheckMiddleware):
         # so we can assign the corresponding SCIMClient object to request.user - which
         # will allow this request to pass request.user.is_authenticated checks from now on,
         # to be served by the relevant views implemented in django-scim2.
-        # Since request.user must be a UserProfile or AnonymousUser, setattr is a type-unsafe
+        # Since request.user must be a UserProfile or AnonymousUser, this is a type-unsafe
         # workaround to make this monkey-patching work.
-        setattr(request, "user", scim_client)
+        request.user = scim_client  # type: ignore[assignment] # wrong type
         return None

zerver/models.py

@@ -3245,22 +3245,20 @@ class AbstractUserMessage(models.Model):
 
     @staticmethod
     def where_unread() -> str:
-        return AbstractUserMessage.where_flag_is_absent(getattr(AbstractUserMessage.flags, "read"))
+        return AbstractUserMessage.where_flag_is_absent(AbstractUserMessage.flags.read)
 
     @staticmethod
     def where_read() -> str:
-        return AbstractUserMessage.where_flag_is_present(getattr(AbstractUserMessage.flags, "read"))
+        return AbstractUserMessage.where_flag_is_present(AbstractUserMessage.flags.read)
 
     @staticmethod
     def where_starred() -> str:
-        return AbstractUserMessage.where_flag_is_present(
+        return AbstractUserMessage.where_flag_is_present(AbstractUserMessage.flags.starred)
-            getattr(AbstractUserMessage.flags, "starred")
-        )
 
     @staticmethod
     def where_active_push_notification() -> str:
         return AbstractUserMessage.where_flag_is_present(
-            getattr(AbstractUserMessage.flags, "active_mobile_push_notification")
+            AbstractUserMessage.flags.active_mobile_push_notification
         )
 
     def flags_list(self) -> List[str]:

zerver/tests/test_link_embed.py

@@ -542,7 +542,7 @@ class PreviewTestCase(ZulipTestCase):
         self.assertEqual(msg.rendered_content, with_preview)
 
         realm = msg.get_realm()
-        setattr(realm, "inline_url_embed_preview", False)
+        realm.inline_url_embed_preview = False
         realm.save()
 
         msg = self._send_message_with_test_org_url(

zerver/tests/test_logging_handlers.py

@@ -240,7 +240,7 @@ class AdminNotifyHandlerTest(ZulipTestCase):
         self.assertEqual(report["stack_trace"], "No stack trace available")
 
         # Test arbitrary exceptions from request.user
-        delattr(record.request, "user")
+        del record.request.user
         with patch("zerver.logging_handlers.traceback.print_exc"):
             report = self.run_handler(record)
         self.assertIn("host", report)

zerver/tests/test_markdown.py

@@ -623,7 +623,7 @@ class MarkdownTest(ZulipTestCase):
         self.assertEqual(converted.rendered_content, with_preview)
 
         realm = msg.get_realm()
-        setattr(realm, "inline_image_preview", False)
+        realm.inline_image_preview = False
         realm.save()
 
         sender_user_profile = self.example_user("othello")

zerver/tests/test_openapi.py

@@ -422,8 +422,8 @@ do not match the types declared in the implementation of {function.__name__}.\n"
         # Iterate through the decorators to find the original
         # function, wrapped by has_request_variables, so we can parse
         # its arguments.
-        while hasattr(function, "__wrapped__"):
+        while (wrapped := getattr(function, "__wrapped__", None)) is not None:
-            function = getattr(function, "__wrapped__")
+            function = wrapped
 
         # Now, we do inference mapping each REQ parameter's
         # declaration details to the Python/mypy types for the

zerver/tests/test_signup.py

@@ -12,7 +12,8 @@ from django.contrib.auth.views import PasswordResetConfirmView
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ValidationError
 from django.core.mail.message import EmailMultiAlternatives
-from django.http import HttpRequest, HttpResponse
+from django.http import HttpRequest, HttpResponse, HttpResponseBase
+from django.template.response import TemplateResponse
 from django.test import Client, override_settings
 from django.urls import reverse
 from django.utils import translation
@@ -198,7 +199,7 @@ class DeactivationNoticeTestCase(ZulipTestCase):
         realm.save(update_fields=["deactivated"])
 
         result = self.client_get("/login/", follow=True)
-        self.assertEqual(getattr(result, "redirect_chain")[-1], ("/accounts/deactivated/", 302))
+        self.assertEqual(result.redirect_chain[-1], ("/accounts/deactivated/", 302))
         self.assertIn("Zulip Dev, has been deactivated.", result.content.decode())
         self.assertNotIn("It has moved to", result.content.decode())
 
@@ -1080,10 +1081,12 @@ class LoginTest(ZulipTestCase):
         description = "https://www.google.com/images/srpr/logo4w.png"
         realm.description = description
         realm.save(update_fields=["description"])
-        response = self.client_get("/login/")
+        response: HttpResponseBase = self.client_get("/login/")
         expected_response = """<p><a href="https://www.google.com/images/srpr/logo4w.png">\
 https://www.google.com/images/srpr/logo4w.png</a></p>"""
-        self.assertEqual(getattr(response, "context_data")["realm_description"], expected_response)
+        assert isinstance(response, TemplateResponse)
+        assert response.context_data is not None
+        self.assertEqual(response.context_data["realm_description"], expected_response)
         self.assertEqual(response.status_code, 200)
 
 

zilencer/management/commands/mark_all_messages_unread.py

@@ -15,6 +15,6 @@ class Command(BaseCommand):
     def handle(self, *args: Any, **options: Any) -> None:
         assert settings.DEVELOPMENT
         UserMessage.objects.all().update(flags=F("flags").bitand(~UserMessage.flags.read))
-        _cache = getattr(cache, "_cache")
+        _cache = cache._cache  # type: ignore[attr-defined] # not in stubs
         assert isinstance(_cache, bmemcached.Client)
         _cache.flush_all()