From 5da5a82a8cc36399c7dbc228109afee2a17ff21e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 17 Apr 2023 12:02:04 -0400 Subject: [PATCH] Get rid of default mutable arguments. --- .ruff.toml | 2 ++ src/allmydata/client.py | 6 +++--- src/allmydata/dirnode.py | 4 +++- src/allmydata/hashtree.py | 7 +++++-- src/allmydata/immutable/upload.py | 4 +++- src/allmydata/interfaces.py | 6 +++--- src/allmydata/node.py | 12 ++++++++---- src/allmydata/nodemaker.py | 5 +++-- src/allmydata/test/cli/wormholetesting.py | 3 ++- src/allmydata/test/no_network.py | 4 +++- src/allmydata/test/test_system.py | 21 +++++++++------------ src/allmydata/test/web/test_web.py | 12 +++++++++--- src/allmydata/util/dbutil.py | 4 +++- 13 files changed, 56 insertions(+), 34 deletions(-) diff --git a/.ruff.toml b/.ruff.toml index 75ff62c2d..516255d2a 100644 --- a/.ruff.toml +++ b/.ruff.toml @@ -9,4 +9,6 @@ select = [ # Make sure we bind closure variables in a loop (equivalent to pylint # cell-var-from-loop): "B023", + # Don't use mutable default arguments: + "B006", ] \ No newline at end of file diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 8a10fe9e7..1d959cb98 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -7,7 +7,7 @@ import os import stat import time import weakref -from typing import Optional +from typing import Optional, Iterable from base64 import urlsafe_b64encode from functools import partial # On Python 2 this will be the backported package: @@ -189,7 +189,7 @@ class Terminator(service.Service): return service.Service.stopService(self) -def read_config(basedir, portnumfile, generated_files=[]): +def read_config(basedir, portnumfile, generated_files: Iterable=()): """ Read and validate configuration for a client-style Node. See :method:`allmydata.node.read_config` for parameter meanings (the @@ -1103,7 +1103,7 @@ class _Client(node.Node, pollmixin.PollMixin): # may get an opaque node if there were any problems. return self.nodemaker.create_from_cap(write_uri, read_uri, deep_immutable=deep_immutable, name=name) - def create_dirnode(self, initial_children={}, version=None): + def create_dirnode(self, initial_children=None, version=None): d = self.nodemaker.create_new_mutable_directory(initial_children, version=version) return d diff --git a/src/allmydata/dirnode.py b/src/allmydata/dirnode.py index fdf373b45..ccd045b05 100644 --- a/src/allmydata/dirnode.py +++ b/src/allmydata/dirnode.py @@ -678,8 +678,10 @@ class DirectoryNode(object): return d # XXX: Too many arguments? Worthwhile to break into mutable/immutable? - def create_subdirectory(self, namex, initial_children={}, overwrite=True, + def create_subdirectory(self, namex, initial_children=None, overwrite=True, mutable=True, mutable_version=None, metadata=None): + if initial_children is None: + initial_children = {} name = normalize(namex) if self.is_readonly(): return defer.fail(NotWriteableError()) diff --git a/src/allmydata/hashtree.py b/src/allmydata/hashtree.py index 17467459b..57bdbd9a1 100644 --- a/src/allmydata/hashtree.py +++ b/src/allmydata/hashtree.py @@ -332,7 +332,7 @@ class IncompleteHashTree(CompleteBinaryTreeMixin, list): name += " (leaf [%d] of %d)" % (leafnum, numleaves) return name - def set_hashes(self, hashes={}, leaves={}): + def set_hashes(self, hashes=None, leaves=None): """Add a bunch of hashes to the tree. I will validate these to the best of my ability. If I already have a @@ -382,7 +382,10 @@ class IncompleteHashTree(CompleteBinaryTreeMixin, list): corrupted or one of the received hashes was corrupted. If it raises NotEnoughHashesError, then the otherhashes dictionary was incomplete. """ - + if hashes is None: + hashes = {} + if leaves is None: + leaves = {} assert isinstance(hashes, dict) for h in hashes.values(): assert isinstance(h, bytes) diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index 0421de4e0..a331cc5db 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -1391,7 +1391,9 @@ class CHKUploader(object): def get_upload_status(self): return self._upload_status -def read_this_many_bytes(uploadable, size, prepend_data=[]): +def read_this_many_bytes(uploadable, size, prepend_data=None): + if prepend_data is None: + prepend_data = [] if size == 0: return defer.succeed([]) d = uploadable.read(size) diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 467d0d450..201ab082e 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -1447,7 +1447,7 @@ class IDirectoryNode(IFilesystemNode): is a file, or if must_be_file is True and the child is a directory, I raise ChildOfWrongTypeError.""" - def create_subdirectory(name, initial_children={}, overwrite=True, + def create_subdirectory(name, initial_children=None, overwrite=True, mutable=True, mutable_version=None, metadata=None): """I create and attach a directory at the given name. The new directory can be empty, or it can be populated with children @@ -2586,7 +2586,7 @@ class IClient(Interface): @return: a Deferred that fires with an IMutableFileNode instance. """ - def create_dirnode(initial_children={}): + def create_dirnode(initial_children=None): """Create a new unattached dirnode, possibly with initial children. @param initial_children: dict with keys that are unicode child names, @@ -2641,7 +2641,7 @@ class INodeMaker(Interface): for use by unit tests, to create mutable files that are smaller than usual.""" - def create_new_mutable_directory(initial_children={}): + def create_new_mutable_directory(initial_children=None): """I create a new mutable directory, and return a Deferred that will fire with the IDirectoryNode instance when it is ready. If initial_children= is provided (a dict mapping unicode child name to diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 58ee33ef5..6c3082b50 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -17,7 +17,7 @@ import errno from base64 import b32decode, b32encode from errno import ENOENT, EPERM from warnings import warn -from typing import Union +from typing import Union, Iterable import attr @@ -172,7 +172,7 @@ def create_node_dir(basedir, readme_text): f.write(readme_text) -def read_config(basedir, portnumfile, generated_files=[], _valid_config=None): +def read_config(basedir, portnumfile, generated_files: Iterable = (), _valid_config=None): """ Read and validate configuration. @@ -741,7 +741,7 @@ def create_connection_handlers(config, i2p_provider, tor_provider): def create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers, - handler_overrides={}, force_foolscap=False, **kwargs): + handler_overrides=None, force_foolscap=False, **kwargs): """ Create a Tub with the right options and handlers. It will be ephemeral unless the caller provides certFile= in kwargs @@ -755,6 +755,8 @@ def create_tub(tub_options, default_connection_handlers, foolscap_connection_han :param bool force_foolscap: If True, only allow Foolscap, not just HTTPS storage protocol. """ + if handler_overrides is None: + handler_overrides = {} # We listen simultaneously for both Foolscap and HTTPS on the same port, # so we have to create a special Foolscap Tub for that to work: if force_foolscap: @@ -922,7 +924,7 @@ def tub_listen_on(i2p_provider, tor_provider, tub, tubport, location): def create_main_tub(config, tub_options, default_connection_handlers, foolscap_connection_handlers, i2p_provider, tor_provider, - handler_overrides={}, cert_filename="node.pem"): + handler_overrides=None, cert_filename="node.pem"): """ Creates a 'main' Foolscap Tub, typically for use as the top-level access point for a running Node. @@ -943,6 +945,8 @@ def create_main_tub(config, tub_options, :param tor_provider: None, or a _Provider instance if txtorcon + Tor are installed. """ + if handler_overrides is None: + handler_overrides = {} portlocation = _tub_portlocation( config, iputil.get_local_addresses_sync, diff --git a/src/allmydata/nodemaker.py b/src/allmydata/nodemaker.py index 1b7ea5f45..39663bda9 100644 --- a/src/allmydata/nodemaker.py +++ b/src/allmydata/nodemaker.py @@ -135,8 +135,9 @@ class NodeMaker(object): d.addCallback(lambda res: n) return d - def create_new_mutable_directory(self, initial_children={}, version=None): - # initial_children must have metadata (i.e. {} instead of None) + def create_new_mutable_directory(self, initial_children=None, version=None): + if initial_children is None: + initial_children = {} for (name, (node, metadata)) in initial_children.items(): precondition(isinstance(metadata, dict), "create_new_mutable_directory requires metadata to be a dict, not None", metadata) diff --git a/src/allmydata/test/cli/wormholetesting.py b/src/allmydata/test/cli/wormholetesting.py index d1a3bfd07..647798bc8 100644 --- a/src/allmydata/test/cli/wormholetesting.py +++ b/src/allmydata/test/cli/wormholetesting.py @@ -70,7 +70,8 @@ class MemoryWormholeServer(object): appid: str, relay_url: str, reactor: Any, - versions: Any={}, + # Unfortunately we need a mutable default to match the real API + versions: Any={}, # noqa: B006 delegate: Optional[Any]=None, journal: Optional[Any]=None, tor: Optional[Any]=None, diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py index ee1f48b17..e3b57fb95 100644 --- a/src/allmydata/test/no_network.py +++ b/src/allmydata/test/no_network.py @@ -476,7 +476,7 @@ class GridTestMixin(object): ]) def set_up_grid(self, num_clients=1, num_servers=10, - client_config_hooks={}, oneshare=False): + client_config_hooks=None, oneshare=False): """ Create a Tahoe-LAFS storage grid. @@ -489,6 +489,8 @@ class GridTestMixin(object): :return: ``None`` """ + if client_config_hooks is None: + client_config_hooks = {} # self.basedir must be set port_assigner = SameProcessStreamEndpointAssigner() port_assigner.setUp() diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py index d11a6e866..b3287bf3b 100644 --- a/src/allmydata/test/test_system.py +++ b/src/allmydata/test/test_system.py @@ -1,20 +1,13 @@ """ Ported to Python 3. """ -from __future__ import print_function -from __future__ import absolute_import -from __future__ import division -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - # Don't import bytes since it causes issues on (so far unported) modules on Python 2. - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, dict, list, object, range, max, min, str # noqa: F401 +from __future__ import annotations from past.builtins import chr as byteschr, long from six import ensure_text import os, re, sys, time, json +from typing import Optional from bs4 import BeautifulSoup @@ -56,10 +49,12 @@ from .common_util import run_cli_unicode class RunBinTahoeMixin(object): - def run_bintahoe(self, args, stdin=None, python_options=[], env=None): + def run_bintahoe(self, args, stdin=None, python_options:Optional[list[str]]=None, env=None): # test_runner.run_bintahoe has better unicode support but doesn't # support env yet and is also synchronous. If we could get rid of # this in favor of that, though, it would probably be an improvement. + if python_options is None: + python_options = [] command = sys.executable argv = python_options + ["-b", "-m", "allmydata.scripts.runner"] + args @@ -1088,7 +1083,9 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): headers["content-type"] = "multipart/form-data; boundary=%s" % str(sepbase, "ascii") return self.POST2(urlpath, body, headers, use_helper) - def POST2(self, urlpath, body=b"", headers={}, use_helper=False): + def POST2(self, urlpath, body=b"", headers=None, use_helper=False): + if headers is None: + headers = {} if use_helper: url = self.helper_webish_url + urlpath else: @@ -1409,7 +1406,7 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): rc,out,err = yield run_cli(verb, *args, nodeargs=nodeargs, **kwargs) defer.returnValue((out,err)) - def _check_ls(out_and_err, expected_children, unexpected_children=[]): + def _check_ls(out_and_err, expected_children, unexpected_children=()): (out, err) = out_and_err self.failUnlessEqual(err, "") for s in expected_children: diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 4c828817a..08dce0ac0 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -565,7 +565,9 @@ class WebMixin(TimezoneMixin): returnValue(data) @inlineCallbacks - def HEAD(self, urlpath, return_response=False, headers={}): + def HEAD(self, urlpath, return_response=False, headers=None): + if headers is None: + headers = {} url = self.webish_url + urlpath response = yield treq.request("head", url, persistent=False, headers=headers) @@ -573,7 +575,9 @@ class WebMixin(TimezoneMixin): raise Error(response.code, response="") returnValue( ("", response.code, response.headers) ) - def PUT(self, urlpath, data, headers={}): + def PUT(self, urlpath, data, headers=None): + if headers is None: + headers = {} url = self.webish_url + urlpath return do_http("put", url, data=data, headers=headers) @@ -618,7 +622,9 @@ class WebMixin(TimezoneMixin): body, headers = self.build_form(**fields) return self.POST2(urlpath, body, headers) - def POST2(self, urlpath, body="", headers={}, followRedirect=False): + def POST2(self, urlpath, body="", headers=None, followRedirect=False): + if headers is None: + headers = {} url = self.webish_url + urlpath if isinstance(body, str): body = body.encode("utf-8") diff --git a/src/allmydata/util/dbutil.py b/src/allmydata/util/dbutil.py index 916382972..45e59cf00 100644 --- a/src/allmydata/util/dbutil.py +++ b/src/allmydata/util/dbutil.py @@ -25,7 +25,7 @@ class DBError(Exception): def get_db(dbfile, stderr=sys.stderr, - create_version=(None, None), updaters={}, just_create=False, dbname="db", + create_version=(None, None), updaters=None, just_create=False, dbname="db", ): """Open or create the given db file. The parent directory must exist. create_version=(SCHEMA, VERNUM), and SCHEMA must have a 'version' table. @@ -33,6 +33,8 @@ def get_db(dbfile, stderr=sys.stderr, to get from ver=1 to ver=2. Returns a (sqlite3,db) tuple, or raises DBError. """ + if updaters is None: + updaters = {} must_create = not os.path.exists(dbfile) try: db = sqlite3.connect(dbfile)