Add Designate hacking checks to the client

This patch adds the Designate hacking checks to also run against
the Designate client code to maintain consistent style
checking across the Designate repositories.

It also resolves one style check failure.

Change-Id: I09786482b2e7fecaa3d4484c9b1a017fae04cfea

5 files changed, 182 insertions, 3 deletions

diff --git a/designateclient/hacking/__init__.py b/designateclient/hacking/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/designateclient/hacking/__init__.py
diff --git a/designateclient/hacking/checks.py b/designateclient/hacking/checks.py
new file mode 100644
index 0000000..e2a5410
--- /dev/null
+++ b/designateclient/hacking/checks.py
@@ -0,0 +1,163 @@
+# Copyright 2014 Hewlett-Packard Development Company, L.P.
+# Copyright (c) 2012, Cloudscaling
+#
+# 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
+
+from hacking import core
+import pycodestyle
+
+# D701: Default parameter value is a mutable type
+# D702: Log messages require translation
+# D703: Found use of _() without explicit import of _!
+# D704: Found import of %s. This oslo library has been graduated!
+# D705: timeutils.utcnow() must be used instead of datetime.%s()
+# D706: Don't translate debug level logs
+# D707: basestring is not Python3-compatible, use str instead.
+# D708: Do not use xrange. Use range for large loops.
+# D709: LOG.audit is deprecated, please use LOG.info!
+# D710: LOG.warn() is not allowed. Use LOG.warning()
+
+UNDERSCORE_IMPORT_FILES = []
+
+
+mutable_default_argument_check = re.compile(
+    r"^\s*def .+\((.+=\{\}|.+=\[\])")
+string_translation = re.compile(r"[^_]*_\(\s*('|\")")
+translated_log = re.compile(
+    r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)"
+    r"\(\s*_\(\s*('|\")")
+underscore_import_check = re.compile(r"(.)*import _(.)*")
+# We need this for cases where they have created their own _ function.
+custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*")
+graduated_oslo_libraries_import_re = re.compile(
+    r"^\s*(?:import|from) designate\.openstack\.common\.?.*?"
+    r"(gettextutils|rpc)"
+    r".*?")
+
+
+@core.flake8ext
+def mutable_default_arguments(physical_line, logical_line, filename):
+    if pycodestyle.noqa(physical_line):
+        return
+
+    if mutable_default_argument_check.match(logical_line):
+        yield (0, "D701: Default parameter value is a mutable type")
+
+
+@core.flake8ext
+def no_translate_debug_logs(logical_line, filename):
+    """Check for 'LOG.debug(_('
+    As per our translation policy,
+    https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
+    we shouldn't translate debug level logs.
+    * This check assumes that 'LOG' is a logger.
+    * Use filename so we can start enforcing this in specific folders instead
+      of needing to do so all at once.
+    N319
+    """
+    if logical_line.startswith("LOG.debug(_("):
+        yield(0, "D706: Don't translate debug level logs")
+
+
+@core.flake8ext
+def check_explicit_underscore_import(logical_line, filename):
+    """Check for explicit import of the _ function
+
+    We need to ensure that any files that are using the _() function
+    to translate logs are explicitly importing the _ function.  We
+    can't trust unit test to catch whether the import has been
+    added so we need to check for it here.
+    """
+    # Build a list of the files that have _ imported.  No further
+    # checking needed once it is found.
+    if filename in UNDERSCORE_IMPORT_FILES:
+        pass
+    elif (underscore_import_check.match(logical_line) or
+          custom_underscore_check.match(logical_line)):
+        UNDERSCORE_IMPORT_FILES.append(filename)
+    elif (translated_log.match(logical_line) or
+          string_translation.match(logical_line)):
+        yield(0, "D703: Found use of _() without explicit import of _!")
+
+
+@core.flake8ext
+def no_import_graduated_oslo_libraries(logical_line, filename):
+    """Check that we don't continue to use o.c. oslo libraries after graduation
+
+    After a library graduates from oslo-incubator, as we make the switch, we
+    should ensure we don't continue to use the oslo-incubator versions.
+
+    In many cases, it's not possible to immediately remove the code from the
+    openstack/common folder due to dependency issues.
+    """
+    # We can't modify oslo-incubator code, so ignore it here.
+    if "designate/openstack/common" in filename:
+        return
+
+    matches = graduated_oslo_libraries_import_re.match(logical_line)
+    if matches:
+        yield(0, "D704: Found import of %s. This oslo library has been "
+                 "graduated!" % matches.group(1))
+
+
+@core.flake8ext
+def use_timeutils_utcnow(logical_line, filename):
+    # tools are OK to use the standard datetime module
+    if "/tools/" in filename:
+        return
+
+    msg = "D705: timeutils.utcnow() must be used instead of datetime.%s()"
+
+    datetime_funcs = ['now', 'utcnow']
+    for f in datetime_funcs:
+        pos = logical_line.find('datetime.%s' % f)
+        if pos != -1:
+            yield (pos, msg % f)
+
+
+@core.flake8ext
+def check_no_basestring(logical_line):
+    if re.search(r"\bbasestring\b", logical_line):
+        msg = ("D707: basestring is not Python3-compatible, use "
+               "str instead.")
+        yield(0, msg)
+
+
+@core.flake8ext
+def check_python3_xrange(logical_line):
+    if re.search(r"\bxrange\s*\(", logical_line):
+        yield(0, "D708: Do not use xrange. Use range for "
+                 "large loops.")
+
+
+@core.flake8ext
+def check_no_log_audit(logical_line):
+    """Ensure that we are not using LOG.audit messages
+    Plans are in place going forward as discussed in the following
+    spec (https://review.opendev.org/#/c/132552/) to take out
+    LOG.audit messages. Given that audit was a concept invented
+    for OpenStack we can enforce not using it.
+    """
+    if "LOG.audit(" in logical_line:
+        yield(0, "D709: LOG.audit is deprecated, please use LOG.info!")
+
+
+@core.flake8ext
+def check_no_log_warn(logical_line):
+    """Disallow 'LOG.warn('
+
+    D710
+    """
+    if logical_line.startswith('LOG.warn('):
+        yield(0, "D710:Use LOG.warning() rather than LOG.warn()")
diff --git a/designateclient/utils.py b/designateclient/utils.py
index ad5835a..1425b83 100644
--- a/designateclient/utils.py
+++ b/designateclient/utils.py
@@ -21,7 +21,7 @@ from keystoneauth1 import adapter
 from designateclient import exceptions
 
 
