Adding custom flake8 check for mutable default arguments

Additionally, removes all current instances of mutable default
arguments

Change-Id: Ib6888070b1570ea55b14053ad8a2490f84cc2eac

designate/hacking/checks.py

@@ -0,0 +1,29 @@
+# Copyright 2014 Hewlett-Packard Development Company, L.P.
+#
+# Author: Kiall Mac Innes <kiall@hp.com>
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+import re
+
+
+mutable_default_argument_check = re.compile(
+    r"def [a-zA-Z0-9].*\(.*(\{|\[|\().*\)\:")
+
+
+def mutable_default_arguments(logical_line, filename):
+    if mutable_default_argument_check.match(logical_line):
+        yield (0, "D701: Default paramater value is a mutable type")
+
+
+def factory(register):
+    register(mutable_default_arguments)

designate/policy.py

@@ -74,8 +74,9 @@ def init(default_rule=None):
     _ENFORCER.set_rules(rules)
 
 
-def check(rule, ctxt, target={}, do_raise=True, exc=exceptions.Forbidden):
+def check(rule, ctxt, target=None, do_raise=True, exc=exceptions.Forbidden):
     creds = ctxt.to_dict()
+    target = target or {}
 
     try:
         result = _ENFORCER.enforce(rule, target, creds, do_raise, exc)

designate/tests/__init__.py

@@ -330,38 +330,52 @@ class TestCase(test.BaseTestCase):
             user=utils.generate_uuid())
 
     # Fixture methods
-    def get_quota_fixture(self, fixture=0, values={}):
+    def get_quota_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.quota_fixtures[fixture])
         _values.update(values)
         return _values
 
-    def get_server_fixture(self, fixture=0, values={}):
+    def get_server_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.server_fixtures[fixture])
         _values.update(values)
         return _values
 
-    def get_tld_fixture(self, fixture=0, values={}):
+    def get_tld_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.tld_fixtures[fixture])
         _values.update(values)
         return _values
 
-    def get_default_tld_fixture(self, fixture=0, values={}):
+    def get_default_tld_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.default_tld_fixtures[fixture])
         _values.update(values)
         return _values
 
-    def get_tsigkey_fixture(self, fixture=0, values={}):
+    def get_tsigkey_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.tsigkey_fixtures[fixture])
         _values.update(values)
         return _values
 
-    def get_domain_fixture(self, fixture=0, values={}):
+    def get_domain_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.domain_fixtures[fixture])
         _values.update(values)
         return _values
 
     def get_recordset_fixture(self, domain_name, type='A', fixture=0,
-                              values={}):
+                              values=None):
+        values = values or {}
+
         _values = copy.copy(self.recordset_fixtures[type][fixture])
         _values.update(values)
 
@@ -372,12 +386,16 @@ class TestCase(test.BaseTestCase):
 
         return _values
 
-    def get_record_fixture(self, recordset_type, fixture=0, values={}):
+    def get_record_fixture(self, recordset_type, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.record_fixtures[recordset_type][fixture])
         _values.update(values)
         return _values
 
-    def get_ptr_fixture(self, fixture=0, values={}):
+    def get_ptr_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.ptr_fixtures[fixture])
         _values.update(values)
         return _values
@@ -391,7 +409,9 @@ class TestCase(test.BaseTestCase):
         with open(path) as zonefile:
             return zonefile.read()
 
-    def get_blacklist_fixture(self, fixture=0, values={}):
+    def get_blacklist_fixture(self, fixture=0, values=None):
+        values = values or {}
+
         _values = copy.copy(self.blacklist_fixtures[fixture])
         _values.update(values)
         return _values

designate/tests/test_backend/test_ipa.py

@@ -99,8 +99,10 @@ class MockRequest(object):
 
 class IPABackendTestCase(tests.TestCase, BackendTestMixin):
 
