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

Commit

Permalink
Bugbear: Add Mutable Parameter fixes (#9682)
Browse files Browse the repository at this point in the history
Part of #9366

Adds in fixes for B006 and B008, both relating to mutable parameter lint errors.

Signed-off-by: Jonathan de Jong <[email protected]>
  • Loading branch information
ShadowJonathan authored Apr 8, 2021
1 parent 64f4f50 commit 2ca4e34
Show file tree
Hide file tree
Showing 38 changed files with 224 additions and 113 deletions.
1 change: 1 addition & 0 deletions changelog.d/9682.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Introduce flake8-bugbear to the test suite and fix some of its lint violations.
5 changes: 4 additions & 1 deletion contrib/cmdclient/console.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import time
import urllib
from http import TwistedHttpClient
from typing import Optional

import nacl.encoding
import nacl.signing
Expand Down Expand Up @@ -718,7 +719,7 @@ def _run_and_pprint(
method,
path,
data=None,
query_params={"access_token": None},
query_params: Optional[dict] = None,
alt_text=None,
):
"""Runs an HTTP request and pretty prints the output.
Expand All @@ -729,6 +730,8 @@ def _run_and_pprint(
data: Raw JSON data if any
query_params: dict of query parameters to add to the url
"""
query_params = query_params or {"access_token": None}

url = self._url() + path
if "access_token" in query_params:
query_params["access_token"] = self._tok()
Expand Down
24 changes: 19 additions & 5 deletions contrib/cmdclient/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import json
import urllib
from pprint import pformat
from typing import Optional

from twisted.internet import defer, reactor
from twisted.web.client import Agent, readBody
Expand Down Expand Up @@ -85,8 +86,9 @@ def get_json(self, url, args=None):
body = yield readBody(response)
defer.returnValue(json.loads(body))

def _create_put_request(self, url, json_data, headers_dict={}):
def _create_put_request(self, url, json_data, headers_dict: Optional[dict] = None):
"""Wrapper of _create_request to issue a PUT request"""
headers_dict = headers_dict or {}

if "Content-Type" not in headers_dict:
raise defer.error(RuntimeError("Must include Content-Type header for PUTs"))
Expand All @@ -95,14 +97,22 @@ def _create_put_request(self, url, json_data, headers_dict={}):
"PUT", url, producer=_JsonProducer(json_data), headers_dict=headers_dict
)

def _create_get_request(self, url, headers_dict={}):
def _create_get_request(self, url, headers_dict: Optional[dict] = None):
"""Wrapper of _create_request to issue a GET request"""
return self._create_request("GET", url, headers_dict=headers_dict)
return self._create_request("GET", url, headers_dict=headers_dict or {})

@defer.inlineCallbacks
def do_request(
self, method, url, data=None, qparams=None, jsonreq=True, headers={}
self,
method,
url,
data=None,
qparams=None,
jsonreq=True,
headers: Optional[dict] = None,
):
headers = headers or {}

if qparams:
url = "%s?%s" % (url, urllib.urlencode(qparams, True))

