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

Commit

Permalink
Enable mypy checking for unreachable code and fix instances. (#8432)
Browse files Browse the repository at this point in the history
  • Loading branch information
clokep authored Oct 1, 2020
1 parent c1ef579 commit 4ff0201
Show file tree
Hide file tree
Showing 17 changed files with 38 additions and 53 deletions.
1 change: 1 addition & 0 deletions changelog.d/8432.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Check for unreachable code with mypy.
1 change: 1 addition & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ check_untyped_defs = True
show_error_codes = True
show_traceback = True
mypy_path = stubs
warn_unreachable = True
files =
synapse/api,
synapse/appservice,
Expand Down
18 changes: 9 additions & 9 deletions synapse/config/tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import warnings
from datetime import datetime
from hashlib import sha256
from typing import List
from typing import List, Optional

from unpaddedbase64 import encode_base64

Expand Down Expand Up @@ -177,8 +177,8 @@ def read_config(self, config: dict, config_dir_path: str, **kwargs):
"use_insecure_ssl_client_just_for_testing_do_not_use"
)

self.tls_certificate = None
self.tls_private_key = None
self.tls_certificate = None # type: Optional[crypto.X509]
self.tls_private_key = None # type: Optional[crypto.PKey]

def is_disk_cert_valid(self, allow_self_signed=True):
"""
Expand Down Expand Up @@ -226,12 +226,12 @@ def is_disk_cert_valid(self, allow_self_signed=True):
days_remaining = (expires_on - now).days
return days_remaining

def read_certificate_from_disk(self, require_cert_and_key):
def read_certificate_from_disk(self, require_cert_and_key: bool):
"""
Read the certificates and private key from disk.
Args:
require_cert_and_key (bool): set to True to throw an error if the certificate
require_cert_and_key: set to True to throw an error if the certificate
and key file are not given
"""
if require_cert_and_key:
Expand Down Expand Up @@ -479,13 +479,13 @@ def generate_config_section(
}
)

def read_tls_certificate(self):
def read_tls_certificate(self) -> crypto.X509:
"""Reads the TLS certificate from the configured file, and returns it
Also checks if it is self-signed, and warns if so
Returns:
OpenSSL.crypto.X509: the certificate
The certificate
"""
cert_path = self.tls_certificate_file
logger.info("Loading TLS certificate from %s", cert_path)
Expand All @@ -504,11 +504,11 @@ def read_tls_certificate(self):

return cert

def read_tls_private_key(self):
def read_tls_private_key(self) -> crypto.PKey:
"""Reads the TLS private key from the configured file, and returns it
Returns:
OpenSSL.crypto.PKey: the private key
The private key
"""
private_key_path = self.tls_private_key_file
logger.info("Loading TLS key from %s", private_key_path)
Expand Down
5 changes: 2 additions & 3 deletions synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
Callable,
Dict,
List,
Match,
Optional,
Tuple,
Union,
Expand Down Expand Up @@ -825,14 +824,14 @@ def server_matches_acl_event(server_name: str, acl_event: EventBase) -> bool:
return False


def _acl_entry_matches(server_name: str, acl_entry: str) -> Match:
def _acl_entry_matches(server_name: str, acl_entry: Any) -> bool:
if not isinstance(acl_entry, str):
logger.warning(
"Ignoring non-str ACL entry '%s' (is %s)", acl_entry, type(acl_entry)
)
return False
regex = glob_to_regex(acl_entry)
return regex.match(server_name)
return bool(regex.match(server_name))


class FederationHandlerRegistry:
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ async def _user_can_delete_alias(self, alias: RoomAlias, user_id: str):
"""
creator = await self.store.get_room_alias_creator(alias.to_string())

if creator is not None and creator == user_id:
if creator == user_id:
return True