-    def get_record_fixture(self, recordset_type, fixture=0, values={}):
+    def get_record_fixture(self, recordset_type, fixture=0, values=None):
         """override to ensure all records have a recordset_id"""
+        values = values or {}
+
         return super(IPABackendTestCase, self).get_record_fixture(
             recordset_type, fixture,
             values={

designate/tests/test_storage/__init__.py

@@ -23,9 +23,9 @@ LOG = logging.getLogger(__name__)
 
 
 class StorageTestCase(object):
-    def create_quota(self, fixture=0, values={}, context=None):
+    def create_quota(self, fixture=0, values=None, context=None):
-        if not context:
+        values = values or {}
-            context = self.admin_context
+        context = context or self.admin_context
 
         fixture = self.get_quota_fixture(fixture, values)
 
@@ -34,23 +34,23 @@ class StorageTestCase(object):
 
         return fixture, self.storage.create_quota(context, fixture)
 
-    def create_server(self, fixture=0, values={}, context=None):
+    def create_server(self, fixture=0, values=None, context=None):
-        if not context:
+        values = values or {}
-            context = self.admin_context
+        context = context or self.admin_context
 
         fixture = self.get_server_fixture(fixture, values)
         return fixture, self.storage.create_server(context, fixture)
 
-    def create_tsigkey(self, fixture=0, values={}, context=None):
+    def create_tsigkey(self, fixture=0, values=None, context=None):
-        if not context:
+        values = values or {}
-            context = self.admin_context
+        context = context or self.admin_context
 
         fixture = self.get_tsigkey_fixture(fixture, values)
         return fixture, self.storage.create_tsigkey(context, fixture)
 
-    def create_domain(self, fixture=0, values={}, context=None):
+    def create_domain(self, fixture=0, values=None, context=None):
-        if not context:
+        values = values or {}
-            context = self.admin_context
+        context = context or self.admin_context
 
         fixture = self.get_domain_fixture(fixture, values)
 
@@ -59,20 +59,20 @@ class StorageTestCase(object):
 
         return fixture, self.storage.create_domain(context, fixture)
 
-    def create_recordset(self, domain, type='A', fixture=0, values={},
+    def create_recordset(self, domain, type='A', fixture=0, values=None,
                          context=None):
-        if not context:
+        values = values or {}
-            context = self.admin_context
+        context = context or self.admin_context
 
         fixture = self.get_recordset_fixture(domain['name'], type, fixture,
                                              values)
         return fixture, self.storage.create_recordset(
             context, domain['id'], fixture)
 
-    def create_record(self, domain, recordset, fixture=0, values={},
+    def create_record(self, domain, recordset, fixture=0, values=None,
                       context=None):
-        if not context:
+        values = values or {}
-            context = self.admin_context
+        context = context or self.admin_context
 
         fixture = self.get_record_fixture(recordset['type'], fixture, values)
         return fixture, self.storage.create_record(

designate/utils.py

@@ -127,7 +127,7 @@ def execute(*cmd, **kw):
                                 root_helper=root_helper, **kw)
 
 
-def get_item_properties(item, fields, mixed_case_fields=[], formatters={}):
+def get_item_properties(item, fields, mixed_case_fields=None, formatters=None):
     """Return a tuple containing the item properties.
 
     :param item: a single item resource (e.g. Server, Tenant, etc)
@@ -137,6 +137,8 @@ def get_item_properties(item, fields, mixed_case_fields=[], formatters={}):
         to format the values
     """
     row = []
+    mixed_case_fields = mixed_case_fields or []
+    formatters = formatters or {}
 
     for field in fields:
         if field in formatters:

tox.ini

@@ -32,3 +32,6 @@ commands = {posargs}
 ignore = H302,H306,H401,H402,H404
 builtins = _
 exclude = .venv,.git,.tox,dist,doc,*openstack/common*,*openstack/deprecated*,*lib/python*,*egg,build,tools
+
+[hacking]
+local-check-factory = designate.hacking.checks.factory