From 3163b078144b76cee3a1bea0cbd86dde7963bace Mon Sep 17 00:00:00 2001
From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 24 Apr 2023 12:10:57 +0200
Subject: [PATCH] Improve static type checking (#336)

---
 changelog.d/336.misc                          |  1 +
 mypy.ini                                      |  9 ---
 sygnal/helper/context_factory.py              | 28 +++++----
 .../proxy/connectproxyclient_twisted.py       | 55 ++++++++++--------
 sygnal/helper/proxy/proxy_asyncio.py          | 20 +++----
 sygnal/helper/proxy/proxyagent_twisted.py     | 58 +++++++++++--------
 sygnal/webpushpushkin.py                      |  3 +-
 tests/test_concurrency_limit.py               |  3 +-
 tests/test_http.py                            | 12 ++--
 tests/test_pushgateway_api_v1.py              | 27 +++++----
 10 files changed, 117 insertions(+), 99 deletions(-)
 create mode 100644 changelog.d/336.misc

diff --git a/changelog.d/336.misc b/changelog.d/336.misc
new file mode 100644
index 00000000..b79e5560
--- /dev/null
+++ b/changelog.d/336.misc
@@ -0,0 +1 @@
+Improve static type checking.
\ No newline at end of file
diff --git a/mypy.ini b/mypy.ini
index 3f0a41c3..d8fb16ad 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -45,9 +45,6 @@ ignore_missing_imports = True
 [mypy-pywebpush]
 ignore_missing_imports = True
 
-[mypy-sygnal.helper.*]
-disallow_untyped_defs = False
-
 [mypy-sygnal.notifications]
 disallow_untyped_defs = False
 
@@ -60,18 +57,12 @@ disallow_untyped_defs = False
 [mypy-tests.asyncio_test_helpers]
 disallow_untyped_defs = False
 
-[mypy-tests.test_http]
-disallow_untyped_defs = False
-
 [mypy-tests.test_httpproxy_asyncio]
 disallow_untyped_defs = False
 
 [mypy-tests.test_httpproxy_twisted]
 disallow_untyped_defs = False
 
-[mypy-tests.test_pushgateway_api_v1]
-disallow_untyped_defs = False
-
 [mypy-tests.testutils]
 disallow_untyped_defs = False
 
diff --git a/sygnal/helper/context_factory.py b/sygnal/helper/context_factory.py
index 9de4a114..f0dbac28 100644
--- a/sygnal/helper/context_factory.py
+++ b/sygnal/helper/context_factory.py
@@ -26,6 +26,7 @@
 from twisted.internet.abstract import isIPAddress, isIPv6Address
 from twisted.internet.interfaces import IOpenSSLClientConnectionCreator
 from twisted.internet.ssl import CertificateOptions, TLSVersion, platformTrust
+from twisted.protocols.tls import TLSMemoryBIOProtocol
 from twisted.python.failure import Failure
 from twisted.web.iweb import IPolicyForHTTPS
 from zope.interface import implementer
@@ -43,7 +44,7 @@ class ClientTLSOptionsFactory:
     constructs an SSLClientConnectionCreator factory accordingly.
     """
 
-    def __init__(self):
+    def __init__(self) -> None:
         # Use CA root certs provided by OpenSSL
         trust_root = platformTrust()
 
@@ -61,13 +62,13 @@ def __init__(self):
         self._verify_ssl_context = self._verify_ssl.getContext()
         self._verify_ssl_context.set_info_callback(self._context_info_cb)
 
-    def get_options(self, host):
+    def get_options(self, host: bytes) -> IOpenSSLClientConnectionCreator:
         ssl_context = self._verify_ssl_context
 
         return SSLClientConnectionCreator(host, ssl_context)
 
     @staticmethod
-    def _context_info_cb(ssl_connection, where, ret):
+    def _context_info_cb(ssl_connection: SSL.Connection, where: int, ret: int) -> None:
         """The 'information callback' for our openssl context object."""
         # we assume that the app_data on the connection object has been set to
         # a TLSMemoryBIOProtocol object. (This is done by SSLClientConnectionCreator)
@@ -83,7 +84,9 @@ def _context_info_cb(ssl_connection, where, ret):
             f = Failure()
             tls_protocol.failVerification(f)
 
-    def creatorForNetloc(self, hostname, port):
+    def creatorForNetloc(
+        self, hostname: bytes, port: int
+    ) -> IOpenSSLClientConnectionCreator:
         """Implements the IPolicyForHTTPS interace so that this can be passed
         directly to agents.
         """
@@ -97,11 +100,13 @@ class SSLClientConnectionCreator:
     Replaces twisted.internet.ssl.ClientTLSOptions
     """
 
-    def __init__(self, hostname, ctx):
+    def __init__(self, hostname: bytes, ctx: SSL.Context):
         self._ctx = ctx
         self._verifier = ConnectionVerifier(hostname)
 
-    def clientConnectionForTLS(self, tls_protocol):
+    def clientConnectionForTLS(
+        self, tls_protocol: TLSMemoryBIOProtocol
+    ) -> SSL.Connection:
         context = self._ctx
         connection = SSL.Connection(context, None)
 
@@ -125,9 +130,10 @@ class ConnectionVerifier:
 
     # This code is based on twisted.internet.ssl.ClientTLSOptions.
 
-    def __init__(self, hostname):
-        if isIPAddress(hostname) or isIPv6Address(hostname):
-            self._hostnameBytes = hostname.encode("ascii")
+    def __init__(self, hostname: bytes):
+        _decoded = hostname.decode("ascii")
+        if isIPAddress(_decoded) or isIPv6Address(_decoded):
+            self._hostnameBytes = hostname
             self._is_ip_address = True
         else:
             # twisted's ClientTLSOptions falls back to the stdlib impl here if
@@ -140,7 +146,9 @@ def __init__(self, hostname):
 
         self._hostnameASCII = self._hostnameBytes.decode("ascii")
 
-    def verify_context_info_cb(self, ssl_connection, where):
+    def verify_context_info_cb(
+        self, ssl_connection: SSL.Connection, where: int
+    ) -> None:
         if where & SSL.SSL_CB_HANDSHAKE_START and not self._is_ip_address:
             ssl_connection.set_tlsext_host_name(self._hostnameBytes)
 
diff --git a/sygnal/helper/proxy/connectproxyclient_twisted.py b/sygnal/helper/proxy/connectproxyclient_twisted.py
index dfe80f99..1cf8c697 100644
--- a/sygnal/helper/proxy/connectproxyclient_twisted.py
+++ b/sygnal/helper/proxy/connectproxyclient_twisted.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Copyright 2019-2020 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -23,8 +22,15 @@
 from twisted.internet import defer, protocol
 from twisted.internet.base import ReactorBase
 from twisted.internet.defer import Deferred
-from twisted.internet.interfaces import IProtocolFactory, IStreamClientEndpoint
+from twisted.internet.interfaces import (
+    IAddress,
+    IConnector,
+    IProtocol,
+    IProtocolFactory,
+    IStreamClientEndpoint,
+)
 from twisted.internet.protocol import Protocol, connectionDone
+from twisted.python.failure import Failure
 from twisted.web import http
 from zope.interface import implementer
 
@@ -46,11 +52,10 @@ class HTTPConnectProxyEndpoint:
 
     Args:
         reactor: the Twisted reactor to use for the connection
-        proxy_endpoint (IStreamClientEndpoint): the endpoint to use to connect to the
-            proxy
-        host (bytes): hostname that we want to CONNECT to
-        port (int): port that we want to connect to
-        proxy_auth (tuple): None or tuple of (username, pasword) for HTTP basic proxy
+        proxy_endpoint: the endpoint to use to connect to the proxy
+        host: hostname that we want to CONNECT to
+        port: port that we want to connect to
+        proxy_auth: None or tuple of (username, pasword) for HTTP basic proxy
             authentication
     """
 
@@ -68,10 +73,10 @@ def __init__(
         self._port = port
         self._proxy_auth = proxy_auth
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         return "<HTTPConnectProxyEndpoint %s>" % (self._proxy_endpoint,)
 
-    def connect(self, protocolFactory: IProtocolFactory):
+    def connect(self, protocolFactory: IProtocolFactory) -> "defer.Deferred[IProtocol]":
         assert isinstance(protocolFactory, protocol.ClientFactory)
         f = HTTPProxiedClientFactory(
             self._host, self._port, self._proxy_auth, protocolFactory
@@ -111,10 +116,10 @@ def __init__(
         self.wrapped_factory = wrapped_factory
         self.on_connection: defer.Deferred = defer.Deferred()
 
-    def startedConnecting(self, connector):
+    def startedConnecting(self, connector: IConnector) -> None:
         return self.wrapped_factory.startedConnecting(connector)
 
-    def buildProtocol(self, addr):
+    def buildProtocol(self, addr: IAddress) -> "HTTPConnectProtocol":
         wrapped_protocol = self.wrapped_factory.buildProtocol(addr)
         assert wrapped_protocol is not None
 
@@ -126,13 +131,13 @@ def buildProtocol(self, addr):
             self.on_connection,
         )
 
-    def clientConnectionFailed(self, connector, reason):
+    def clientConnectionFailed(self, connector: IConnector, reason: Failure) -> None:
         logger.debug("Connection to proxy failed: %s", reason)
         if not self.on_connection.called:
             self.on_connection.errback(reason)
         return self.wrapped_factory.clientConnectionFailed(connector, reason)
 
-    def clientConnectionLost(self, connector, reason):
+    def clientConnectionLost(self, connector: IConnector, reason: Failure) -> None:
         logger.debug("Connection to proxy lost: %s", reason)
         if not self.on_connection.called:
             self.on_connection.errback(reason)
@@ -175,10 +180,10 @@ def __init__(
         )
         self.http_setup_client.on_connected.addCallback(self.proxyConnected)
 
-    def connectionMade(self):
+    def connectionMade(self) -> None:
         self.http_setup_client.makeConnection(self.transport)
 
-    def connectionLost(self, reason=connectionDone):
+    def connectionLost(self, reason: Failure = connectionDone) -> None:
         if self.wrapped_protocol.connected:
             self.wrapped_protocol.connectionLost(reason)
 
@@ -187,7 +192,7 @@ def connectionLost(self, reason=connectionDone):
         if not self.connected_deferred.called:
             self.connected_deferred.errback(reason)
 
-    def proxyConnected(self, _):
+    def proxyConnected(self, _: Optional["defer.Deferred[None]"]) -> None:
         self.wrapped_protocol.makeConnection(self.transport)
 
         self.connected_deferred.callback(self.wrapped_protocol)
@@ -197,7 +202,7 @@ def proxyConnected(self, _):
         if buf:
             self.wrapped_protocol.dataReceived(buf)
 
-    def dataReceived(self, data):
+    def dataReceived(self, data: bytes) -> None:
         # if we've set up the HTTP protocol, we can send the data there
         if self.wrapped_protocol.connected:
             return self.wrapped_protocol.dataReceived(data)
@@ -211,9 +216,9 @@ class HTTPConnectSetupClient(http.HTTPClient):
     """HTTPClient protocol to send a CONNECT message for proxies and read the response.
 
     Args:
-        host (bytes): The hostname to send in the CONNECT message
-        port (int): The port to send in the CONNECT message
-        proxy_auth (tuple): None or tuple of (username, pasword) for HTTP basic proxy
+        host: The hostname to send in the CONNECT message
+        port: The port to send in the CONNECT message
+        proxy_auth: None or tuple of (username, pasword) for HTTP basic proxy
             authentication
     """
 
@@ -223,7 +228,7 @@ def __init__(self, host: bytes, port: int, proxy_auth: Optional[Tuple[str, str]]
         self._proxy_auth = proxy_auth
         self.on_connected: defer.Deferred = defer.Deferred()
 
-    def connectionMade(self):
+    def connectionMade(self) -> None:
         logger.debug("Connected to proxy, sending CONNECT")
         self.sendCommand(b"CONNECT", b"%s:%d" % (self.host, self.port))
         if self._proxy_auth is not None:
@@ -233,14 +238,14 @@ def connectionMade(self):
             self.sendHeader(b"Proxy-Authorization", b"basic " + encoded_credentials)
         self.endHeaders()
 
-    def handleStatus(self, version, status, message):
+    def handleStatus(self, version: bytes, status: bytes, message: bytes) -> None:
         logger.debug("Got Status: %s %s %s", status, message, version)
         if status != b"200":
-            raise ProxyConnectError("Unexpected status on CONNECT: %s" % status)
+            raise ProxyConnectError(f"Unexpected status on CONNECT: {status!s}")
 
-    def handleEndHeaders(self):
+    def handleEndHeaders(self) -> None:
         logger.debug("End Headers")
         self.on_connected.callback(None)
 
-    def handleResponse(self, body):
+    def handleResponse(self, body: bytes) -> None:
         pass
diff --git a/sygnal/helper/proxy/proxy_asyncio.py b/sygnal/helper/proxy/proxy_asyncio.py
index 6f136ada..94bbb7ae 100644
--- a/sygnal/helper/proxy/proxy_asyncio.py
+++ b/sygnal/helper/proxy/proxy_asyncio.py
@@ -20,7 +20,7 @@
 from asyncio.transports import Transport
 from base64 import urlsafe_b64encode
 from ssl import Purpose, SSLContext, create_default_context
-from typing import Callable, Optional, Tuple, Union
+from typing import Any, Callable, Optional, Tuple, Union
 
 import attr
 
@@ -296,7 +296,7 @@ async def create_connection(
         host: str,
         port: int,
         ssl: Union[bool, SSLContext] = False,
-    ):
+    ) -> Tuple[BaseTransport, Protocol]:
         proxy_url_parts = decompose_http_proxy_url(self.proxy_url_str)
 
         sslcontext: Optional[SSLContext]
@@ -309,7 +309,7 @@ async def create_connection(
         else:
             sslcontext = None
 
-        def make_protocol():
+        def make_protocol() -> HttpConnectProtocol:
             proxy_setup_protocol = HttpConnectProtocol(
                 (host, port),
                 proxy_url_parts.credentials,
@@ -339,7 +339,7 @@ def make_protocol():
 
         return transport, user_protocol
 
-    def __getattr__(self, item):
+    def __getattr__(self, item: str) -> Any:
         """
         We use this to delegate other method calls to the real EventLoop.
         """
@@ -356,27 +356,27 @@ class _BufferedWrapperProtocol(Protocol):
     _connected: bool = False
     _buffer: bytearray = attr.Factory(bytearray)
 
-    def connection_made(self, transport: BaseTransport):
+    def connection_made(self, transport: BaseTransport) -> None:
         self._connected = True
         self._protocol.connection_made(transport)
         if self._buffer:
             self._protocol.data_received(self._buffer)
             self._buffer = bytearray()
 
-    def connection_lost(self, exc: Optional[Exception]):
+    def connection_lost(self, exc: Optional[Exception]) -> None:
         self._protocol.connection_lost(exc)
 
-    def pause_writing(self):
+    def pause_writing(self) -> None:
         self._protocol.pause_writing()
 
-    def resume_writing(self):
+    def resume_writing(self) -> None:
         self._protocol.resume_writing()
 
-    def data_received(self, data: bytes):
+    def data_received(self, data: bytes) -> None:
         if self._connected:
             self._protocol.data_received(data)
         else:
             self._buffer.extend(data)
 
-    def eof_received(self):
+    def eof_received(self) -> Optional[bool]:
         return self._protocol.eof_received()
diff --git a/sygnal/helper/proxy/proxyagent_twisted.py b/sygnal/helper/proxy/proxyagent_twisted.py
index c7697174..325a0f71 100644
--- a/sygnal/helper/proxy/proxyagent_twisted.py
+++ b/sygnal/helper/proxy/proxyagent_twisted.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Copyright 2019-2020 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -18,15 +17,21 @@
 
 import logging
 import re
-from typing import Optional
+from typing import Any, Dict, Optional
 
 from twisted.internet import defer
 from twisted.internet.endpoints import HostnameEndpoint, wrapClientTLS
-from twisted.internet.interfaces import IStreamClientEndpoint
+from twisted.internet.interfaces import IReactorCore, IStreamClientEndpoint
 from twisted.python.failure import Failure
-from twisted.web.client import URI, BrowserLikePolicyForHTTPS, _AgentBase
+from twisted.web.client import (
+    URI,
+    BrowserLikePolicyForHTTPS,
+    HTTPConnectionPool,
+    _AgentBase,
+)
 from twisted.web.error import SchemeNotSupported
-from twisted.web.iweb import IAgent
+from twisted.web.http_headers import Headers
+from twisted.web.iweb import IAgent, IBodyProducer, IPolicyForHTTPS, IResponse
 from zope.interface import implementer
 
 from sygnal.helper.proxy import decompose_http_proxy_url
@@ -42,35 +47,34 @@ class ProxyAgent(_AgentBase):
     """An Agent implementation which will use an HTTP proxy if one was requested
 
     Args:
-        reactor: twisted reactor to place outgoing
-            connections.
+        reactor: twisted reactor to place outgoing connections.
 
-        contextFactory (IPolicyForHTTPS): A factory for TLS contexts, to control the
+        contextFactory: A factory for TLS contexts, to control the
             verification parameters of OpenSSL.  The default is to use a
             `BrowserLikePolicyForHTTPS`, so unless you have special
             requirements you can leave this as-is.
 
-        connectTimeout (float): The amount of time that this Agent will wait
+        connectTimeout: The amount of time that this Agent will wait
             for the peer to accept a connection.
 
-        bindAddress (bytes): The local address for client sockets to bind to.
+        bindAddress: The local address for client sockets to bind to.
 
-        pool (HTTPConnectionPool|None): connection pool to be used. If None, a
+        pool: connection pool to be used. If None, a
             non-persistent pool instance will be created.
     """
 
     def __init__(
         self,
-        reactor,
-        contextFactory=BrowserLikePolicyForHTTPS(),
-        connectTimeout=None,
-        bindAddress=None,
-        pool=None,
+        reactor: IReactorCore,
+        contextFactory: IPolicyForHTTPS = BrowserLikePolicyForHTTPS(),
+        connectTimeout: Optional[float] = None,
+        bindAddress: Optional[bytes] = None,
+        pool: Optional[HTTPConnectionPool] = None,
         proxy_url_str: Optional[str] = None,
     ):
         _AgentBase.__init__(self, reactor, pool)
 
-        self._endpoint_kwargs = {}
+        self._endpoint_kwargs: Dict[str, Any] = {}
         if connectTimeout is not None:
             self._endpoint_kwargs["timeout"] = connectTimeout
         if bindAddress is not None:
@@ -89,7 +93,13 @@ def __init__(
         self._policy_for_https = contextFactory
         self._reactor = reactor
 
-    def request(self, method, uri, headers=None, bodyProducer=None):
+    def request(
+        self,
+        method: bytes,
+        uri: bytes,
+        headers: Optional[Headers] = None,
+        bodyProducer: Optional[IBodyProducer] = None,
+    ) -> "defer.Deferred[IResponse]":
         """
         Issue a request to the server indicated by the given uri.
 
@@ -101,20 +111,20 @@ def request(self, method, uri, headers=None, bodyProducer=None):
         See also: twisted.web.iweb.IAgent.request
 
         Args:
-            method (bytes): The request method to use, such as `GET`, `POST`, etc
+            method: The request method to use, such as `GET`, `POST`, etc
 
-            uri (bytes): The location of the resource to request.
+            uri: The location of the resource to request.
 
-            headers (Headers|None): Extra headers to send with the request
+            headers: Extra headers to send with the request
 
-            bodyProducer (IBodyProducer|None): An object which can generate bytes to
+            bodyProducer: An object which can generate bytes to
                 make up the body of this request (for example, the properly encoded
                 contents of a file for a file upload). Or, None if the request is to
                 have no body.
 
         Returns:
-            Deferred[IResponse]: completes when the header of the response has
-                 been received (regardless of the response status code).
+            completes when the header of the response has been received
+                (regardless of the response status code).
         """
         uri = uri.strip()
         if not _VALID_URI.match(uri):
diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py
index da42629d..074a34da 100644
--- a/sygnal/webpushpushkin.py
+++ b/sygnal/webpushpushkin.py
@@ -25,6 +25,7 @@
 from prometheus_client import Gauge, Histogram
 from py_vapid import Vapid, VapidException
 from pywebpush import CaseInsensitiveDict, webpush
+from twisted.internet import defer
 from twisted.internet.defer import DeferredSemaphore
 from twisted.web.client import FileBodyProducer, HTTPConnectionPool, readBody
 from twisted.web.http_headers import Headers
@@ -401,7 +402,7 @@ def __init__(self, endpoint: str, data: bytes, vapid_headers: CaseInsensitiveDic
 
     def execute(
         self, http_agent: ProxyAgent, low_priority: bool, topic: bytes
-    ) -> IResponse:
+    ) -> "defer.Deferred[IResponse]":
         body_producer = FileBodyProducer(BytesIO(self.data))
         # Convert the headers to the camelcase version.
         headers = {
diff --git a/tests/test_concurrency_limit.py b/tests/test_concurrency_limit.py
index 958521f4..ee6aa958 100644
--- a/tests/test_concurrency_limit.py
+++ b/tests/test_concurrency_limit.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Copyright 2019–2020 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -44,7 +43,7 @@
 
 class SlowConcurrencyLimitedDummyPushkin(ConcurrencyLimitedPushkin):
     async def _dispatch_notification_unlimited(
-        self, n: Notification, device: Device, context: "NotificationContext"
+        self, n: Notification, device: Device, context: NotificationContext
     ) -> List[str]:
         """
         We will deliver the notification to the mighty nobody
diff --git a/tests/test_http.py b/tests/test_http.py
index 1ce4183f..c190a204 100644
--- a/tests/test_http.py
+++ b/tests/test_http.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Copyright 2019 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -12,6 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+from typing import Any, Dict
 from unittest.mock import MagicMock, patch
 
 from aioapns.common import NotificationResult
@@ -49,7 +49,7 @@
 
 
 class HttpTestCase(testutils.TestCase):
-    def setUp(self):
+    def setUp(self) -> None:
         self.apns_mock_class = patch("sygnal.apnspushkin.APNs").start()
         self.apns_mock = MagicMock()
         self.apns_mock_class.return_value = self.apns_mock
@@ -69,13 +69,13 @@ def setUp(self):
             # see https://github.com/python/mypy/issues/2427
             value._send_notification = self.apns_pushkin_snotif  # type: ignore[assignment] # noqa: E501
 
-    def config_setup(self, config):
+    def config_setup(self, config: Dict[str, Any]) -> None:
         super().config_setup(config)
         config["apps"][PUSHKIN_ID_1] = {"type": "apns", "certfile": TEST_CERTFILE_PATH}
         config["apps"][PUSHKIN_ID_2] = {"type": "apns", "certfile": TEST_CERTFILE_PATH}
         config["apps"][PUSHKIN_ID_3] = {"type": "apns", "certfile": TEST_CERTFILE_PATH}
 
-    def test_with_specific_appid(self):
+    def test_with_specific_appid(self) -> None:
         """
         Tests the expected case: A specific app id must be processed.
         """
@@ -94,7 +94,7 @@ def test_with_specific_appid(self):
 
         self.assertEqual({"rejected": []}, resp)
 
-    def test_with_matching_appid(self):
+    def test_with_matching_appid(self) -> None:
         """
         Tests the matching case: A matching app id (only one time) must be processed.
         """
@@ -113,7 +113,7 @@ def test_with_matching_appid(self):
 
         self.assertEqual({"rejected": []}, resp)
 
-    def test_with_ambigious_appid(self):
+    def test_with_ambigious_appid(self) -> None:
         """
         Tests the rejection case: An ambigious app id should be rejected without
         processing.
diff --git a/tests/test_pushgateway_api_v1.py b/tests/test_pushgateway_api_v1.py
index dd672710..ab4ee7cf 100644
--- a/tests/test_pushgateway_api_v1.py
+++ b/tests/test_pushgateway_api_v1.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Copyright 2019 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -13,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from typing import Any, Dict, List
+
 from twisted.internet.address import IPv6Address
 from twisted.internet.testing import StringTransport
 
@@ -20,7 +21,7 @@
     NotificationDispatchException,
     TemporaryNotificationDispatchException,
 )
-from sygnal.notifications import Pushkin
+from sygnal.notifications import Device, Notification, NotificationContext, Pushkin
 
 from tests import testutils
 
@@ -60,7 +61,9 @@ class TestPushkin(Pushkin):
     A synthetic Pushkin with simple rules.
     """
 
-    async def dispatch_notification(self, n, device, context):
+    async def dispatch_notification(
+        self, n: Notification, device: Device, context: NotificationContext
+    ) -> List[str]:
         if device.pushkey == "raise_exception":
             raise Exception("Bad things have occurred!")
         elif device.pushkey == "remote_error":
@@ -75,7 +78,7 @@ async def dispatch_notification(self, n, device, context):
 
 
 class PushGatewayApiV1TestCase(testutils.TestCase):
-    def config_setup(self, config):
+    def config_setup(self, config: Dict[str, Any]) -> None:
         """
         Set up a TestPushkin for the test.
         """
@@ -84,7 +87,7 @@ def config_setup(self, config):
             "type": "tests.test_pushgateway_api_v1.TestPushkin"
         }
 
