request: Remove ExtractRecipients type safety hole on REQ.

It was allowing us to get away with wrong types on a few functions:
`check_send_typing_notification` and `send_notification_backend` can be
(and are) called with a list of `int` as `notification_to`, not just a
list of `str`.

The problem it was working around already had a better solution using
the dummy `type` argument.  Use that.

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

zerver/lib/actions.py

@@ -1682,7 +1682,7 @@ def do_send_typing_notification(realm: Realm, notification: Dict[str, Any]) -> N
 
 # check_send_typing_notification:
 # Checks the typing notification and sends it
-def check_send_typing_notification(sender: UserProfile, notification_to: Sequence[str],
+def check_send_typing_notification(sender: UserProfile, notification_to: Union[Sequence[str], Sequence[int]],
                                    operator: str) -> None:
     typing_notification = check_typing_notification(sender, notification_to, operator)
     do_send_typing_notification(sender.realm, typing_notification)

zerver/lib/request.pyi

@@ -6,8 +6,8 @@
 # scan the parameter list for REQ objects and patch the parameters as the true
 # types.
 
-from typing import Any, Dict, Callable, List, TypeVar, Optional, Union, Type
-from zerver.lib.types import ViewFuncT, Validator, ExtractRecipients
+from typing import Dict, Callable, List, TypeVar, Optional, Union, Type
+from zerver.lib.types import ViewFuncT, Validator
 from zerver.lib.exceptions import JsonableError as JsonableError
 
 ResultT = TypeVar('ResultT')
@@ -22,8 +22,8 @@ NotSpecified = _NotSpecified()
 def REQ(whence: Optional[str] = None,
         *,
         type: Type[ResultT] = Type[None],
-        converter: Union[Optional[Callable[[str], ResultT]], ExtractRecipients] = None,
-        default: Union[_NotSpecified, ResultT, None] = Optional[NotSpecified],
+        converter: Optional[Callable[[str], ResultT]] = None,
+        default: Union[_NotSpecified, ResultT, None] = NotSpecified,
         validator: Optional[Validator] = None,
         str_validator: Optional[Validator] = None,
         argument_type: Optional[str] = None,

zerver/lib/types.py

@@ -1,4 +1,4 @@
-from typing import TypeVar, Callable, Optional, List, Dict, Union, Tuple, Any, Iterable
+from typing import TypeVar, Callable, Optional, List, Dict, Union, Tuple, Any
 from typing_extensions import TypedDict
 from django.http import HttpResponse
 
@@ -7,12 +7,6 @@ ViewFuncT = TypeVar('ViewFuncT', bound=Callable[..., HttpResponse])
 # See zerver/lib/validator.py for more details of Validators,
 # including many examples
 Validator = Callable[[str, object], Optional[str]]
-
-# This type is specific for zerver.lib.actions.extract_recipients. After
-# deciding to support IDs for some of our API, extract_recipients'
-# implementation diverged from other converter functions in many ways.
-# See zerver/lib/request.pyi to see how this is used.
-ExtractRecipients = Callable[[Union[str, Iterable[str], Iterable[int]]], Union[List[str], List[int]]]
 ExtendedValidator = Callable[[str, str, object], Optional[str]]
 RealmUserValidator = Callable[[int, List[int], bool], Optional[str]]
 

zerver/views/messages.py

@@ -1254,7 +1254,8 @@ def handle_deferred_message(sender: UserProfile, client: Client,
 def send_message_backend(request: HttpRequest, user_profile: UserProfile,
                          message_type_name: str=REQ('type'),
                          message_to: Union[Sequence[int], Sequence[str]]=REQ(
-                             'to', converter=extract_recipients, default=[]),
+                             'to', type=Union[List[int], List[str]],
+                             converter=extract_recipients, default=[]),
                          forged: bool=REQ(default=False,
                                           documentation_pending=True),
                          topic_name: Optional[str]=REQ_topic(),

zerver/views/typing.py

@@ -1,5 +1,5 @@
 from django.http import HttpRequest, HttpResponse
-from typing import List
+from typing import List, Union
 
 from zerver.decorator import has_request_variables, REQ
 from zerver.lib.actions import check_send_typing_notification, \
@@ -11,7 +11,8 @@ from zerver.models import UserProfile
 def send_notification_backend(
         request: HttpRequest, user_profile: UserProfile,
         operator: str=REQ('op'),
-        notification_to: List[str]=REQ('to', converter=extract_recipients, default=[]),
+        notification_to: Union[List[str], List[int]]=REQ(
+            'to', type=Union[List[str], List[int]], converter=extract_recipients, default=[]),
 ) -> HttpResponse:
     check_send_typing_notification(user_profile, notification_to, operator)
     return json_success()