Expand All @@ -123,8 +133,12 @@ def do_request(
defer.returnValue(json.loads(body))

@defer.inlineCallbacks
def _create_request(self, method, url, producer=None, headers_dict={}):
def _create_request(
self, method, url, producer=None, headers_dict: Optional[dict] = None
):
"""Creates and sends a request to the given url"""
headers_dict = headers_dict or {}

headers_dict["User-Agent"] = ["Synapse Cmd Client"]

retries_left = 5
Expand Down
4 changes: 2 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ ignore =
# E203: whitespace before ':' (which is contrary to pep8?)
# E731: do not assign a lambda expression, use a def
# E501: Line too long (black enforces this for us)
# B00*: Subsection of the bugbear suite (TODO: add in remaining fixes)
ignore=W503,W504,E203,E731,E501,B006,B007,B008
# B007: Subsection of the bugbear suite (TODO: add in remaining fixes)
ignore=W503,W504,E203,E731,E501,B007

[isort]
line_length = 88
Expand Down
6 changes: 3 additions & 3 deletions synapse/appservice/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
components.
"""
import logging
from typing import List
from typing import List, Optional

from synapse.appservice import ApplicationService, ApplicationServiceState
from synapse.events import EventBase
Expand Down Expand Up @@ -191,11 +191,11 @@ async def send(
self,
service: ApplicationService,
events: List[EventBase],
ephemeral: List[JsonDict] = [],
ephemeral: Optional[List[JsonDict]] = None,
):
try:
txn = await self.store.create_appservice_txn(
service=service, events=events, ephemeral=ephemeral
service=service, events=events, ephemeral=ephemeral or []
)
service_is_up = await self._is_service_up(service)
if service_is_up:
Expand Down
6 changes: 4 additions & 2 deletions synapse/config/ratelimiting.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Dict
from typing import Dict, Optional

from ._base import Config

Expand All @@ -21,8 +21,10 @@ class RateLimitConfig:
def __init__(
self,
config: Dict[str, float],
defaults={"per_second": 0.17, "burst_count": 3.0},
defaults: Optional[Dict[str, float]] = None,
):
defaults = defaults or {"per_second": 0.17, "burst_count": 3.0}

self.per_second = config.get("per_second", defaults["per_second"])
self.burst_count = int(config.get("burst_count", defaults["burst_count"]))

Expand Down
14 changes: 10 additions & 4 deletions synapse/events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,11 @@ def __init__(
self,
event_dict: JsonDict,
room_version: RoomVersion,
internal_metadata_dict: JsonDict = {},
internal_metadata_dict: Optional[JsonDict] = None,
rejected_reason: Optional[str] = None,
):
internal_metadata_dict = internal_metadata_dict or {}

event_dict = dict(event_dict)

# Signatures is a dict of dicts, and this is faster than doing a
Expand Down Expand Up @@ -386,9 +388,11 @@ def __init__(
self,
event_dict: JsonDict,
room_version: RoomVersion,
internal_metadata_dict: JsonDict = {},
internal_metadata_dict: Optional[JsonDict] = None,
rejected_reason: Optional[str] = None,
):
internal_metadata_dict = internal_metadata_dict or {}

event_dict = dict(event_dict)

# Signatures is a dict of dicts, and this is faster than doing a
Expand Down Expand Up @@ -507,9 +511,11 @@ def _event_type_from_format_version(format_version: int) -> Type[EventBase]:
def make_event_from_dict(
event_dict: JsonDict,
room_version: RoomVersion = RoomVersions.V1,
internal_metadata_dict: JsonDict = {},
internal_metadata_dict: Optional[JsonDict] = None,
rejected_reason: Optional[str] = None,
) -> EventBase:
"""Construct an EventBase from the given event dict"""
event_type = _event_type_from_format_version(room_version.event_format)
return event_type(event_dict, room_version, internal_metadata_dict, rejected_reason)
return event_type(
event_dict, room_version, internal_metadata_dict or {}, rejected_reason
)
5 changes: 3 additions & 2 deletions synapse/federation/units.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"""

import logging
from typing import Optional

import attr

Expand Down Expand Up @@ -98,7 +99,7 @@ class Transaction(JsonEncodedObject):
"pdus",
]

def __init__(self, transaction_id=None, pdus=[], **kwargs):
def __init__(self, transaction_id=None, pdus: Optional[list] = None, **kwargs):
"""If we include a list of pdus then we decode then as PDU's
automatically.
"""
Expand All @@ -107,7 +108,7 @@ def __init__(self, transaction_id=None, pdus=[], **kwargs):
if "edus" in kwargs and not kwargs["edus"]:
del kwargs["edus"]

super().__init__(transaction_id=transaction_id, pdus=pdus, **kwargs)
super().__init__(transaction_id=transaction_id, pdus=pdus or [], **kwargs)

@staticmethod
def create_new(pdus, **kwargs):
Expand Down
4 changes: 2 additions & 2 deletions synapse/handlers/appservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def notify_interested_services_ephemeral(
self,
stream_key: str,
new_token: Optional[int],
users: Collection[Union[str, UserID]] = [],
users: Optional[Collection[Union[str, UserID]]] = None,
):
"""This is called by the notifier in the background
when a ephemeral event handled by the homeserver.
Expand Down Expand Up @@ -215,7 +215,7 @@ def notify_interested_services_ephemeral(
# We only start a new background process if necessary rather than
# optimistically (to cut down on overhead).
self._notify_interested_services_ephemeral(
services, stream_key, new_token, users
services, stream_key, new_token, users or []
)

@wrap_as_background_process("notify_interested_services_ephemeral")
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,7 @@ async def _make_and_verify_event(
room_id: str,
user_id: str,
membership: str,
content: JsonDict = {},
content: JsonDict,
params: Optional[Dict[str, Union[str, Iterable[str]]]] = None,
) -> Tuple[str, EventBase, RoomVersion]:
(
Expand Down
11 changes: 8 additions & 3 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async def get_state_events(
self,
user_id: str,
room_id: str,
state_filter: StateFilter = StateFilter.all(),
state_filter: Optional[StateFilter] = None,
at_token: Optional[StreamToken] = None,
is_guest: bool = False,
) -> List[dict]:
Expand All @@ -164,6 +164,8 @@ async def get_state_events(
AuthError (403) if the user doesn't have permission to view
members of this room.
"""
state_filter = state_filter or StateFilter.all()

