From fb59d5912de75caaffda7ef78a5b0bb38bc3471f Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sat, 7 May 2022 23:22:24 +0100 Subject: [PATCH 01/12] Use `ParamSpec` in s.app._base --- synapse/app/_base.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index d28b87a3f4d8..3623c1724ded 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -38,6 +38,7 @@ from cryptography.utils import CryptographyDeprecationWarning from matrix_common.versionstring import get_distribution_version_string +from typing_extensions import ParamSpec import twisted from twisted.internet import defer, error, reactor as _reactor @@ -81,11 +82,12 @@ # list of tuples of function, args list, kwargs dict _sighup_callbacks: List[ - Tuple[Callable[..., None], Tuple[Any, ...], Dict[str, Any]] + Tuple[Callable[..., None], Tuple[object, ...], Dict[str, object]] ] = [] +P = ParamSpec("P") -def register_sighup(func: Callable[..., None], *args: Any, **kwargs: Any) -> None: +def register_sighup(func: Callable[P, None], *args: P.args, **kwargs: P.kwargs) -> None: """ Register a function to be called when a SIGHUP occurs. @@ -93,7 +95,9 @@ def register_sighup(func: Callable[..., None], *args: Any, **kwargs: Any) -> Non func: Function to be called when sent a SIGHUP signal. *args, **kwargs: args and kwargs to be passed to the target function. """ - _sighup_callbacks.append((func, args, kwargs)) + # This type-ignore should be redundant once we use a mypy release with + # https://github.com/python/mypy/pull/12668. + _sighup_callbacks.append((func, args, kwargs)) # type: ignore[arg-type] def start_worker_reactor( @@ -214,7 +218,9 @@ def redirect_stdio_to_logs() -> None: print("Redirected stdout/stderr to logs") -def register_start(cb: Callable[..., Awaitable], *args: Any, **kwargs: Any) -> None: +def register_start( + cb: Callable[P, Awaitable], *args: P.args, **kwargs: P.kwargs +) -> None: """Register a callback with the reactor, to be called once it is running This can be used to initialise parts of the system which require an asynchronous From 0ed7c3343fe4d1dcc2ac1fde28c873d0a18d096c Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sat, 7 May 2022 23:35:16 +0100 Subject: [PATCH 02/12] Use `ParamSpec` in `Signal` Don't think there's any way to use it in distributor, because we dispatch based on strings. But we only seem to use it for `user_left_room` so it's probably not worth it. Was the signals mechanism home-grown but then never used? --- synapse/util/distributor.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/synapse/util/distributor.py b/synapse/util/distributor.py index 91837655f8fa..b580bdd0deef 100644 --- a/synapse/util/distributor.py +++ b/synapse/util/distributor.py @@ -12,7 +12,19 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import Any, Callable, Dict, List +from typing import ( + Any, + Awaitable, + Callable, + Dict, + Generic, + List, + Optional, + TypeVar, + Union, +) + +from typing_extensions import ParamSpec from twisted.internet import defer @@ -75,7 +87,11 @@ def fire(self, name: str, *args: Any, **kwargs: Any) -> None: run_as_background_process(name, self.signals[name].fire, *args, **kwargs) -class Signal: +P = ParamSpec("P") +R = TypeVar("R") + + +class Signal(Generic[P]): """A Signal is a dispatch point that stores a list of callables as observers of it. @@ -87,16 +103,16 @@ class Signal: def __init__(self, name: str): self.name: str = name - self.observers: List[Callable] = [] + self.observers: List[Callable[P, Any]] = [] - def observe(self, observer: Callable) -> None: + def observe(self, observer: Callable[P, Any]) -> None: """Adds a new callable to the observer list which will be invoked by the 'fire' method. Each observer callable may return a Deferred.""" self.observers.append(observer) - def fire(self, *args: Any, **kwargs: Any) -> "defer.Deferred[List[Any]]": + def fire(self, *args: P.args, **kwargs: P.kwargs) -> "defer.Deferred[List[Any]]": """Invokes every callable in the observer list, passing in the args and kwargs. Exceptions thrown by observers are logged but ignored. It is not an error to fire a signal with no observers. @@ -104,7 +120,7 @@ def fire(self, *args: Any, **kwargs: Any) -> "defer.Deferred[List[Any]]": Returns a Deferred that will complete when all the observers have completed.""" - async def do(observer: Callable[..., Any]) -> Any: + async def do(observer: Callable[P, Union[R, Awaitable[R]]]) -> Optional[R]: try: return await maybe_awaitable(observer(*args, **kwargs)) except Exception as e: @@ -114,6 +130,7 @@ async def do(observer: Callable[..., Any]) -> Any: observer, e, ) + return None deferreds = [run_in_background(do, o) for o in self.observers] From c71104f53c6b1a56048e09dae66dedce7aef6ba0 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 00:40:52 +0100 Subject: [PATCH 03/12] Use ParamSpec in s.util.patch_inline_callbacks I don't even want to know what kind of sorcery this is --- synapse/util/patch_inline_callbacks.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/synapse/util/patch_inline_callbacks.py b/synapse/util/patch_inline_callbacks.py index dace68666c9c..f97f98a05742 100644 --- a/synapse/util/patch_inline_callbacks.py +++ b/synapse/util/patch_inline_callbacks.py @@ -16,6 +16,8 @@ import sys from typing import Any, Callable, Generator, List, TypeVar, cast +from typing_extensions import ParamSpec + from twisted.internet import defer from twisted.internet.defer import Deferred from twisted.python.failure import Failure @@ -25,6 +27,7 @@ T = TypeVar("T") +P = ParamSpec("P") def do_patch() -> None: @@ -41,13 +44,13 @@ def do_patch() -> None: return def new_inline_callbacks( - f: Callable[..., Generator["Deferred[object]", object, T]] - ) -> Callable[..., "Deferred[T]"]: + f: Callable[P, Generator["Deferred[object]", object, T]] + ) -> Callable[P, "Deferred[T]"]: @functools.wraps(f) - def wrapped(*args: Any, **kwargs: Any) -> "Deferred[T]": + def wrapped(*args: P.args, **kwargs: P.kwargs) -> "Deferred[T]": start_context = current_context() changes: List[str] = [] - orig: Callable[..., "Deferred[T]"] = orig_inline_callbacks( + orig: Callable[P, "Deferred[T]"] = orig_inline_callbacks( _check_yield_points(f, changes) ) @@ -115,7 +118,7 @@ def check_ctx(r: T) -> T: def _check_yield_points( - f: Callable[..., Generator["Deferred[object]", object, T]], + f: Callable[P, Generator["Deferred[object]", object, T]], changes: List[str], ) -> Callable: """Wraps a generator that is about to be passed to defer.inlineCallbacks @@ -138,7 +141,7 @@ def _check_yield_points( @functools.wraps(f) def check_yield_points_inner( - *args: Any, **kwargs: Any + *args: P.args, **kwargs: P.kwargs ) -> Generator["Deferred[object]", object, T]: gen = f(*args, **kwargs) From a8db29a2038db9412ecf47419fb1d1ef5cc7a5e1 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 03:27:31 +0100 Subject: [PATCH 04/12] Minor use of ParamSpec s.util.metrics --- synapse/util/metrics.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/synapse/util/metrics.py b/synapse/util/metrics.py index 98ee49af6eb6..bc3b4938ea15 100644 --- a/synapse/util/metrics.py +++ b/synapse/util/metrics.py @@ -15,10 +15,10 @@ import logging from functools import wraps from types import TracebackType -from typing import Any, Callable, Optional, Type, TypeVar, cast +from typing import Awaitable, Callable, Optional, Type, TypeVar from prometheus_client import Counter -from typing_extensions import Protocol +from typing_extensions import Concatenate, ParamSpec, Protocol from synapse.logging.context import ( ContextResourceUsage, @@ -72,16 +72,21 @@ class _InFlightMetric(Protocol): ) -T = TypeVar("T", bound=Callable[..., Any]) +P = ParamSpec("P") +R = TypeVar("R") class HasClock(Protocol): clock: Clock -def measure_func(name: Optional[str] = None) -> Callable[[T], T]: - """ - Used to decorate an async function with a `Measure` context manager. +def measure_func( + name: Optional[str] = None, +) -> Callable[[Callable[P, Awaitable[R]]], Callable[P, Awaitable[R]]]: + """Decorate an async method with a `Measure` context manager. + + The Measure is created using `self.clock`; it should only be used to decorate + methods in classes defining an instance-level `clock` attribute. Usage: @@ -97,18 +102,24 @@ async def foo(...): """ - def wrapper(func: T) -> T: + def wrapper( + func: Callable[Concatenate[HasClock, P], Awaitable[R]] + ) -> Callable[P, Awaitable[R]]: block_name = func.__name__ if name is None else name @wraps(func) - async def measured_func(self: HasClock, *args: Any, **kwargs: Any) -> Any: + async def measured_func(self: HasClock, *args: P.args, **kwargs: P.kwargs) -> R: with Measure(self.clock, block_name): r = await func(self, *args, **kwargs) return r - return cast(T, measured_func) + # There are some shenanigans here, because we're decorating a method but + # explicitly making use of the `self` parameter. The key thing here is that the + # return type within the return type for `measure_func` itself describes how the + # decorated function will be called. + return measured_func # type: ignore[return-value] - return wrapper + return wrapper # type: ignore[return-value] class Measure: From 2cbc7f39cf56bb2b60ab7942548c16f16f22c971 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 14:39:02 +0100 Subject: [PATCH 05/12] ParamSpec in presence_router --- synapse/events/presence_router.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/synapse/events/presence_router.py b/synapse/events/presence_router.py index 98555c8c0c65..8437ce52dc97 100644 --- a/synapse/events/presence_router.py +++ b/synapse/events/presence_router.py @@ -22,9 +22,12 @@ List, Optional, Set, + TypeVar, Union, ) +from typing_extensions import ParamSpec + from synapse.api.presence import UserPresenceState from synapse.util.async_helpers import maybe_awaitable @@ -40,6 +43,10 @@ logger = logging.getLogger(__name__) +P = ParamSpec("P") +R = TypeVar("R") + + def load_legacy_presence_router(hs: "HomeServer") -> None: """Wrapper that loads a presence router module configured using the old configuration, and registers the hooks they implement. @@ -63,13 +70,15 @@ def load_legacy_presence_router(hs: "HomeServer") -> None: # All methods that the module provides should be async, but this wasn't enforced # in the old module system, so we wrap them if needed - def async_wrapper(f: Optional[Callable]) -> Optional[Callable[..., Awaitable]]: + def async_wrapper( + f: Optional[Callable[P, R]] + ) -> Optional[Callable[P, Awaitable[R]]]: # f might be None if the callback isn't implemented by the module. In this # case we don't want to register a callback at all so we return None. if f is None: return None - def run(*args: Any, **kwargs: Any) -> Awaitable: + def run(*args: P.args, **kwargs: P.kwargs) -> Awaitable[R]: # Assertion required because mypy can't prove we won't change `f` # back to `None`. See # https://mypy.readthedocs.io/en/latest/common_issues.html#narrowing-and-inner-functions @@ -80,7 +89,7 @@ def run(*args: Any, **kwargs: Any) -> Awaitable: return run # Register the hooks through the module API. - hooks = { + hooks: Dict[str, Optional[Callable[..., Any]]] = { hook: async_wrapper(getattr(presence_router, hook, None)) for hook in presence_router_methods } From 3ef4187d94e34faa4726cad9abf4b83e7180a6f6 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 15:17:22 +0100 Subject: [PATCH 06/12] Use more `ParamSpec` in s.storage.main.database --- synapse/storage/database.py | 50 ++++++++++++++++-------- synapse/storage/databases/main/events.py | 12 ++++-- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index df1e9c1b831d..41f566b6487a 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -38,7 +38,7 @@ import attr from prometheus_client import Histogram -from typing_extensions import Literal +from typing_extensions import Concatenate, Literal, ParamSpec from twisted.enterprise import adbapi @@ -192,9 +192,9 @@ def __getattr__(self, name): # The type of entry which goes on our after_callbacks and exception_callbacks lists. -_CallbackListEntry = Tuple[Callable[..., object], Iterable[Any], Dict[str, Any]] - +_CallbackListEntry = Tuple[Callable[..., object], Tuple[object, ...], Dict[str, object]] +P = ParamSpec("P") R = TypeVar("R") @@ -239,7 +239,9 @@ def __init__( self.after_callbacks = after_callbacks self.exception_callbacks = exception_callbacks - def call_after(self, callback: Callable[..., object], *args: Any, **kwargs: Any): + def call_after( + self, callback: Callable[P, object], *args: P.args, **kwargs: P.kwargs + ) -> None: """Call the given callback on the main twisted thread after the transaction has finished. @@ -256,11 +258,12 @@ def call_after(self, callback: Callable[..., object], *args: Any, **kwargs: Any) # LoggingTransaction isn't expecting there to be any callbacks; assert that # is not the case. assert self.after_callbacks is not None - self.after_callbacks.append((callback, args, kwargs)) + # type-ignore: need mypy containing https://github.com/python/mypy/pull/12668 + self.after_callbacks.append((callback, args, kwargs)) # type: ignore[arg-type] def call_on_exception( - self, callback: Callable[..., object], *args: Any, **kwargs: Any - ): + self, callback: Callable[P, object], *args: P.args, **kwargs: P.kwargs + ) -> None: """Call the given callback on the main twisted thread after the transaction has failed. @@ -274,7 +277,8 @@ def call_on_exception( # LoggingTransaction isn't expecting there to be any callbacks; assert that # is not the case. assert self.exception_callbacks is not None - self.exception_callbacks.append((callback, args, kwargs)) + # type-ignore: need mypy containing https://github.com/python/mypy/pull/12668 + self.exception_callbacks.append((callback, args, kwargs)) # type: ignore[arg-type] def fetchone(self) -> Optional[Tuple]: return self.txn.fetchone() @@ -339,7 +343,13 @@ def _make_sql_one_line(self, sql: str) -> str: "Strip newlines out of SQL so that the loggers in the DB are on one line" return " ".join(line.strip() for line in sql.splitlines() if line.strip()) - def _do_execute(self, func: Callable[..., R], sql: str, *args: Any) -> R: + def _do_execute( + self, + func: Callable[Concatenate[str, P], R], + sql: str, + *args: P.args, + **kwargs: P.kwargs, + ) -> R: sql = self._make_sql_one_line(sql) # TODO(paul): Maybe use 'info' and 'debug' for values? @@ -348,7 +358,10 @@ def _do_execute(self, func: Callable[..., R], sql: str, *args: Any) -> R: sql = self.database_engine.convert_param_style(sql) if args: try: - sql_logger.debug("[SQL values] {%s} %r", self.name, args[0]) + # The type-ignore should be redundant once mypy releases a version with + # https://github.com/python/mypy/pull/12668. (`args` might be empty, + # (but we'll catch the index error if so.) + sql_logger.debug("[SQL values] {%s} %r", self.name, args[0]) # type: ignore[index] except Exception: # Don't let logging failures stop SQL from working pass @@ -363,7 +376,7 @@ def _do_execute(self, func: Callable[..., R], sql: str, *args: Any) -> R: opentracing.tags.DATABASE_STATEMENT: sql, }, ): - return func(sql, *args) + return func(sql, *args, **kwargs) except Exception as e: sql_logger.debug("[SQL FAIL] {%s} %s", self.name, e) raise @@ -540,9 +553,9 @@ def new_transaction( desc: str, after_callbacks: List[_CallbackListEntry], exception_callbacks: List[_CallbackListEntry], - func: Callable[..., R], - *args: Any, - **kwargs: Any, + func: Callable[Concatenate[LoggingTransaction, P], R], + *args: P.args, + **kwargs: P.kwargs, ) -> R: """Start a new database transaction with the given connection. @@ -572,7 +585,10 @@ def new_transaction( # will fail if we have to repeat the transaction. # For now, we just log an error, and hope that it works on the first attempt. # TODO: raise an exception. - for i, arg in enumerate(args): + + # Type-ignore Mypy doesn't yet consider ParamSpec.args to be iterable; see + # https://github.com/python/mypy/pull/12668 + for i, arg in enumerate(args): # type: ignore[arg-type, var-annotated] if inspect.isgenerator(arg): logger.error( "Programming error: generator passed to new_transaction as " @@ -580,7 +596,9 @@ def new_transaction( i, func, ) - for name, val in kwargs.items(): + # Type-ignore Mypy doesn't yet consider ParamSpec.args to be a mapping; see + # https://github.com/python/mypy/pull/12668 + for name, val in kwargs.items(): # type: ignore[attr-defined] if inspect.isgenerator(val): logger.error( "Programming error: generator passed to new_transaction as " diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 9a6c2fd47a55..f33e61bc6d76 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1648,9 +1648,15 @@ def prefill(): txn.call_after(prefill) def _store_redaction(self, txn: LoggingTransaction, event: EventBase) -> None: - # Invalidate the caches for the redacted event, note that these caches - # are also cleared as part of event replication in _invalidate_caches_for_event. - txn.call_after(self.store._invalidate_get_event_cache, event.redacts) + """Invalidate the caches for the redacted event. + + Note that these caches are also cleared as part of event replication in + _invalidate_caches_for_event. + """ + + # type-ignore: mypy detects that event.redacts may be None. Presumably the + # application has ensured that this is not the case if we call this function. + txn.call_after(self.store._invalidate_get_event_cache, event.redacts) # type: ignore[arg-type] txn.call_after(self.store.get_relations_for_event.invalidate, (event.redacts,)) txn.call_after(self.store.get_applicable_edit.invalidate, (event.redacts,)) From 266ba480fdec7638d18a7b274fb26b26f190b027 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 17:11:16 +0100 Subject: [PATCH 07/12] ParamSpec in module api --- synapse/module_api/__init__.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 834fe1b62c77..73f92d2df8d6 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -30,6 +30,7 @@ import attr import jinja2 +from typing_extensions import ParamSpec from twisted.internet import defer from twisted.web.resource import Resource @@ -129,6 +130,7 @@ T = TypeVar("T") +P = ParamSpec("P") """ This package defines the 'stable' API which can be used by extension modules which @@ -799,9 +801,9 @@ def invalidate_access_token( def run_db_interaction( self, desc: str, - func: Callable[..., T], - *args: Any, - **kwargs: Any, + func: Callable[P, T], + *args: P.args, + **kwargs: P.kwargs, ) -> "defer.Deferred[T]": """Run a function with a database connection @@ -817,8 +819,9 @@ def run_db_interaction( Returns: Deferred[object]: result of func """ + # type-ignore: See https://github.com/python/mypy/issues/8862 return defer.ensureDeferred( - self._store.db_pool.runInteraction(desc, func, *args, **kwargs) + self._store.db_pool.runInteraction(desc, func, *args, **kwargs) # type: ignore[arg-type] ) def complete_sso_login( @@ -1296,9 +1299,9 @@ async def get_room_state( async def defer_to_thread( self, - f: Callable[..., T], - *args: Any, - **kwargs: Any, + f: Callable[P, T], + *args: P.args, + **kwargs: P.kwargs, ) -> T: """Runs the given function in a separate thread from Synapse's thread pool. From 47837255ca9989b7cd44c2384695662a9c145496 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 17:14:19 +0100 Subject: [PATCH 08/12] ParamSpec in s.rest.client.transactions --- synapse/rest/client/knock.py | 4 +--- synapse/rest/client/transactions.py | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/synapse/rest/client/knock.py b/synapse/rest/client/knock.py index 0152a0c66a50..ad025c8a4529 100644 --- a/synapse/rest/client/knock.py +++ b/synapse/rest/client/knock.py @@ -15,8 +15,6 @@ import logging from typing import TYPE_CHECKING, Awaitable, Dict, List, Optional, Tuple -from twisted.web.server import Request - from synapse.api.constants import Membership from synapse.api.errors import SynapseError from synapse.http.server import HttpServer @@ -97,7 +95,7 @@ async def on_POST( return 200, {"room_id": room_id} def on_PUT( - self, request: Request, room_identifier: str, txn_id: str + self, request: SynapseRequest, room_identifier: str, txn_id: str ) -> Awaitable[Tuple[int, JsonDict]]: set_tag("txn_id", txn_id) diff --git a/synapse/rest/client/transactions.py b/synapse/rest/client/transactions.py index 914fb3acf5aa..61375651bc15 100644 --- a/synapse/rest/client/transactions.py +++ b/synapse/rest/client/transactions.py @@ -15,7 +15,9 @@ """This module contains logic for storing HTTP PUT transactions. This is used to ensure idempotency when performing PUTs using the REST API.""" import logging -from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, Tuple +from typing import TYPE_CHECKING, Awaitable, Callable, Dict, Tuple + +from typing_extensions import ParamSpec from twisted.python.failure import Failure from twisted.web.server import Request @@ -32,6 +34,9 @@ CLEANUP_PERIOD_MS = 1000 * 60 * 30 # 30 mins +P = ParamSpec("P") + + class HttpTransactionCache: def __init__(self, hs: "HomeServer"): self.hs = hs @@ -65,9 +70,9 @@ def _get_transaction_key(self, request: Request) -> str: def fetch_or_execute_request( self, request: Request, - fn: Callable[..., Awaitable[Tuple[int, JsonDict]]], - *args: Any, - **kwargs: Any, + fn: Callable[P, Awaitable[Tuple[int, JsonDict]]], + *args: P.args, + **kwargs: P.kwargs, ) -> Awaitable[Tuple[int, JsonDict]]: """A helper function for fetch_or_execute which extracts a transaction key from the given request. @@ -82,9 +87,9 @@ def fetch_or_execute_request( def fetch_or_execute( self, txn_key: str, - fn: Callable[..., Awaitable[Tuple[int, JsonDict]]], - *args: Any, - **kwargs: Any, + fn: Callable[P, Awaitable[Tuple[int, JsonDict]]], + *args: P.args, + **kwargs: P.kwargs, ) -> Awaitable[Tuple[int, JsonDict]]: """Fetches the response for this transaction, or executes the given function to produce a response for this transaction. From b95880b1a58def32db11a0246ce167a03906cd3b Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 18:03:21 +0100 Subject: [PATCH 09/12] ParamSpec in s.util.async_helpers --- synapse/util/async_helpers.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index e27c5d298f6a..b91020117f41 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -42,7 +42,7 @@ ) import attr -from typing_extensions import AsyncContextManager, Literal +from typing_extensions import AsyncContextManager, Concatenate, Literal, ParamSpec from twisted.internet import defer from twisted.internet.defer import CancelledError @@ -237,9 +237,16 @@ async def _concurrently_execute_inner(value: T) -> None: ) +P = ParamSpec("P") +R = TypeVar("R") + + async def yieldable_gather_results( - func: Callable[..., Awaitable[T]], iter: Iterable, *args: Any, **kwargs: Any -) -> List[T]: + func: Callable[Concatenate[T, P], Awaitable[R]], + iter: Iterable[T], + *args: P.args, + **kwargs: P.kwargs, +) -> List[R]: """Executes the function with each argument concurrently. Args: @@ -255,7 +262,15 @@ async def yieldable_gather_results( try: return await make_deferred_yieldable( defer.gatherResults( - [run_in_background(func, item, *args, **kwargs) for item in iter], + # type-ignore: mypy reports two errors: + # error: Argument 1 to "run_in_background" has incompatible type + # "Callable[[T, **P], Awaitable[R]]"; expected + # "Callable[[T, **P], Awaitable[R]]" [arg-type] + # error: Argument 2 to "run_in_background" has incompatible type + # "T"; expected "[T, **P.args]" [arg-type] + # The former looks like a mypy bug, and the latter looks like a + # false positive. + [run_in_background(func, item, *args, **kwargs) for item in iter], # type: ignore[arg-type] consumeErrors=True, ) ) @@ -577,9 +592,6 @@ async def _ctx_manager() -> AsyncIterator[None]: return _ctx_manager() -R = TypeVar("R") - - def timeout_deferred( deferred: "defer.Deferred[_T]", timeout: float, reactor: IReactorTime ) -> "defer.Deferred[_T]": From ce18fab3dbe8bd943daec7e84507815dd2e26b33 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Sun, 8 May 2022 21:34:13 +0100 Subject: [PATCH 10/12] Changelog --- changelog.d/12667.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12667.misc diff --git a/changelog.d/12667.misc b/changelog.d/12667.misc new file mode 100644 index 000000000000..2b17502d6b57 --- /dev/null +++ b/changelog.d/12667.misc @@ -0,0 +1 @@ +Use `ParamSpec` to refine type hints. From cf077fd185057a60d9c62ad6a74ceafb900186e7 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 9 May 2022 10:35:41 +0100 Subject: [PATCH 11/12] Explicit assert, rather than type-ignore --- synapse/storage/databases/main/events.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index f33e61bc6d76..ed29a0a5e2db 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1653,10 +1653,8 @@ def _store_redaction(self, txn: LoggingTransaction, event: EventBase) -> None: Note that these caches are also cleared as part of event replication in _invalidate_caches_for_event. """ - - # type-ignore: mypy detects that event.redacts may be None. Presumably the - # application has ensured that this is not the case if we call this function. - txn.call_after(self.store._invalidate_get_event_cache, event.redacts) # type: ignore[arg-type] + assert event.redacts is not None + txn.call_after(self.store._invalidate_get_event_cache, event.redacts) txn.call_after(self.store.get_relations_for_event.invalidate, (event.redacts,)) txn.call_after(self.store.get_applicable_edit.invalidate, (event.redacts,)) From 109b78126a346d535b244bb5d4632db7b663db8b Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 9 May 2022 11:00:35 +0100 Subject: [PATCH 12/12] Require typing-extensions >= 3.10.0.1 Doesn't change the lockfile, but should fix olddeps --- poetry.lock | 2 +- pyproject.toml | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/poetry.lock b/poetry.lock index ddafaaeba066..f649efdf2b9c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1563,7 +1563,7 @@ url_preview = ["lxml"] [metadata] lock-version = "1.1" python-versions = "^3.7.1" -content-hash = "eebc9e1d720e2e866f5fddda98ce83d858949a6fdbe30c7e5aef4cf9d17be498" +content-hash = "d39d5ac5d51c014581186b7691999b861058b569084c525523baf70b77f292b1" [metadata.files] attrs = [ diff --git a/pyproject.toml b/pyproject.toml index 4c51b8c4a112..2c4b7eb08ecd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -143,7 +143,9 @@ netaddr = ">=0.7.18" Jinja2 = ">=3.0" bleach = ">=1.4.3" # We use `ParamSpec` and `Concatenate`, which were added in `typing-extensions` 3.10.0.0. -typing-extensions = ">=3.10.0" +# Additionally we need https://github.com/python/typing/pull/817 to allow types to be +# generic over ParamSpecs. +typing-extensions = ">=3.10.0.1" # We enforce that we have a `cryptography` version that bundles an `openssl` # with the latest security patches. cryptography = ">=3.4.7"