Merge #16726: tests: Avoid common Python default parameter gotcha when mutable dict/list:s are used as default parameter values

e4f4ea47eb lint: Catch use of [] or {} as default parameter values in Python functions (practicalswift)
25dd867150 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 e4f4ea47eb. Testing tip: swap the two commits.

Tree-SHA512: 56e14d24fc866211a20185c9fdb274ed046c3aed2dc0e07699e58b6f9fa3b79f6d0c880fb02d72b7fe5cc5eb7c0ff6da0ead33123344e1a872209370c2e49e3f
This commit is contained in:
MarcoFalke 2019-08-28 13:34:22 -04:00
commit cc40b55da7
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
3 changed files with 58 additions and 2 deletions

View file

@ -803,7 +803,9 @@ class HeaderAndShortIDs:
return [ key0, key1 ]
# Version 2 compact blocks use wtxid in shortids (rather than txid)
def initialize_from_block(self, block, nonce=0, prefill_list = [0], use_witness = False):
def initialize_from_block(self, block, nonce=0, prefill_list=None, use_witness=False):
if prefill_list is None:
prefill_list = [0]
self.header = CBlockHeader(block)
self.nonce = nonce
self.prefilled_txn = [ PrefilledTransaction(i, block.vtx[i]) for i in prefill_list ]

View file

@ -44,8 +44,10 @@ class ImportMultiTest(BitcoinTestFramework):
def setup_network(self):
self.setup_nodes()
def test_importmulti(self, req, success, error_code=None, error_message=None, warnings=[]):
def test_importmulti(self, req, success, error_code=None, error_message=None, warnings=None):
"""Run importmulti and assert success"""
if warnings is None:
warnings = []
result = self.nodes[1].importmulti([req])
observed_warnings = []
if 'warnings' in result[0]:

View file

@ -0,0 +1,52 @@
#!/usr/bin/env bash
#
# Copyright (c) 2019 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
#
# Detect when a mutable list or dict is used as a default parameter value in a Python function.
export LC_ALL=C
EXIT_CODE=0
OUTPUT=$(git grep -E '^\s*def [a-zA-Z0-9_]+\(.*=\s*(\[|\{)' -- "*.py")
if [[ ${OUTPUT} != "" ]]; then
echo "A mutable list or dict seems to be used as default parameter value:"
echo
echo "${OUTPUT}"
echo
cat << EXAMPLE
This is how mutable list and dict default parameter values behave:
>>> 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})
The intended behaviour was likely:
>>> 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})
EXAMPLE
EXIT_CODE=1
fi
exit ${EXIT_CODE}