if at_token:
# FIXME this claims to get the state at a stream position, but
# get_recent_events_for_room operates by topo ordering. This therefore
Expand Down Expand Up @@ -874,7 +876,7 @@ async def handle_new_client_event(
event: EventBase,
context: EventContext,
ratelimit: bool = True,
extra_users: List[UserID] = [],
extra_users: Optional[List[UserID]] = None,
ignore_shadow_ban: bool = False,
) -> EventBase:
"""Processes a new event.
Expand Down Expand Up @@ -902,6 +904,7 @@ async def handle_new_client_event(
Raises:
ShadowBanError if the requester has been shadow-banned.
"""
extra_users = extra_users or []

# we don't apply shadow-banning to membership events here. Invites are blocked
# higher up the stack, and we allow shadow-banned users to send join and leave
Expand Down Expand Up @@ -1071,7 +1074,7 @@ async def persist_and_notify_client_event(
event: EventBase,
context: EventContext,
ratelimit: bool = True,
extra_users: List[UserID] = [],
extra_users: Optional[List[UserID]] = None,
) -> EventBase:
"""Called when we have fully built the event, have already
calculated the push actions for the event, and checked auth.
Expand All @@ -1083,6 +1086,8 @@ async def persist_and_notify_client_event(
it was de-duplicated (e.g. because we had already persisted an
event with the same transaction ID.)
"""
extra_users = extra_users or []

assert self.storage.persistence is not None
assert self._events_shard_config.should_handle(
self._instance_name, event.room_id
Expand Down
4 changes: 3 additions & 1 deletion synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ async def register_user(
user_type: Optional[str] = None,
default_display_name: Optional[str] = None,
address: Optional[str] = None,
bind_emails: Iterable[str] = [],
bind_emails: Optional[Iterable[str]] = None,
by_admin: bool = False,
user_agent_ips: Optional[List[Tuple[str, str]]] = None,
auth_provider_id: Optional[str] = None,
Expand Down Expand Up @@ -204,6 +204,8 @@ async def register_user(
Raises:
SynapseError if there was a problem registering.
"""
bind_emails = bind_emails or []

await self.check_registration_ratelimit(address)

result = await self.spam_checker.check_registration_for_spam(
Expand Down
8 changes: 4 additions & 4 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ async def _load_filtered_recents(
)

async def get_state_after_event(
self, event: EventBase, state_filter: StateFilter = StateFilter.all()
self, event: EventBase, state_filter: Optional[StateFilter] = None
) -> StateMap[str]:
"""
Get the room state after the given event
Expand All @@ -558,7 +558,7 @@ async def get_state_after_event(
state_filter: The state filter used to fetch state from the database.
"""
state_ids = await self.state_store.get_state_ids_for_event(
event.event_id, state_filter=state_filter
event.event_id, state_filter=state_filter or StateFilter.all()
)
if event.is_state():
state_ids = dict(state_ids)
Expand All @@ -569,7 +569,7 @@ async def get_state_at(
self,
room_id: str,
stream_position: StreamToken,
state_filter: StateFilter = StateFilter.all(),
state_filter: Optional[StateFilter] = None,
) -> StateMap[str]:
"""Get the room state at a particular stream position
Expand All @@ -589,7 +589,7 @@ async def get_state_at(
if last_events:
last_event = last_events[-1]
state = await self.get_state_after_event(
last_event, state_filter=state_filter
last_event, state_filter=state_filter or StateFilter.all()
)

else:
Expand Down
4 changes: 2 additions & 2 deletions synapse/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ class SimpleHttpClient:
def __init__(
self,
hs: "HomeServer",
treq_args: Dict[str, Any] = {},
treq_args: Optional[Dict[str, Any]] = None,
ip_whitelist: Optional[IPSet] = None,
ip_blacklist: Optional[IPSet] = None,
use_proxy: bool = False,
Expand All @@ -317,7 +317,7 @@ def __init__(

self._ip_whitelist = ip_whitelist
self._ip_blacklist = ip_blacklist
self._extra_treq_args = treq_args
self._extra_treq_args = treq_args or {}

self.user_agent = hs.version_string
self.clock = hs.get_clock()
Expand Down
6 changes: 4 additions & 2 deletions synapse/http/proxyagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from twisted.web.client import URI, BrowserLikePolicyForHTTPS, _AgentBase
from twisted.web.error import SchemeNotSupported
from twisted.web.http_headers import Headers
from twisted.web.iweb import IAgent
from twisted.web.iweb import IAgent, IPolicyForHTTPS

from synapse.http.connectproxyclient import HTTPConnectProxyEndpoint

Expand Down Expand Up @@ -88,12 +88,14 @@ def __init__(
self,
reactor,
proxy_reactor=None,
contextFactory=BrowserLikePolicyForHTTPS(),
contextFactory: Optional[IPolicyForHTTPS] = None,
connectTimeout=None,
bindAddress=None,
pool=None,
use_proxy=False,
):
contextFactory = contextFactory or BrowserLikePolicyForHTTPS()

_AgentBase.__init__(self, reactor, pool)

if proxy_reactor is None:
Expand Down
Loading

0 comments on commit 2ca4e34

Please sign in to comment.