From 46526cf20552656a74acbae2bb013cdab80334c8 Mon Sep 17 00:00:00 2001 From: David Rheinsberg Date: Tue, 26 May 2020 17:31:01 +0200 Subject: [PATCH] osbuild: avoid `[]` as default value Using `[]` as default value for arguments makes `pylint` complain. The reason is that it creates an array statically at the time the function is parsed, rather than dynamically on invocation of the function. This means, when you append to this array, you change the global instance and every further invocation of that function works on this modified array. While our use-cases are safe, this is indeed a common pitfall. Lets avoid using this and resort to `None` instead. This silences a lot of warnings from pylint about "dangerous use of []". --- osbuild/main_cli.py | 2 +- osbuild/meta.py | 4 ++-- osbuild/util/jsoncomm.py | 4 ++-- test/test.py | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/osbuild/main_cli.py b/osbuild/main_cli.py index 88cfdd84..1bbb70f9 100644 --- a/osbuild/main_cli.py +++ b/osbuild/main_cli.py @@ -95,7 +95,7 @@ def parse_arguments(sys_argv): # pylint: disable=too-many-branches -def osbuild_cli(*, sys_argv=[]): +def osbuild_cli(*, sys_argv): args = parse_arguments(sys_argv) manifest = parse_manifest(args.manifest_path) diff --git a/osbuild/meta.py b/osbuild/meta.py index c0edd0a6..25495b15 100644 --- a/osbuild/meta.py +++ b/osbuild/meta.py @@ -128,7 +128,7 @@ class ValidationResult: self.errors.add(err) return self - def merge(self, result: "ValidationResult", *, path=[]): + def merge(self, result: "ValidationResult", *, path=None): """Merge all errors of `result` into this Merge all the errors of in `result` into this, @@ -137,7 +137,7 @@ class ValidationResult: """ for err in result: err = copy.deepcopy(err) - err.rebase(path) + err.rebase(path or []) self.errors.add(err) def as_dict(self): diff --git a/osbuild/util/jsoncomm.py b/osbuild/util/jsoncomm.py index 1591642b..500e8d41 100644 --- a/osbuild/util/jsoncomm.py +++ b/osbuild/util/jsoncomm.py @@ -271,7 +271,7 @@ class Socket(contextlib.AbstractContextManager): return (payload, fdset, msg[3]) - def send(self, payload: object, *, destination: Optional[str] = None, fds: list = []): + def send(self, payload: object, *, destination: Optional[str] = None, fds: Optional[list] = None): """Send Message Send a new message via this socket. This operation is synchronous. The @@ -299,7 +299,7 @@ class Socket(contextlib.AbstractContextManager): serialized = json.dumps(payload).encode() cmsg = [] - if len(fds) > 0: + if fds and len(fds) > 0: cmsg.append((socket.SOL_SOCKET, socket.SCM_RIGHTS, array.array("i", fds))) n = self._socket.sendmsg([serialized], cmsg, 0, destination) diff --git a/test/test.py b/test/test.py index 8b1da354..8bc9457d 100644 --- a/test/test.py +++ b/test/test.py @@ -227,7 +227,7 @@ class OSBuild(contextlib.AbstractContextManager): print(data_stderr) print("-- END ---------------------------------") - def compile(self, data_stdin, checkpoints=[]): + def compile(self, data_stdin, checkpoints=None): """Compile an Artifact This takes a manifest as `data_stdin`, executes the pipeline, and @@ -247,7 +247,7 @@ class OSBuild(contextlib.AbstractContextManager): cmd_args += ["--output-directory", self._outputdir] cmd_args += ["--store", self._cachedir] - for c in checkpoints: + for c in (checkpoints or []): cmd_args += ["--checkpoint", c] # Spawn the `osbuild` executable, feed it the specified data on @@ -272,7 +272,7 @@ class OSBuild(contextlib.AbstractContextManager): self._print_result(p.returncode, data_stdout, data_stderr) self._unittest.assertEqual(p.returncode, 0) - def compile_file(self, file_stdin, checkpoints=[]): + def compile_file(self, file_stdin, checkpoints=None): """Compile an Artifact This is similar to `compile()` but takes a file-path instead of raw