-    def test_good_requests_give_200(self):
+    def test_good_requests_give_200(self) -> None:
         """
         Test that good requests give a 200 response code.
         """
@@ -98,7 +101,7 @@ def test_good_requests_give_200(self):
             )
         )
 
-    def test_accepted_devices_are_not_rejected(self):
+    def test_accepted_devices_are_not_rejected(self) -> None:
         """
         Test that devices which are accepted by the Pushkin
         do not lead to a rejection being returned to the homeserver.
@@ -108,7 +111,7 @@ def test_accepted_devices_are_not_rejected(self):
             {"rejected": []},
         )
 
-    def test_rejected_devices_are_rejected(self):
+    def test_rejected_devices_are_rejected(self) -> None:
         """
         Test that devices which are rejected by the Pushkin
         DO lead to a rejection being returned to the homeserver.
@@ -118,7 +121,7 @@ def test_rejected_devices_are_rejected(self):
             {"rejected": [DEVICE_REJECTED["pushkey"]]},
         )
 
-    def test_only_rejected_devices_are_rejected(self):
+    def test_only_rejected_devices_are_rejected(self) -> None:
         """
         Test that devices which are rejected by the Pushkin
         are the only ones to have a rejection returned to the homeserver,
@@ -131,13 +134,13 @@ def test_only_rejected_devices_are_rejected(self):
             {"rejected": [DEVICE_REJECTED["pushkey"]]},
         )
 
-    def test_bad_requests_give_400(self):
+    def test_bad_requests_give_400(self) -> None:
         """
         Test that bad requests lead to a 400 Bad Request response.
         """
         self.assertEqual(self._request({}), 400)
 
-    def test_exceptions_give_500(self):
+    def test_exceptions_give_500(self) -> None:
         """
         Test that internal exceptions/errors lead to a 500 Internal Server Error
         response.
@@ -162,7 +165,7 @@ def test_exceptions_give_500(self):
             500,
         )
 
-    def test_remote_errors_give_502(self):
+    def test_remote_errors_give_502(self) -> None:
         """
         Test that errors caused by remote services such as GCM or APNS
         lead to a 502 Bad Gateway response.
@@ -187,7 +190,7 @@ def test_remote_errors_give_502(self):
             502,
         )
 
-    def test_overlong_requests_are_rejected(self):
+    def test_overlong_requests_are_rejected(self) -> None:
         # as a control case, first send a regular request.
 
         # connect the site to a fake transport.