Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Disallow untyped defs in synapse._scripts (#12422)
Browse files Browse the repository at this point in the history
Of note: 

* No untyped defs in `register_new_matrix_user`

This one might be contraversial. `request_registration` has three
dependency-injection arguments used for testing. I'm removing the
injection of the `requests` module and using `unitest.mock.patch` in the
test cases instead.

Doing `reveal_type(requests)` and `reveal_type(requests.get)` before the
change:

```
synapse/_scripts/register_new_matrix_user.py:45: note: Revealed type is "Any"
synapse/_scripts/register_new_matrix_user.py:46: note: Revealed type is "Any"
```

And after:

```
synapse/_scripts/register_new_matrix_user.py:44: note: Revealed type is "types.ModuleType"
synapse/_scripts/register_new_matrix_user.py:45: note: Revealed type is "def (url: Union[builtins.str, builtins.bytes], params: Union[Union[_typeshed.SupportsItems[Union[builtins.str, builtins.bytes, builtins.int, builtins.float], Union[builtins.str, builtins.bytes, builtins.int, builtins.float, typing.Iterable[Union[builtins.str, builtins.bytes, builtins.int, builtins.float]], None]], Tuple[Union[builtins.str, builtins.bytes, builtins.int, builtins.float], Union[builtins.str, builtins.bytes, builtins.int, builtins.float, typing.Iterable[Union[builtins.str, builtins.bytes, builtins.int, builtins.float]], None]], typing.Iterable[Tuple[Union[builtins.str, builtins.bytes, builtins.int, builtins.float], Union[builtins.str, builtins.bytes, builtins.int, builtins.float, typing.Iterable[Union[builtins.str, builtins.bytes, builtins.int, builtins.float]], None]]], builtins.str, builtins.bytes], None] =, data: Union[Any, None] =, headers: Union[Any, None] =, cookies: Union[Any, None] =, files: Union[Any, None] =, auth: Union[Any, None] =, timeout: Union[Any, None] =, allow_redirects: builtins.bool =, proxies: Union[Any, None] =, hooks: Union[Any, None] =, stream: Union[Any, None] =, verify: Union[Any, None] =, cert: Union[Any, None] =, json: Union[Any, None] =) -> requests.models.Response"
```

* Drive-by comment in `synapse.storage.types`

* No untyped defs in `synapse_port_db`

This was by far the most painful. I'm happy to break this up into
smaller pieces for review if it's not managable as-is.
  • Loading branch information
David Robertson committed Apr 11, 2022
1 parent 5f72ea1 commit 961ee75
Show file tree
Hide file tree
Showing 14 changed files with 221 additions and 140 deletions.
1 change: 1 addition & 0 deletions changelog.d/12422.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make `synapse._scripts` pass type checks.
3 changes: 3 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ exclude = (?x)
|tests/utils.py
)$

[mypy-synapse._scripts.*]
disallow_untyped_defs = True

[mypy-synapse.api.*]
disallow_untyped_defs = True

Expand Down
11 changes: 5 additions & 6 deletions synapse/_scripts/export_signing_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@
import argparse
import sys
import time
from typing import Optional
from typing import NoReturn, Optional

from signedjson.key import encode_verify_key_base64, get_verify_key, read_signing_keys
from signedjson.types import VerifyKey


def exit(status: int = 0, message: Optional[str] = None):
def exit(status: int = 0, message: Optional[str] = None) -> NoReturn:
if message:
print(message, file=sys.stderr)
sys.exit(status)


def format_plain(public_key: VerifyKey):
def format_plain(public_key: VerifyKey) -> None:
print(
"%s:%s %s"
% (
Expand All @@ -38,7 +38,7 @@ def format_plain(public_key: VerifyKey):
)


def format_for_config(public_key: VerifyKey, expiry_ts: int):
def format_for_config(public_key: VerifyKey, expiry_ts: int) -> None:
print(
' "%s:%s": { key: "%s", expired_ts: %i }'
% (
Expand All @@ -50,7 +50,7 @@ def format_for_config(public_key: VerifyKey, expiry_ts: int):
)


def main():
def main() -> None:
parser = argparse.ArgumentParser()

parser.add_argument(
Expand Down Expand Up @@ -94,7 +94,6 @@ def main():
message="Error reading key from file %s: %s %s"
% (file.name, type(e), e),
)
res = []
for key in res:
formatter(get_verify_key(key))

Expand Down
2 changes: 1 addition & 1 deletion synapse/_scripts/generate_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from synapse.config.homeserver import HomeServerConfig


def main():
def main() -> None:
parser = argparse.ArgumentParser()
parser.add_argument(
"--config-dir",
Expand Down
2 changes: 1 addition & 1 deletion synapse/_scripts/generate_log_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from synapse.config.logger import DEFAULT_LOG_CONFIG


def main():
def main() -> None:
parser = argparse.ArgumentParser()

parser.add_argument(
Expand Down
2 changes: 1 addition & 1 deletion synapse/_scripts/generate_signing_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from synapse.util.stringutils import random_string


def main():
def main() -> None:
parser = argparse.ArgumentParser()

parser.add_argument(
Expand Down
4 changes: 2 additions & 2 deletions synapse/_scripts/hash_password.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import yaml


def prompt_for_pass():
def prompt_for_pass() -> str:
password = getpass.getpass("Password: ")

if not password:
Expand All @@ -23,7 +23,7 @@ def prompt_for_pass():
return password


def main():
def main() -> None:
bcrypt_rounds = 12
password_pepper = ""

Expand Down
19 changes: 12 additions & 7 deletions synapse/_scripts/move_remote_media_to_new_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
logger = logging.getLogger()


def main(src_repo, dest_repo):
def main(src_repo: str, dest_repo: str) -> None:
src_paths = MediaFilePaths(src_repo)
dest_paths = MediaFilePaths(dest_repo)
for line in sys.stdin:
Expand All @@ -55,14 +55,19 @@ def main(src_repo, dest_repo):
move_media(parts[0], parts[1], src_paths, dest_paths)


def move_media(origin_server, file_id, src_paths, dest_paths):
def move_media(
origin_server: str,
file_id: str,
src_paths: MediaFilePaths,
dest_paths: MediaFilePaths,
) -> None:
"""Move the given file, and any thumbnails, to the dest repo
Args:
origin_server (str):
file_id (str):
src_paths (MediaFilePaths):
dest_paths (MediaFilePaths):
origin_server:
file_id:
src_paths:
dest_paths:
"""
logger.info("%s/%s", origin_server, file_id)

Expand Down Expand Up @@ -91,7 +96,7 @@ def move_media(origin_server, file_id, src_paths, dest_paths):
)


def mkdir_and_move(original_file, dest_file):
def mkdir_and_move(original_file: str, dest_file: str) -> None:
dirname = os.path.dirname(dest_file)
if not os.path.exists(dirname):
logger.debug("mkdir %s", dirname)
Expand Down
3 changes: 1 addition & 2 deletions synapse/_scripts/register_new_matrix_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import sys
from typing import Callable, Optional

import requests as _requests
import requests
import yaml


Expand All @@ -33,7 +33,6 @@ def request_registration(
shared_secret: str,
admin: bool = False,
user_type: Optional[str] = None,
requests=_requests,
_print: Callable[[str], None] = print,
exit: Callable[[int], None] = sys.exit,
) -> None:
Expand Down

0 comments on commit 961ee75

Please sign in to comment.