-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add stricter mypy options #15694
Add stricter mypy options #15694
Changes from all commits
3804007
392f00f
3e2e5cf
d646247
3cc29bf
022f81b
9b07e20
2b9132b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Improve type hints. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,17 +2,29 @@ | |
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 | ||
warn_unused_configs = True | ||
# disallow_any_generics = True | ||
disallow_subclassing_any = True | ||
# disallow_untyped_calls = True | ||
disallow_untyped_defs = True | ||
strict_equality = True | ||
disallow_incomplete_defs = True | ||
# check_untyped_defs = True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised---I thought we had this option enabled! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This keeps confusing me, but |
||
# 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 | ||
|
||
# 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 | ||
|
@@ -31,6 +43,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. | ||
|
@@ -40,6 +53,7 @@ disallow_untyped_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 | ||
|
||
|
@@ -49,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 | ||
|
||
|
@@ -73,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 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'd need stubs/annotations for authlib for this to go away, I think? Ditto for LogoutToken. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I figured since there was only 2 instances it was better to enable the option to ignore it twice? |
||
"""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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really see how this is relevant to warn_unused_configs: https://mypy.readthedocs.io/en/stable/config_file.html#confval-warn_unused_configs makes it sound like the option tells you when you've screwed up your config. (Happy to remove any redundant type-ignores though.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also don't really understand how it is relevant, but when I enabled that option it gave errors on these lines? 🤷 |
||
) | ||
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( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it useful to list the disabled ones? I think so, but curious of what others think.