From c5e05aa5c23abbc62ff65e18dee98c70d818c440 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 7 Oct 2021 20:59:36 +0100 Subject: [PATCH 1/3] disallow-untyped-defs for synapse.module_api --- mypy.ini | 3 ++ synapse/module_api/__init__.py | 91 +++++++++++++++++++++++----------- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/mypy.ini b/mypy.ini index bc4f59154d9e..73c3a980a436 100644 --- a/mypy.ini +++ b/mypy.ini @@ -166,6 +166,9 @@ disallow_untyped_defs = True [mypy-synapse.metrics.*] disallow_untyped_defs = True +[mypy-synapse.module_api.*] +disallow_untyped_defs = True + [mypy-synapse.push.*] disallow_untyped_defs = True diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 96d7a8f2a95b..bd524d0ffab8 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -24,6 +24,7 @@ List, Optional, Tuple, + TypeVar, Union, ) @@ -104,6 +105,9 @@ from synapse.app.generic_worker import GenericWorkerSlavedStore from synapse.server import HomeServer + +T = TypeVar("T") + """ This package defines the 'stable' API which can be used by extension modules which are loaded into Synapse. @@ -146,6 +150,12 @@ class UserIpAndAgent: last_seen: int +# This is sad: I'd like to express that the arguments to this Callable are +# themselves callable. "Callback protocols" let you do this, but I couldn't find +# a way to do so that mypy agreed was type safe. +RegisterCallbacks = Callable[..., None] + + class ModuleApi: """A proxy object that gets passed to various plugin modules so they can register new users etc if necessary. @@ -307,7 +317,7 @@ def register_password_auth_provider_callbacks( auth_checkers=auth_checkers, ) - def register_web_resource(self, path: str, resource: Resource): + def register_web_resource(self, path: str, resource: Resource) -> None: """Registers a web resource to be served at the given path. This function should be called during initialisation of the module. @@ -432,7 +442,7 @@ def get_qualified_user_id(self, username: str) -> str: username: provided user id Returns: - str: qualified @user:id + qualified @user:id """ if username.startswith("@"): return username @@ -468,7 +478,7 @@ async def get_threepids_for_user(self, user_id: str) -> List[Dict[str, str]]: """ return await self._store.user_get_threepids(user_id) - def check_user_exists(self, user_id: str): + def check_user_exists(self, user_id: str) -> "defer.Deferred[Optional[str]]": """Check if user exists. Added in Synapse v0.25.0. @@ -477,13 +487,18 @@ def check_user_exists(self, user_id: str): user_id: Complete @user:id Returns: - Deferred[str|None]: Canonical (case-corrected) user_id, or None + Canonical (case-corrected) user_id, or None if the user is not registered. """ return defer.ensureDeferred(self._auth_handler.check_user_exists(user_id)) @defer.inlineCallbacks - def register(self, localpart, displayname=None, emails: Optional[List[str]] = None): + def register( + self, + localpart: str, + displayname: Optional[str] = None, + emails: Optional[List[str]] = None, + ) -> Generator["defer.Deferred[Any]", Any, Tuple[str, str]]: """Registers a new user with given localpart and optional displayname, emails. Also returns an access token for the new user. @@ -495,12 +510,12 @@ def register(self, localpart, displayname=None, emails: Optional[List[str]] = No Added in Synapse v0.25.0. Args: - localpart (str): The localpart of the new user. - displayname (str|None): The displayname of the new user. - emails (List[str]): Emails to bind to the new user. + localpart: The localpart of the new user. + displayname: The displayname of the new user. + emails: Emails to bind to the new user. Returns: - Deferred[tuple[str, str]]: a 2-tuple of (user_id, access_token) + a 2-tuple of (user_id, access_token) """ logger.warning( "Using deprecated ModuleApi.register which creates a dummy user device." @@ -510,23 +525,26 @@ def register(self, localpart, displayname=None, emails: Optional[List[str]] = No return user_id, access_token def register_user( - self, localpart, displayname=None, emails: Optional[List[str]] = None - ): + self, + localpart: str, + displayname: Optional[str] = None, + emails: Optional[List[str]] = None, + ) -> "defer.Deferred[str]": """Registers a new user with given localpart and optional displayname, emails. Added in Synapse v1.2.0. Args: - localpart (str): The localpart of the new user. - displayname (str|None): The displayname of the new user. - emails (List[str]): Emails to bind to the new user. + localpart: The localpart of the new user. + displayname: The displayname of the new user. + emails: Emails to bind to the new user. Raises: SynapseError if there is an error performing the registration. Check the 'errcode' property for more information on the reason for failure Returns: - defer.Deferred[str]: user_id + user_id """ return defer.ensureDeferred( self._hs.get_registration_handler().register_user( @@ -536,20 +554,25 @@ def register_user( ) ) - def register_device(self, user_id, device_id=None, initial_display_name=None): + def register_device( + self, + user_id: str, + device_id: Optional[str] = None, + initial_display_name: Optional[str] = None, + ) -> "defer.Deferred[Tuple[str, str, Optional[int], Optional[str]]]": """Register a device for a user and generate an access token. Added in Synapse v1.2.0. Args: - user_id (str): full canonical @user:id - device_id (str|None): The device ID to check, or None to generate + user_id: full canonical @user:id + device_id: The device ID to check, or None to generate a new one. - initial_display_name (str|None): An optional display name for the + initial_display_name: An optional display name for the device. Returns: - defer.Deferred[tuple[str, str]]: Tuple of device ID and access token + Tuple of device ID, access token, access token expiration time and refresh token """ return defer.ensureDeferred( self._hs.get_registration_handler().register_device( @@ -603,7 +626,9 @@ def generate_short_term_login_token( ) @defer.inlineCallbacks - def invalidate_access_token(self, access_token): + def invalidate_access_token( + self, access_token: str + ) -> Generator["defer.Deferred[Any]", Any, None]: """Invalidate an access token for a user Added in Synapse v0.25.0. @@ -635,14 +660,20 @@ def invalidate_access_token(self, access_token): self._auth_handler.delete_access_token(access_token) ) - def run_db_interaction(self, desc, func, *args, **kwargs): + def run_db_interaction( + self, + desc: str, + func: Callable[..., T], + *args: Any, + **kwargs: Any, + ) -> "defer.Deferred[T]": """Run a function with a database connection Added in Synapse v0.25.0. Args: - desc (str): description for the transaction, for metrics etc - func (func): function to be run. Passed a database cursor object + desc: description for the transaction, for metrics etc + func: function to be run. Passed a database cursor object as well as *args and **kwargs *args: positional args to be passed to func **kwargs: named args to be passed to func @@ -656,7 +687,7 @@ def run_db_interaction(self, desc, func, *args, **kwargs): def complete_sso_login( self, registered_user_id: str, request: SynapseRequest, client_redirect_url: str - ): + ) -> None: """Complete a SSO login by redirecting the user to a page to confirm whether they want their access token sent to `client_redirect_url`, or redirect them to that URL with a token directly if the URL matches with one of the whitelisted clients. @@ -686,7 +717,7 @@ async def complete_sso_login_async( client_redirect_url: str, new_user: bool = False, auth_provider_id: str = "", - ): + ) -> None: """Complete a SSO login by redirecting the user to a page to confirm whether they want their access token sent to `client_redirect_url`, or redirect them to that URL with a token directly if the URL matches with one of the whitelisted clients. @@ -925,11 +956,11 @@ def looping_background_call( self, f: Callable, msec: float, - *args, + *args: object, desc: Optional[str] = None, run_on_all_instances: bool = False, - **kwargs, - ): + **kwargs: object, + ) -> None: """Wraps a function as a background process and calls it repeatedly. NOTE: Will only run on the instance that is configured to run @@ -976,7 +1007,7 @@ async def send_mail( subject: str, html: str, text: str, - ): + ) -> None: """Send an email on behalf of the homeserver. Added in Synapse v1.39.0. From ab3904b89345cea2f7ef327437f4e51bd9f063c5 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 26 Nov 2021 18:37:41 +0000 Subject: [PATCH 2/3] Ch-ch-ch-ch changelog turn and face `git blame` --- changelog.d/11029.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11029.misc diff --git a/changelog.d/11029.misc b/changelog.d/11029.misc new file mode 100644 index 000000000000..111de5fc7aeb --- /dev/null +++ b/changelog.d/11029.misc @@ -0,0 +1 @@ +Improve type annotations in `synapse.module_api`. \ No newline at end of file From a694057953a3c6fe50823312fcaf7a6f9a620c8e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 29 Nov 2021 10:56:46 +0000 Subject: [PATCH 3/3] Remove redundant type alias --- synapse/module_api/__init__.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index bd524d0ffab8..19e570ede28e 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -150,12 +150,6 @@ class UserIpAndAgent: last_seen: int -# This is sad: I'd like to express that the arguments to this Callable are -# themselves callable. "Callback protocols" let you do this, but I couldn't find -# a way to do so that mypy agreed was type safe. -RegisterCallbacks = Callable[..., None] - - class ModuleApi: """A proxy object that gets passed to various plugin modules so they can register new users etc if necessary.