Removes the use of mutables as default args

Passing mutable objects as default args is a known Python pitfall.
We'd better avoid this. This commit changes mutable default args with
None, then use 'arg = arg or {}', 'arg = arg or []'. For unit code which
doesn't use the args , just set with None. This commit also adds hacking
check.

This code was taken from commit 0bea84ac20fe498bd08f7212a0017196c8cb0812
in Nova.

Change-Id: I36d07cade687690dc02a8f6cc3d70f5d00caf112
Co-Authored-By: ChangBo Guo(gcb) <glongwave@gmail.com>

HACKING.rst

@@ -18,6 +18,7 @@ Neutron Specific Commandments
 - [N326] Python 3: do not use basestring.
 - [N327] Python 3: do not use dict.iteritems.
 - [N328] Detect wrong usage with assertEqual
+- [N329] Method's default argument shouldn't be mutable
 
 Creating Unit Tests
 -------------------

neutron/agent/linux/interface.py

@@ -105,8 +105,7 @@ class LinuxInterfaceDriver(object):
         return False
 
     def init_l3(self, device_name, ip_cidrs, namespace=None,
-                preserve_ips=[],
-                clean_connections=False):
+                preserve_ips=None, clean_connections=False):
         """Set the L3 settings for the interface using data from the port.
 
         ip_cidrs: list of 'X.X.X.X/YY' strings
@@ -114,6 +113,7 @@ class LinuxInterfaceDriver(object):
         clean_connections: Boolean to indicate if we should cleanup connections
           associated to removed ips
         """
+        preserve_ips = preserve_ips or []
         device = ip_lib.IPDevice(device_name, namespace=namespace)
 
         # The LLA generated by the operating system is not known to

neutron/agent/linux/iptables_firewall.py

@@ -128,7 +128,8 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
                     self.devices_with_udpated_sg_members[sg_id].append(device)
 
     def security_group_updated(self, action_type, sec_group_ids,
-                               device_ids=[]):
+                               device_ids=None):
+        device_ids = device_ids or []
         if action_type == 'sg_rule':
             self.updated_rule_sg_ids.update(sec_group_ids)
         elif action_type == 'sg_member':

neutron/api/api_common.py

@@ -29,7 +29,7 @@ from neutron.i18n import _LW
 LOG = logging.getLogger(__name__)
 
 
-def get_filters(request, attr_info, skips=[]):
+def get_filters(request, attr_info, skips=None):
     """Extracts the filters from the request string.
 
     Returns a dict of lists for the filters:
@@ -37,6 +37,7 @@ def get_filters(request, attr_info, skips=[]):
     becomes:
     {'check': [u'a', u'b'], 'name': [u'Bob']}
     """
+    skips = skips or []
     res = {}
     for key, values in six.iteritems(request.GET.dict_of_lists()):
         if key in skips:

neutron/api/extensions.py

@@ -641,7 +641,10 @@ class ResourceExtension(object):
     """Add top level resources to the OpenStack API in Neutron."""
 
     def __init__(self, collection, controller, parent=None, path_prefix="",
-                 collection_actions={}, member_actions={}, attr_map={}):
+                 collection_actions=None, member_actions=None, attr_map=None):
+        collection_actions = collection_actions or {}
+        member_actions = member_actions or {}
+        attr_map = attr_map or {}
         self.collection = collection
         self.controller = controller
         self.parent = parent

neutron/hacking/checks.py

@@ -38,6 +38,7 @@ _all_log_levels = {
     'exception': '_LE',
 }
 _all_hints = set(_all_log_levels.values())
+mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
 
 
 def _regex_for_level(level, hint):
@@ -186,6 +187,12 @@ def check_asserttrue(logical_line, filename):
             yield (0, msg)
 
 
+def no_mutable_default_args(logical_line):
+    msg = "N329: Method's default argument shouldn't be mutable!"
+    if mutable_default_args.match(logical_line):
+        yield (0, msg)
+
+
 def factory(register):
     register(validate_log_translations)
     register(use_jsonutils)
@@ -197,3 +204,4 @@ def factory(register):
     register(check_no_basestring)
     register(check_python3_no_iteritems)
     register(check_asserttrue)
+    register(no_mutable_default_args)

neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py

@@ -155,7 +155,8 @@ class SriovNicSwitchAgent(object):
         mgr.initialize(connection, 'sriov')
         return mgr
 
-    def setup_eswitch_mgr(self, device_mappings, exclude_devices={}):
+    def setup_eswitch_mgr(self, device_mappings, exclude_devices=None):
+        exclude_devices = exclude_devices or {}
         self.eswitch_mgr = esm.ESwitchManager()
         self.eswitch_mgr.discover_devices(device_mappings, exclude_devices)
 

neutron/tests/unit/agent/linux/test_interface.py

@@ -361,9 +361,9 @@ class TestOVSInterfaceDriver(TestBase):
                                          'aa:bb:cc:dd:ee:ff',
                                          internal=True)
 
