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

Upgrade mypy to version 0.931 #12030

Merged
merged 4 commits into from
Feb 18, 2022
Merged

Upgrade mypy to version 0.931 #12030

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4396368
Upgrade mypy to 0.931 and mypy-zope to 0.3.5
Feb 17, 2022
4ba686d
Make mypy happy again
Feb 17, 2022
dd59860
Add newsfile
Feb 18, 2022
5330353
Use `attrs.fields()` instead of reaching into private fields
Feb 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/12030.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Upgrade mypy to version 0.931.
4 changes: 2 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ def exec_file(path_segments):
]

CONDITIONAL_REQUIREMENTS["mypy"] = [
"mypy==0.910",
"mypy-zope==0.3.2",
"mypy==0.931",
"mypy-zope==0.3.5",
"types-bleach>=4.1.0",
"types-jsonschema>=3.2.0",
"types-opentracing>=2.4.2",
Expand Down
13 changes: 9 additions & 4 deletions stubs/sortedcontainers/sorteddict.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,18 @@ class SortedDict(Dict[_KT, _VT]):
def __copy__(self: _SD) -> _SD: ...
@classmethod
@overload
def fromkeys(cls, seq: Iterable[_T_h]) -> SortedDict[_T_h, None]: ...
def fromkeys(
cls, seq: Iterable[_T_h], value: None = ...
) -> SortedDict[_T_h, None]: ...
@classmethod
@overload
def fromkeys(cls, seq: Iterable[_T_h], value: _S) -> SortedDict[_T_h, _S]: ...
def keys(self) -> SortedKeysView[_KT]: ...
def items(self) -> SortedItemsView[_KT, _VT]: ...
def values(self) -> SortedValuesView[_VT]: ...
# As of Python 3.10, `dict_{keys,items,values}` have an extra `mapping` attribute and so
# `Sorted{Keys,Items,Values}View` are no longer compatible with them.
# See https://github.com/python/typeshed/issues/6837
def keys(self) -> SortedKeysView[_KT]: ... # type: ignore[override]
def items(self) -> SortedItemsView[_KT, _VT]: ... # type: ignore[override]
def values(self) -> SortedValuesView[_VT]: ... # type: ignore[override]
@overload
def pop(self, key: _KT) -> _VT: ...
@overload
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ async def _calculate_event_contexts(
self.storage, user.to_string(), res.events_after
)

context = {
context: JsonDict = {
"events_before": events_before,
"events_after": events_after,
"start": await now_token.copy_and_replace(
Expand Down
10 changes: 6 additions & 4 deletions synapse/push/baserules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ def make_base_prepend_rules(
return rules


BASE_APPEND_CONTENT_RULES = [
# We have to annotate these types, otherwise mypy infers them as
# `List[Dict[str, Sequence[Collection[str]]]]`.
BASE_APPEND_CONTENT_RULES: List[Dict[str, Any]] = [
{
"rule_id": "global/content/.m.rule.contains_user_name",
"conditions": [
Expand All @@ -149,7 +151,7 @@ def make_base_prepend_rules(
]


BASE_PREPEND_OVERRIDE_RULES = [
BASE_PREPEND_OVERRIDE_RULES: List[Dict[str, Any]] = [
{
"rule_id": "global/override/.m.rule.master",
"enabled": False,
Expand All @@ -159,7 +161,7 @@ def make_base_prepend_rules(
]


BASE_APPEND_OVERRIDE_RULES = [
BASE_APPEND_OVERRIDE_RULES: List[Dict[str, Any]] = [
{
"rule_id": "global/override/.m.rule.suppress_notices",
"conditions": [
Expand Down Expand Up @@ -278,7 +280,7 @@ def make_base_prepend_rules(
]


BASE_APPEND_UNDERRIDE_RULES = [
BASE_APPEND_UNDERRIDE_RULES: List[Dict[str, Any]] = [
{
"rule_id": "global/underride/.m.rule.call",
"conditions": [
Expand Down
2 changes: 1 addition & 1 deletion synapse/push/httppusher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ async def _build_notification_dict(
# This was checked in the __init__, but mypy doesn't seem to know that.
assert self.data is not None
if self.data.get("format") == "event_id_only":
d = {
d: Dict[str, Any] = {
"notification": {
"event_id": event.event_id,
"room_id": event.room_id,
Expand Down
4 changes: 2 additions & 2 deletions synapse/streams/events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class _EventSourcesInner:
account_data: AccountDataEventSource

def get_sources(self) -> Iterator[Tuple[str, EventSource]]:
for attribute in _EventSourcesInner.__attrs_attrs__: # type: ignore[attr-defined]
yield attribute.name, getattr(self, attribute.name)
for attribute in _EventSourcesInner.__attrs_attrs__:
yield attribute.name, getattr(self, attribute.name) # type: ignore[attr-defined]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous version of mypy didn't think .__attrs_attrs__ was a real attribute.
mypy 0.931 thinks it is, and thinks that attribute is an object, which doesn't have a .name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would mypy be happier with attrs.fields_dict here? Requires attrs 18.1.

(Probably fine as-is though.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a little cleaner, thanks! We already require attrs>=19.2.0 so it's fine to use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still looks a bit odd to have the getattr call. Maybe get_sources could boil down to yield from attrs.asdict(self).items()? Actually, I think not: looks like one is supposed to inherit from _EventSourcesInner?

I'm happy with this as it stands.



class EventSources:
Expand Down
8 changes: 5 additions & 3 deletions synapse/util/daemonize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import signal
import sys
from types import FrameType, TracebackType
from typing import NoReturn, Type
from typing import NoReturn, Optional, Type


def daemonize_process(pid_file: str, logger: logging.Logger, chdir: str = "/") -> None:
Expand Down Expand Up @@ -100,7 +100,9 @@ def daemonize_process(pid_file: str, logger: logging.Logger, chdir: str = "/") -
# also catch any other uncaught exceptions before we get that far.)

def excepthook(
type_: Type[BaseException], value: BaseException, traceback: TracebackType
type_: Type[BaseException],
value: BaseException,
traceback: Optional[TracebackType],
Comment on lines +103 to +105
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to https://github.com/python/typeshed/pull/6678/files?

) -> None:
logger.critical("Unhanded exception", exc_info=(type_, value, traceback))

Expand All @@ -123,7 +125,7 @@ def excepthook(
sys.exit(1)

# write a log line on SIGTERM.
def sigterm(signum: signal.Signals, frame: FrameType) -> NoReturn:
def sigterm(signum: int, frame: Optional[FrameType]) -> NoReturn:
Copy link
Contributor Author

@squahtx squahtx Feb 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to https://github.com/python/typeshed/pull/5622/files?

logger.warning("Caught signal %s. Stopping daemon." % signum)
sys.exit(0)

Expand Down
6 changes: 4 additions & 2 deletions synapse/util/patch_inline_callbacks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import functools
import sys
from typing import Any, Callable, Generator, List, TypeVar
from typing import Any, Callable, Generator, List, TypeVar, cast

from twisted.internet import defer
from twisted.internet.defer import Deferred
Expand Down Expand Up @@ -174,7 +174,9 @@ def check_yield_points_inner(
)
)
changes.append(err)
return getattr(e, "value", None)
# The `StopIteration` or `_DefGen_Return` contains the return value from the
# generator.
return cast(T, e.value)
Comment on lines -177 to +179
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is e.value always guaranteed to be present? I.e. was the fallback of None redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of the exception types we catch here have a value attribute:

class StopIteration(Exception):
    value: Any

class _DefGen_Return(BaseException):
    def __init__(self, value: object) -> None:
        self.value = value

Since the exception comes from the generator gen, which is supposedly a Generator[..., ..., T], the StopIteration ought to contain a T.
I can't say the same for _DefGen_Return because I have no idea what its purpose is. I'm guessing it works the same way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of the exception types we catch here have a value attribute:

class StopIteration(Exception):
    value: Any

class _DefGen_Return(BaseException):
    def __init__(self, value: object) -> None:
        self.value = value

Since the exception comes from the generator gen, which is supposedly a Generator[..., ..., T], the StopIteration ought to contain a T. I can't say the same for _DefGen_Return because I have no idea what its purpose is. I'm guessing it works the same way.

Seems plausible: https://github.com/twisted/twisted/blob/5c24e99e671c4082a1ddc8dbeb869402294bd0dc/src/twisted/internet/defer.py#L1508-L1521


frame = gen.gi_frame

Expand Down