Commit Graph

1 Commits

Author SHA1 Message Date
MarcoFalke
a84ec5cc19 Merge #16726: tests: Avoid common Python default parameter gotcha when mutable dict/list:s are used as default parameter values
e4f4ea47ebf7774fb6f445adde7bf7ea71fa05a1 lint: Catch use of [] or {} as default parameter values in Python functions (practicalswift)
25dd86715039586d92176eee16e9c6644d2547f0 Avoid using mutable default parameter values (practicalswift)

Pull request description:

  Avoid common Python default parameter gotcha when mutable `dict`/`list`:s are used as default parameter values.

  Examples of this gotcha caught during review:
  * https://github.com/bitcoin/bitcoin/pull/16673#discussion_r317415261
  * https://github.com/bitcoin/bitcoin/pull/14565#discussion_r241942304

  Perhaps surprisingly this is how mutable list and dictionary default parameter values behave in Python:

  ```
  >>> def f(i, j=[], k={}):
  ...     j.append(i)
  ...     k[i] = True
  ...     return j, k
  ...
  >>> f(1)
  ([1], {1: True})
  >>> f(1)
  ([1, 1], {1: True})
  >>> f(2)
  ([1, 1, 2], {1: True, 2: True})
  ```

  In contrast to:

  ```
  >>> def f(i, j=None, k=None):
  ...     if j is None:
  ...         j = []
  ...     if k is None:
  ...         k = {}
  ...     j.append(i)
  ...     k[i] = True
  ...     return j, k
  ...
  >>> f(1)
  ([1], {1: True})
  >>> f(1)
  ([1], {1: True})
  >>> f(2)
  ([2], {2: True})
  ```

  The latter is typically the intended behaviour.

  This PR fixes two instances of this and adds a check guarding against this gotcha going forward :-)

ACKs for top commit:
  Sjors:
    Oh Python... ACK e4f4ea47ebf7774fb6f445adde7bf7ea71fa05a1. Testing tip: swap the two commits.

Tree-SHA512: 56e14d24fc866211a20185c9fdb274ed046c3aed2dc0e07699e58b6f9fa3b79f6d0c880fb02d72b7fe5cc5eb7c0ff6da0ead33123344e1a872209370c2e49e3f
2023-04-04 12:45:27 -05:00