-def get_item_properties(item, fields, mixed_case_fields=[], formatters={}):
+def get_item_properties(item, fields, mixed_case_fields=(), formatters=None):
     """Return a tuple containing the item properties.
 
     :param item: a single item resource (e.g. Server, Tenant, etc)
@@ -30,6 +30,8 @@ def get_item_properties(item, fields, mixed_case_fields=[], formatters={}):
     :param formatters: dictionary mapping field names to callables
         to format the values
     """
+    if formatters is None:
+        formatters = {}
     row = []
 
     for field in fields:
diff --git a/doc/source/conf.py b/doc/source/conf.py
index 31c3b65..34bbb5f 100644
--- a/doc/source/conf.py
+++ b/doc/source/conf.py
@@ -21,12 +21,13 @@ html_theme = 'openstackdocs'
 
 apidoc_module_dir = '../../designateclient'
 apidoc_output_dir = 'reference/api'
-apidoc_excluded_paths = [ 'tests/*', 'functionaltests/*' ]
+apidoc_excluded_paths = [ 'tests/*', 'functionaltests/*', 'hacking/*' ]
 apidoc_separate_modules = True
 
 autodoc_exclude_modules = [
   'designateclient.tests.*',
-  'designateclient.functionaltests.*']
+  'designateclient.functionaltests.*',
+  'designateclient.hacking.*']
 
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
diff --git a/tox.ini b/tox.ini
index 8bdf84c..0c687b0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -97,6 +97,19 @@ ignore = H105,H302,H404,H405,W504,H904
 builtins = _
 exclude = .venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build,tools
 
+[flake8:local-plugins]
+extension =
+  D701 = checks:mutable_default_arguments
+  D703 = checks:check_explicit_underscore_import
+  D704 = checks:no_import_graduated_oslo_libraries
+  D705 = checks:use_timeutils_utcnow
+  D706 = checks:no_translate_debug_logs
+  D707 = checks:check_no_basestring
+  D708 = checks:check_python3_xrange
+  D709 = checks:check_no_log_audit
+  D710 = checks:check_no_log_warn
+paths = ./designateclient/hacking
+
 [testenv:lower-constraints]
 skip_install = True
 commands =