From 3804007791da8df258e14ec9a95186474e63aa70 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 23 May 2023 13:45:44 -0400 Subject: [PATCH 1/8] Reorganize. --- mypy.ini | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mypy.ini b/mypy.ini index a7ec66196d41..b1042332b7f5 100644 --- a/mypy.ini +++ b/mypy.ini @@ -2,17 +2,19 @@ namespace_packages = True plugins = pydantic.mypy, mypy_zope:plugin, scripts-dev/mypy_synapse_plugin.py follow_imports = normal -check_untyped_defs = True show_error_codes = True show_traceback = True mypy_path = stubs warn_unreachable = True -warn_unused_ignores = True local_partial_types = True no_implicit_optional = True + +# Strict checks, see mypy --help disallow_untyped_defs = True -strict_equality = True warn_redundant_casts = True +warn_unused_ignores = True +strict_equality = True + # Run mypy type checking with the minimum supported Python version to catch new usage # that isn't backwards-compatible (types, overloads, etc). python_version = 3.8 From 392f00fa0e121b2da35e7cba06c7fc19164e83cd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 May 2023 15:07:21 -0400 Subject: [PATCH 2/8] Enable warn_unused_configs. --- mypy.ini | 1 + synapse/federation/federation_server.py | 4 ++-- synapse/handlers/pagination.py | 4 ++-- synapse/http/server.py | 14 +++++++------- synapse/util/__init__.py | 4 ++-- synapse/util/async_helpers.py | 2 +- synapse/util/caches/lrucache.py | 6 ++---- tests/server.py | 2 +- 8 files changed, 18 insertions(+), 19 deletions(-) diff --git a/mypy.ini b/mypy.ini index b1042332b7f5..fb41e75dcff3 100644 --- a/mypy.ini +++ b/mypy.ini @@ -10,6 +10,7 @@ local_partial_types = True no_implicit_optional = True # Strict checks, see mypy --help +warn_unused_configs = True disallow_untyped_defs = True warn_redundant_casts = True warn_unused_ignores = True diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index e17cb840de99..149351dda025 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -515,7 +515,7 @@ async def process_pdu(pdu: EventBase) -> JsonDict: logger.error( "Failed to handle PDU %s", event_id, - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore + exc_info=(f.type, f.value, f.getTracebackObject()), ) return {"error": str(e)} @@ -1247,7 +1247,7 @@ async def _process_incoming_pdus_in_room_inner( logger.error( "Failed to handle PDU %s", event.event_id, - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore + exc_info=(f.type, f.value, f.getTracebackObject()), ) received_ts = await self.store.remove_received_event_from_staging( diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 63b35c8d621f..d5257acb7da3 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -360,7 +360,7 @@ async def _purge_history( except Exception: f = Failure() logger.error( - "[purge] failed", exc_info=(f.type, f.value, f.getTracebackObject()) # type: ignore + "[purge] failed", exc_info=(f.type, f.value, f.getTracebackObject()) ) self._purges_by_id[purge_id].status = PurgeStatus.STATUS_FAILED self._purges_by_id[purge_id].error = f.getErrorMessage() @@ -689,7 +689,7 @@ async def _shutdown_and_purge_room( f = Failure() logger.error( "failed", - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore + exc_info=(f.type, f.value, f.getTracebackObject()), ) self._delete_by_id[delete_id].status = DeleteStatus.STATUS_FAILED self._delete_by_id[delete_id].error = f.getErrorMessage() diff --git a/synapse/http/server.py b/synapse/http/server.py index 04768c6a237f..933172c87327 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -108,7 +108,7 @@ def return_json_error( if f.check(SynapseError): # mypy doesn't understand that f.check asserts the type. - exc: SynapseError = f.value # type: ignore + exc: SynapseError = f.value error_code = exc.code error_dict = exc.error_dict(config) if exc.headers is not None: @@ -124,7 +124,7 @@ def return_json_error( "Got cancellation before client disconnection from %r: %r", request.request_metrics.name, request, - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore[arg-type] + exc_info=(f.type, f.value, f.getTracebackObject()), ) else: error_code = 500 @@ -134,7 +134,7 @@ def return_json_error( "Failed handle request via %r: %r", request.request_metrics.name, request, - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore[arg-type] + exc_info=(f.type, f.value, f.getTracebackObject()), ) # Only respond with an error response if we haven't already started writing, @@ -172,7 +172,7 @@ def return_html_error( """ if f.check(CodeMessageException): # mypy doesn't understand that f.check asserts the type. - cme: CodeMessageException = f.value # type: ignore + cme: CodeMessageException = f.value code = cme.code msg = cme.msg if cme.headers is not None: @@ -189,7 +189,7 @@ def return_html_error( logger.error( "Failed handle request %r", request, - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore[arg-type] + exc_info=(f.type, f.value, f.getTracebackObject()), ) elif f.check(CancelledError): code = HTTP_STATUS_REQUEST_CANCELLED @@ -199,7 +199,7 @@ def return_html_error( logger.error( "Got cancellation before client disconnection when handling request %r", request, - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore[arg-type] + exc_info=(f.type, f.value, f.getTracebackObject()), ) else: code = HTTPStatus.INTERNAL_SERVER_ERROR @@ -208,7 +208,7 @@ def return_html_error( logger.error( "Failed handle request %r", request, - exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore[arg-type] + exc_info=(f.type, f.value, f.getTracebackObject()), ) if isinstance(error_template, str): diff --git a/synapse/util/__init__.py b/synapse/util/__init__.py index 9ddd26ccaa2d..7ea0c4c36bcc 100644 --- a/synapse/util/__init__.py +++ b/synapse/util/__init__.py @@ -76,7 +76,7 @@ def unwrapFirstError(failure: Failure) -> Failure: # the subFailure's value, which will do a better job of preserving stacktraces. # (actually, you probably want to use yieldable_gather_results anyway) failure.trap(defer.FirstError) - return failure.value.subFailure # type: ignore[union-attr] # Issue in Twisted's annotations + return failure.value.subFailure P = ParamSpec("P") @@ -178,7 +178,7 @@ def log_failure( """ logger.error( - msg, exc_info=(failure.type, failure.value, failure.getTracebackObject()) # type: ignore[arg-type] + msg, exc_info=(failure.type, failure.value, failure.getTracebackObject()) ) if not consumeErrors: diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index 01e3cd46f650..4041e49e71dc 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -138,7 +138,7 @@ def errback(f: Failure) -> Optional[Failure]: for observer in observers: # This is a little bit of magic to correctly propagate stack # traces when we `await` on one of the observer deferreds. - f.value.__failure__ = f # type: ignore[union-attr] + f.value.__failure__ = f try: observer.errback(f) except Exception as e: diff --git a/synapse/util/caches/lrucache.py b/synapse/util/caches/lrucache.py index 452d5d04c1c0..ed0da17227d3 100644 --- a/synapse/util/caches/lrucache.py +++ b/synapse/util/caches/lrucache.py @@ -93,10 +93,8 @@ def _get_size_of(val: Any, *, recurse: bool = True) -> int: # a general type var, distinct from either KT or VT T = TypeVar("T") -P = TypeVar("P") - -class _TimedListNode(ListNode[P]): +class _TimedListNode(ListNode[T]): """A `ListNode` that tracks last access time.""" __slots__ = ["last_access_ts_secs"] @@ -821,7 +819,7 @@ class AsyncLruCache(Generic[KT, VT]): utilize external cache systems that require await behaviour to be created. """ - def __init__(self, *args, **kwargs): # type: ignore + def __init__(self, *args: Any, **kwargs: Any): self._lru_cache: LruCache[KT, VT] = LruCache(*args, **kwargs) async def get( diff --git a/tests/server.py b/tests/server.py index 7296f0a55281..a12c3e3b9a09 100644 --- a/tests/server.py +++ b/tests/server.py @@ -642,7 +642,7 @@ def runInteraction( pool.runWithConnection = runWithConnection # type: ignore[assignment] pool.runInteraction = runInteraction # type: ignore[assignment] # Replace the thread pool with a threadless 'thread' pool - pool.threadpool = ThreadPool(clock._reactor) # type: ignore[assignment] + pool.threadpool = ThreadPool(clock._reactor) pool.running = True # We've just changed the Databases to run DB transactions on the same From 3e2e5cf36958d5cef88ea39aae59e9b40d24ff34 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 May 2023 15:17:17 -0400 Subject: [PATCH 3/8] Enable strict_concatenate. --- mypy.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/mypy.ini b/mypy.ini index fb41e75dcff3..566acdc83187 100644 --- a/mypy.ini +++ b/mypy.ini @@ -15,6 +15,7 @@ disallow_untyped_defs = True warn_redundant_casts = True warn_unused_ignores = True strict_equality = True +strict_concatenate = True # Run mypy type checking with the minimum supported Python version to catch new usage # that isn't backwards-compatible (types, overloads, etc). From d6462471338aebc7cb8b70ae6d8a29f8f6931e21 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 May 2023 15:22:25 -0400 Subject: [PATCH 4/8] Enable disallow_subclassing_any. --- mypy.ini | 1 + synapse/api/auth/msc3861_delegated.py | 2 +- synapse/handlers/oidc.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mypy.ini b/mypy.ini index 566acdc83187..a12fc5d57ba4 100644 --- a/mypy.ini +++ b/mypy.ini @@ -11,6 +11,7 @@ no_implicit_optional = True # Strict checks, see mypy --help warn_unused_configs = True +disallow_subclassing_any = True disallow_untyped_defs = True warn_redundant_casts = True warn_unused_ignores = True diff --git a/synapse/api/auth/msc3861_delegated.py b/synapse/api/auth/msc3861_delegated.py index 31c1de0119a2..bd4fc9c0ee3d 100644 --- a/synapse/api/auth/msc3861_delegated.py +++ b/synapse/api/auth/msc3861_delegated.py @@ -59,7 +59,7 @@ def scope_to_list(scope: str) -> List[str]: return scope.strip().split(" ") -class PrivateKeyJWTWithKid(PrivateKeyJWT): +class PrivateKeyJWTWithKid(PrivateKeyJWT): # type: ignore[misc] """An implementation of the private_key_jwt client auth method that includes a kid header. This is needed because some providers (Keycloak) require the kid header to figure diff --git a/synapse/handlers/oidc.py b/synapse/handlers/oidc.py index e7e0b5e049b3..24b68e03012d 100644 --- a/synapse/handlers/oidc.py +++ b/synapse/handlers/oidc.py @@ -1354,7 +1354,7 @@ async def handle_backchannel_logout( finish_request(request) -class LogoutToken(JWTClaims): +class LogoutToken(JWTClaims): # type: ignore[misc] """ Holds and verify claims of a logout token, as per https://openid.net/specs/openid-connect-backchannel-1_0.html#LogoutToken From 3cc29bf37f8647eba478fac52aa41b41b735585c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 May 2023 15:24:19 -0400 Subject: [PATCH 5/8] Enable disallow_incomplete_defs. --- mypy.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mypy.ini b/mypy.ini index a12fc5d57ba4..469ea513ffa4 100644 --- a/mypy.ini +++ b/mypy.ini @@ -13,6 +13,7 @@ no_implicit_optional = True warn_unused_configs = True disallow_subclassing_any = True disallow_untyped_defs = True +disallow_incomplete_defs = True warn_redundant_casts = True warn_unused_ignores = True strict_equality = True @@ -36,6 +37,7 @@ warn_unused_ignores = False [mypy-synapse.util.caches.treecache] disallow_untyped_defs = False +disallow_incomplete_defs = False ;; Dependencies without annotations ;; Before ignoring a module, check to see if type stubs are available. From 022f81b11284069fc747a70b610b49b419f9a882 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 May 2023 15:37:35 -0400 Subject: [PATCH 6/8] Add disabled options. --- mypy.ini | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mypy.ini b/mypy.ini index 469ea513ffa4..addb381f563e 100644 --- a/mypy.ini +++ b/mypy.ini @@ -11,11 +11,17 @@ no_implicit_optional = True # Strict checks, see mypy --help warn_unused_configs = True +# disallow_any_generics = True disallow_subclassing_any = True +# disallow_untyped_calls = True disallow_untyped_defs = True disallow_incomplete_defs = True +# check_untyped_defs = True +# disallow_untyped_decorators = True warn_redundant_casts = True warn_unused_ignores = True +# warn_return_any = True +# no_implicit_reexport = True strict_equality = True strict_concatenate = True From 9b07e20130d9cd18a589e85fa52d85b3bfc3d356 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 May 2023 15:38:23 -0400 Subject: [PATCH 7/8] Newsfragment --- changelog.d/15694.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15694.misc diff --git a/changelog.d/15694.misc b/changelog.d/15694.misc new file mode 100644 index 000000000000..93ceaeafc9b9 --- /dev/null +++ b/changelog.d/15694.misc @@ -0,0 +1 @@ +Improve type hints. From 2b9132bab061da9322fba63f9710b71221af2561 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 May 2023 15:56:43 -0400 Subject: [PATCH 8/8] Add links for missing packages. --- mypy.ini | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mypy.ini b/mypy.ini index addb381f563e..56cd1d560ea8 100644 --- a/mypy.ini +++ b/mypy.ini @@ -53,6 +53,7 @@ disallow_incomplete_defs = False ;; which we can pull in as a dev dependency by adding to `pyproject.toml`'s ;; `[tool.poetry.dev-dependencies]` list. +# https://github.com/lepture/authlib/issues/460 [mypy-authlib.*] ignore_missing_imports = True @@ -62,9 +63,11 @@ ignore_missing_imports = True [mypy-lxml] ignore_missing_imports = True +# https://github.com/msgpack/msgpack-python/issues/448 [mypy-msgpack] ignore_missing_imports = True +# https://github.com/wolever/parameterized/issues/143 [mypy-parameterized.*] ignore_missing_imports = True @@ -86,6 +89,7 @@ ignore_missing_imports = True [mypy-srvlookup.*] ignore_missing_imports = True +# https://github.com/twisted/treq/pull/366 [mypy-treq.*] ignore_missing_imports = True