# Resolve the alias to the corresponding room.
Expand Down
2 changes: 0 additions & 2 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,6 @@ async def _generate_room_id(
try:
random_string = stringutils.random_string(18)
gen_room_id = RoomID(random_string, self.hs.hostname).to_string()
if isinstance(gen_room_id, bytes):
gen_room_id = gen_room_id.decode("utf-8")
await self.store.store_room(
room_id=gen_room_id,
room_creator_user_id=creator_id,
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ async def copy_user_state_on_room_upgrade(

async def send_membership_event(
self,
requester: Requester,
requester: Optional[Requester],
event: EventBase,
context: EventContext,
ratelimit: bool = True,
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class SyncConfig:
class TimelineBatch:
prev_batch = attr.ib(type=StreamToken)
events = attr.ib(type=List[EventBase])
limited = attr.ib(bool)
limited = attr.ib(type=bool)

def __bool__(self) -> bool:
"""Make the result appear empty if there are no updates. This is used
Expand Down
4 changes: 2 additions & 2 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ async def _async_render(self, request: Request):
if isinstance(raw_callback_return, (defer.Deferred, types.CoroutineType)):
callback_return = await raw_callback_return
else:
callback_return = raw_callback_return
callback_return = raw_callback_return # type: ignore

return callback_return

Expand Down Expand Up @@ -406,7 +406,7 @@ async def _async_render(self, request):
if isinstance(raw_callback_return, (defer.Deferred, types.CoroutineType)):
callback_return = await raw_callback_return
else:
callback_return = raw_callback_return
callback_return = raw_callback_return # type: ignore

return callback_return

Expand Down
10 changes: 1 addition & 9 deletions synapse/logging/_structured.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import logging
import os.path
import sys
Expand Down Expand Up @@ -89,14 +88,7 @@ def __call__(self, event: dict) -> None:
context = current_context()

# Copy the context information to the log event.
if context is not None:
context.copy_to_twisted_log_entry(event)
else:
# If there's no logging context, not even the root one, we might be
# starting up or it might be from non-Synapse code. Log it as if it
# came from the root logger.
event["request"] = None
event["scope"] = None
context.copy_to_twisted_log_entry(event)

self.observer(event)

Expand Down
4 changes: 2 additions & 2 deletions synapse/push/push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import logging
import re
from typing import Any, Dict, List, Pattern, Union
from typing import Any, Dict, List, Optional, Pattern, Union

from synapse.events import EventBase
from synapse.types import UserID
Expand Down Expand Up @@ -181,7 +181,7 @@ def _contains_display_name(self, display_name: str) -> bool:

return r.search(body)

def _get_value(self, dotted_key: str) -> str:
def _get_value(self, dotted_key: str) -> Optional[str]:
return self._value_cache.get(dotted_key, None)


Expand Down
10 changes: 6 additions & 4 deletions synapse/replication/tcp/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@
import logging
import struct
from inspect import isawaitable
from typing import TYPE_CHECKING, List
from typing import TYPE_CHECKING, List, Optional

from prometheus_client import Counter

from twisted.internet import task
from twisted.protocols.basic import LineOnlyReceiver
from twisted.python.failure import Failure

Expand Down Expand Up @@ -152,9 +153,10 @@ def __init__(self, clock: Clock, handler: "ReplicationCommandHandler"):

self.last_received_command = self.clock.time_msec()
self.last_sent_command = 0
self.time_we_closed = None # When we requested the connection be closed
# When we requested the connection be closed
self.time_we_closed = None # type: Optional[int]

self.received_ping = False # Have we reecived a ping from the other side
self.received_ping = False # Have we received a ping from the other side

self.state = ConnectionStates.CONNECTING

Expand All @@ -165,7 +167,7 @@ def __init__(self, clock: Clock, handler: "ReplicationCommandHandler"):
self.pending_commands = [] # type: List[Command]

# The LoopingCall for sending pings.
self._send_ping_loop = None
self._send_ping_loop = None # type: Optional[task.LoopingCall]

# a logcontext which we use for processing incoming commands. We declare it as a
# background process so that the CPU stats get reported to prometheus.
Expand Down
2 changes: 1 addition & 1 deletion synapse/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ def _make_state_cache_entry(

# failing that, look for the closest match.
prev_group = None
delta_ids = None
delta_ids = None # type: Optional[StateMap[str]]

for old_group, old_state in state_groups_ids.items():
n_delta_ids = {k: v for k, v in new_state.items() if old_state.get(k) != v}
Expand Down
6 changes: 3 additions & 3 deletions synapse/storage/databases/main/censor_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
from synapse.storage._base import SQLBaseStore
from synapse.storage.database import DatabasePool
from synapse.storage.databases.main.cache import CacheInvalidationWorkerStore
from synapse.storage.databases.main.events import encode_json
from synapse.storage.databases.main.events_worker import EventsWorkerStore
from synapse.util.frozenutils import frozendict_json_encoder

if TYPE_CHECKING:
from synapse.server import HomeServer
Expand Down Expand Up @@ -105,7 +105,7 @@ async def _censor_redactions(self):
and original_event.internal_metadata.is_redacted()
):
# Redaction was allowed
pruned_json = encode_json(
pruned_json = frozendict_json_encoder.encode(
prune_event_dict(
original_event.room_version, original_event.get_dict()
)
Expand Down Expand Up @@ -171,7 +171,7 @@ def delete_expired_event_txn(txn):
return

# Prune the event's dict then convert it to JSON.
pruned_json = encode_json(
pruned_json = frozendict_json_encoder.encode(
prune_event_dict(event.room_version, event.get_dict())
)

Expand Down
18 changes: 5 additions & 13 deletions synapse/storage/databases/main/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,6 @@
)


def encode_json(json_object):
"""
Encode a Python object as JSON and return it in a Unicode string.
"""
out = frozendict_json_encoder.encode(json_object)
if isinstance(out, bytes):
out = out.decode("utf8")
return out


_EventCacheEntry = namedtuple("_EventCacheEntry", ("event", "redacted_event"))


Expand Down Expand Up @@ -743,7 +733,9 @@ def _update_outliers_txn(self, txn, events_and_contexts):
logger.exception("")
raise

metadata_json = encode_json(event.internal_metadata.get_dict())
metadata_json = frozendict_json_encoder.encode(
event.internal_metadata.get_dict()
)

sql = "UPDATE event_json SET internal_metadata = ? WHERE event_id = ?"
txn.execute(sql, (metadata_json, event.event_id))
Expand Down Expand Up @@ -797,10 +789,10 @@ def event_dict(event):
{
"event_id": event.event_id,
"room_id": event.room_id,
"internal_metadata": encode_json(
"internal_metadata": frozendict_json_encoder.encode(
event.internal_metadata.get_dict()
),
"json": encode_json(event_dict(event)),
"json": frozendict_json_encoder.encode(event_dict(event)),
"format_version": event.format_version,
}
for event, _ in events_and_contexts
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ async def get_recent_event_ids_for_room(

async def get_room_event_before_stream_ordering(
self, room_id: str, stream_ordering: int
) -> Tuple[int, int, str]:
) -> Optional[Tuple[int, int, str]]:
"""Gets details of the first event in a room at or before a stream ordering
Args:
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/util/id_generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ def _mark_id_as_finished(self, next_id: int):
self._unfinished_ids.discard(next_id)
self._finished_ids.add(next_id)

new_cur = None
new_cur = None # type: Optional[int]

if self._unfinished_ids:
# If there are unfinished IDs then the new position will be the
Expand Down

0 comments on commit 4ff0201

Please sign in to comment.