-    def _test_plug(self, additional_expectation=[], bridge=None,
+    def _test_plug(self, additional_expectation=None, bridge=None,
                    namespace=None):
-
+        additional_expectation = additional_expectation or []
         if not bridge:
             bridge = 'br-int'
 

neutron/tests/unit/api/test_extensions.py

@@ -51,7 +51,8 @@ extensions_path = ':'.join(neutron.tests.unit.extensions.__path__)
 
 class ExtensionsTestApp(base_wsgi.Router):
 
-    def __init__(self, options={}):
+    def __init__(self, options=None):
+        options = options or {}
         mapper = routes.Mapper()
         controller = ext_stubs.StubBaseAppController()
         mapper.resource("dummy_resource", "/dummy_resources",

neutron/tests/unit/api/v2/test_base.py

@@ -146,7 +146,8 @@ class APIv2TestCase(APIv2TestBase):
         fields.extend(policy_attrs)
         return fields
 
-    def _get_collection_kwargs(self, skipargs=[], **kwargs):
+    def _get_collection_kwargs(self, skipargs=None, **kwargs):
+        skipargs = skipargs or []
         args_list = ['filters', 'fields', 'sorts', 'limit', 'marker',
                      'page_reverse']
         args_dict = dict(

neutron/tests/unit/db/test_db_base_plugin_v2.py

@@ -1220,7 +1220,9 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s
             self, expected_status=webob.exc.HTTPOk.code,
             expected_error='StateInvalid', subnet=None,
             device_owner=DEVICE_OWNER_COMPUTE, updated_fixed_ips=None,
-            host_arg={}, arg_list=[]):
+            host_arg=None, arg_list=None):
+        host_arg = host_arg or {}
+        arg_list = arg_list or []
         with self.port(device_owner=device_owner, subnet=subnet,
                        arg_list=arg_list, **host_arg) as port:
             self.assertIn('mac_address', port['port'])

neutron/tests/unit/extension_stubs.py

@@ -39,7 +39,8 @@ class StubExtension(object):
 
 class StubPlugin(object):
 
-    def __init__(self, supported_extensions=[]):
+    def __init__(self, supported_extensions=None):
+        supported_extensions = supported_extensions or []
         self.supported_extension_aliases = supported_extensions
 
 

neutron/tests/unit/extensions/test_dns.py

@@ -151,9 +151,12 @@ class DnsExtensionTestCase(test_db_base_plugin_v2.TestNetworksV2):
             self._verify_dns_assigment(res['port'],
                                        ips_list=['10.0.0.2'])
 
-    def _verify_dns_assigment(self, port, ips_list=[], exp_ips_ipv4=0,
-                              exp_ips_ipv6=0, ipv4_cidrs=[], ipv6_cidrs=[],
+    def _verify_dns_assigment(self, port, ips_list=None, exp_ips_ipv4=0,
+                              exp_ips_ipv6=0, ipv4_cidrs=None, ipv6_cidrs=None,
                               dns_name=''):
+        ips_list = ips_list or []
+        ipv4_cidrs = ipv4_cidrs or []
+        ipv6_cidrs = ipv6_cidrs or []
         self.assertEqual(port['dns_name'], dns_name)
         dns_assignment = port['dns_assignment']
         if ips_list:

neutron/tests/unit/extensions/test_l3.py

@@ -353,7 +353,8 @@ class L3NatTestCaseMixin(object):
 
     def _add_external_gateway_to_router(self, router_id, network_id,
                                         expected_code=exc.HTTPOk.code,
-                                        neutron_context=None, ext_ips=[]):
+                                        neutron_context=None, ext_ips=None):
+        ext_ips = ext_ips or []
         body = {'router':
                 {'external_gateway_info': {'network_id': network_id}}}
         if ext_ips:

neutron/tests/unit/extensions/test_securitygroup.py

@@ -213,8 +213,9 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
                                                                    network)
 
     def get_ports(self, context, filters=None, fields=None,
-                  sorts=[], limit=None, marker=None,
+                  sorts=None, limit=None, marker=None,
                   page_reverse=False):
+        sorts = sorts or []
         neutron_lports = super(SecurityGroupTestPlugin, self).get_ports(
             context, filters, sorts=sorts, limit=limit, marker=marker,
             page_reverse=page_reverse)

neutron/tests/unit/hacking/test_checks.py

@@ -177,3 +177,16 @@ class HackingTestCase(base.BaseTestCase):
         self.assertEqual(
             0, len(list(checks.check_asserttrue(pass_code,
                                             "neutron/tests/test_assert.py"))))
+
+    def test_no_mutable_default_args(self):
+        self.assertEqual(1, len(list(checks.no_mutable_default_args(
+            " def fake_suds_context(calls={}):"))))
+
+        self.assertEqual(1, len(list(checks.no_mutable_default_args(
+            "def get_info_from_bdm(virt_type, bdm, mapping=[])"))))
+
+        self.assertEqual(0, len(list(checks.no_mutable_default_args(
+            "defined = []"))))
+
+        self.assertEqual(0, len(list(checks.no_mutable_default_args(
+            "defined, undefined = [], {}"))))

neutron/tests/unit/plugins/ml2/drivers/freescale/test_mechanism_fslsdn.py

@@ -221,7 +221,8 @@ class TestFslSdnMechanismDriver(base.BaseTestCase):
         return FakeContext(subnet)
 
     def _get_port_context(self, tenant_id, net_id, port_id,
-                          fixed_ips=[]):
+                          fixed_ips=None):
+        fixed_ips = fixed_ips or []
         # sample data for testing purpose only
         port = {'device_id': '1234',
                 'name': 'FakePort',