Add hacking checks

Change-Id: I98e78c2c4df9587f7feb732edb684f197f82de0b

.gitignore

@@ -55,4 +55,7 @@ ChangeLog
 .*sw?
 
 # Files created by releasenotes build
-releasenotes/build
+releasenotes/build
+
+# PyCharm project directory
+.idea

octaviaclient/hacking/checks.py

@@ -0,0 +1,263 @@
+# Copyright (c) 2014 OpenStack Foundation.
+#
+# 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
+
+
+"""
+Guidelines for writing new hacking checks
+
+ - Use only for octaviaclient specific tests. OpenStack general tests
+   should be submitted to the common 'hacking' module.
+ - Pick numbers in the range O3xx. Find the current test with
+   the highest allocated number and then pick the next value.
+ - Keep the test method code in the source file ordered based
+   on the O3xx value.
+ - List the new rule in the top level HACKING.rst file
+ - Add test cases for each new rule to
+   octaviaclient/tests/unit/test_hacking.py
+
+"""
+
+author_tag_re = (re.compile("^\s*#\s*@?(a|A)uthor"),
+                 re.compile("^\.\.\s+moduleauthor::"))
+
+_all_log_levels = {'critical', 'error', 'exception', 'info', 'warning'}
+_all_hints = {'_LC', '_LE', '_LI', '_', '_LW'}
+
+_log_translation_hint = re.compile(
+    r".*LOG\.(%(levels)s)\(\s*(%(hints)s)\(" % {
+        'levels': '|'.join(_all_log_levels),
+        'hints': '|'.join(_all_hints),
+    })
+
+assert_trueinst_re = re.compile(
+    r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
+    "(\w|\.|\'|\"|\[|\])+\)\)")
+assert_equal_in_end_with_true_or_false_re = re.compile(
+    r"assertEqual\((\w|[][.'\"])+ in (\w|[][.'\", ])+, (True|False)\)")
+assert_equal_in_start_with_true_or_false_re = re.compile(
+    r"assertEqual\((True|False), (\w|[][.'\"])+ in (\w|[][.'\", ])+\)")
+assert_equal_with_true_re = re.compile(
+    r"assertEqual\(True,")
+assert_equal_with_false_re = re.compile(
+    r"assertEqual\(False,")
+mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
+assert_equal_end_with_none_re = re.compile(r"(.)*assertEqual\(.+, None\)")
+assert_equal_start_with_none_re = re.compile(r".*assertEqual\(None, .+\)")
+assert_not_equal_end_with_none_re = re.compile(
+    r"(.)*assertNotEqual\(.+, None\)")
+assert_not_equal_start_with_none_re = re.compile(
+    r"(.)*assertNotEqual\(None, .+\)")
+assert_no_xrange_re = re.compile(
+    r"\s*xrange\s*\(")
+
+
+def _translation_checks_not_enforced(filename):
+    # Do not do these validations on tests
+    return any(pat in filename for pat in ["/tests/", "rally-jobs/plugins/"])
+
+
+def assert_true_instance(logical_line):
+    """Check for assertTrue(isinstance(a, b)) sentences
+
+    O316
+    """
+    if assert_trueinst_re.match(logical_line):
+        yield (0, "O316: assertTrue(isinstance(a, b)) sentences not allowed")
+
+
+def assert_equal_or_not_none(logical_line):
+    """Check for assertEqual(A, None) or assertEqual(None, A) sentences,
+
+    assertNotEqual(A, None) or assertNotEqual(None, A) sentences
+
+    O318
+    """
+    msg = ("O318: assertEqual/assertNotEqual(A, None) or "
+           "assertEqual/assertNotEqual(None, A) sentences not allowed")
+    res = (assert_equal_start_with_none_re.match(logical_line) or
+           assert_equal_end_with_none_re.match(logical_line) or
+           assert_not_equal_start_with_none_re.match(logical_line) or
+           assert_not_equal_end_with_none_re.match(logical_line))
+    if res:
+        yield (0, msg)
+
+
+def no_author_tags(physical_line):
+    for regex in author_tag_re:
+        if regex.match(physical_line):
+            physical_line = physical_line.lower()
+            pos = physical_line.find('moduleauthor')
+            if pos < 0:
+                pos = physical_line.find('author')
+            return pos, "O322: Don't use author tags"
+
+
+def assert_equal_true_or_false(logical_line):
+    """Check for assertEqual(True, A) or assertEqual(False, A) sentences
+
+    O323
+    """
+    res = (assert_equal_with_true_re.search(logical_line) or
+           assert_equal_with_false_re.search(logical_line))
+    if res:
+        yield (0, "O323: assertEqual(True, A) or assertEqual(False, A) "
+               "sentences not allowed")
+
+
+def no_mutable_default_args(logical_line):
+    msg = "O324: Method's default argument shouldn't be mutable!"
+    if mutable_default_args.match(logical_line):
+        yield (0, msg)
+
+
+def assert_equal_in(logical_line):
+    """Check for assertEqual(A in B, True), assertEqual(True, A in B),
+
+    assertEqual(A in B, False) or assertEqual(False, A in B) sentences
+
+    O338
+    """
+    res = (assert_equal_in_start_with_true_or_false_re.search(logical_line) or
+           assert_equal_in_end_with_true_or_false_re.search(logical_line))
+    if res:
+        yield (0, "O338: Use assertIn/NotIn(A, B) rather than "
+               "assertEqual(A in B, True/False) when checking collection "
+               "contents.")
+
+
+def no_log_warn(logical_line):
+    """Disallow 'LOG.warn('
+
+    O339
+    """
+    if logical_line.startswith('LOG.warn('):
+        yield(0, "O339:Use LOG.warning() rather than LOG.warn()")
+
+
+def no_xrange(logical_line):
+    """Disallow 'xrange()'
+
+    O340
+    """
+    if assert_no_xrange_re.match(logical_line):
+        yield(0, "O340: Do not use xrange().")
+
+
+def no_translate_logs(logical_line, filename):
+    """O341 - Don't translate logs.
+
+    Check for 'LOG.*(_(' and 'LOG.*(_Lx('
+
+    Translators don't provide translations for log messages, and operators
+    asked not to translate them.
+
+    * This check assumes that 'LOG' is a logger.
+
+    :param logical_line: The logical line to check.
+    :param filename: The file name where the logical line exists.
+    :returns: None if the logical line passes the check, otherwise a tuple
+    is yielded that contains the offending index in logical line and a
+    message describe the check validation failure.
+    """
+    if _translation_checks_not_enforced(filename):
+        return
+
+    msg = "O341: Log messages should not be translated!"
+    match = _log_translation_hint.match(logical_line)
+    if match:
+        yield (logical_line.index(match.group()), msg)
+
+
+def check_raised_localized_exceptions(logical_line, filename):
+    """O342 - Untranslated exception message.
+
+    :param logical_line: The logical line to check.
+    :param filename: The file name where the logical line exists.
+    :returns: None if the logical line passes the check, otherwise a tuple
+    is yielded that contains the offending index in logical line and a
+    message describe the check validation failure.
+    """
+    if _translation_checks_not_enforced(filename):
+        return
+
+    logical_line = logical_line.strip()
+    raised_search = re.compile(
+        r"raise (?:\w*)\((.*)\)").match(logical_line)
+    if raised_search:
+        exception_msg = raised_search.groups()[0]
+        if exception_msg.startswith("\"") or exception_msg.startswith("\'"):
+            msg = "O342: Untranslated exception message."
+            yield (logical_line.index(exception_msg), msg)
+
+
+def check_no_basestring(logical_line):
+    """O343 - basestring is not Python3-compatible.
+
+    :param logical_line: The logical line to check.
+    :returns: None if the logical line passes the check, otherwise a tuple
+    is yielded that contains the offending index in logical line and a
+    message describe the check validation failure.
+    """
+    if re.search(r"\bbasestring\b", logical_line):
+        msg = ("O343: basestring is not Python3-compatible, use "
+               "six.string_types instead.")
+        yield(0, msg)
+
+
+def check_python3_no_iteritems(logical_line):
+    """O344 - Use dict.items() instead of dict.iteritems().
+
+    :param logical_line: The logical line to check.
+    :returns: None if the logical line passes the check, otherwise a tuple
+    is yielded that contains the offending index in logical line and a
+    message describe the check validation failure.
+    """
+    if re.search(r".*\.iteritems\(\)", logical_line):
+        msg = ("O344: Use dict.items() instead of dict.iteritems() to be "
+               "compatible with both Python 2 and Python 3. In Python 2, "
+               "dict.items() may be inefficient for very large dictionaries. "
+               "If you can prove that you need the optimization of an "
+               "iterator for Python 2, then you can use six.iteritems(dict).")
+        yield(0, msg)
+
+
+def check_no_eventlet_imports(logical_line):
+    """O345 - Usage of Python eventlet module not allowed.
+
+    :param logical_line: The logical line to check.
+    :returns: None if the logical line passes the check, otherwise a tuple
+    is yielded that contains the offending index in logical line and a
+    message describe the check validation failure.
+    """
+    if re.match(r'(import|from)\s+[(]?eventlet', logical_line):
+        msg = 'O345 Usage of Python eventlet module not allowed'
+        yield logical_line.index('eventlet'), msg
+
+
+def factory(register):
+    register(assert_true_instance)
+    register(assert_equal_or_not_none)
+    register(no_translate_logs)
+    register(no_author_tags)
+    register(assert_equal_true_or_false)
+    register(no_mutable_default_args)
+    register(assert_equal_in)
+    register(no_log_warn)
+    register(no_xrange)
+    register(check_raised_localized_exceptions)
+    register(check_no_basestring)
+    register(check_python3_no_iteritems)
+    register(check_no_eventlet_imports)

octaviaclient/tests/unit/test_hacking.py

@@ -0,0 +1,144 @@
+#    Copyright 2015
+#
+#    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.
+
+from oslotest import base
+
+from octaviaclient.hacking import checks
+
+
+class HackingTestCase(base.BaseTestCase):
+    """Hacking test class.
+
+    This class tests the hacking checks in octaviaclient.hacking.checks by
+    passing strings to the check methods like the pep8/flake8 parser would. The
+    parser loops over each line in the file and then passes the parameters to
+    the check method. The parameter names in the check method dictate what type
+    of object is passed to the check method. The parameter types are::
+
+        logical_line: A processed line with the following modifications:
+            - Multi-line statements converted to a single line.
+            - Stripped left and right.
+            - Contents of strings replaced with "xxx" of same length.
+            - Comments removed.
+        physical_line: Raw line of text from the input file.
+        lines: a list of the raw lines from the input file
+        tokens: the tokens that contribute to this logical line
+        line_number: line number in the input file
+        total_lines: number of lines in the input file
+        blank_lines: blank lines before this one
+        indent_char: indentation character in this file (" " or "\t")
+        indent_level: indentation (with tabs expanded to multiples of 8)
+        previous_indent_level: indentation on previous line
+        previous_logical: previous logical line
+        filename: Path of the file being run through pep8
+
+    When running a test on a check method the return will be False/None if
+    there is no violation in the sample input. If there is an error a tuple is
+    returned with a position in the line, and a message. So to check the result
+    just assertTrue if the check is expected to fail and assertFalse if it
+    should pass.
+    """
+
+    def test_assert_true_instance(self):
+        self.assertEqual(1, len(list(checks.assert_true_instance(
+            "self.assertTrue(isinstance(e, "
+            "exception.BuildAbortException))"))))
+
+        self.assertEqual(0, len(list(checks.assert_true_instance(
+            "self.assertTrue()"))))
+
+    def test_assert_equal_or_not_none(self):
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertEqual(A, None)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertEqual(None, A)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertNotEqual(A, None)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertNotEqual(None, A)"))))
+
+        self.assertEqual(0,
+                         len(list(checks.assert_equal_or_not_none(
+                             "self.assertIsNone()"))))
+
+        self.assertEqual(0,
+                         len(list(checks.assert_equal_or_not_none(
+                             "self.assertIsNotNone()"))))
+
+    def test_assert_equal_in(self):
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual(a in b, True)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual('str' in 'string', True)"))))
+
+        self.assertEqual(0, len(list(checks.assert_equal_in(
+            "self.assertEqual(any(a==1 for a in b), True)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual(True, a in b)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual(True, 'str' in 'string')"))))
+
+        self.assertEqual(0, len(list(checks.assert_equal_in(
+            "self.assertEqual(True, any(a==1 for a in b))"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual(a in b, False)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual('str' in 'string', False)"))))
+
+        self.assertEqual(0, len(list(checks.assert_equal_in(
+            "self.assertEqual(any(a==1 for a in b), False)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual(False, a in b)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_in(
+            "self.assertEqual(False, 'str' in 'string')"))))
+
+        self.assertEqual(0, len(list(checks.assert_equal_in(
+            "self.assertEqual(False, any(a==1 for a in b))"))))
+
+    def test_assert_equal_true_or_false(self):
+        self.assertEqual(1, len(list(checks.assert_equal_true_or_false(
+            "self.assertEqual(True, A)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_true_or_false(
+            "self.assertEqual(False, A)"))))
+
+        self.assertEqual(0, len(list(checks.assert_equal_true_or_false(
+            "self.assertTrue()"))))
+
+        self.assertEqual(0, len(list(checks.assert_equal_true_or_false(
+            "self.assertFalse()"))))
+
+    def test_no_log_warn(self):
+        self.assertEqual(1, len(list(checks.no_log_warn(
+            "LOG.warn()"))))
+
+        self.assertEqual(0, len(list(checks.no_log_warn(
+            "LOG.warning()"))))
+
+    def test_no_xrange(self):
+        self.assertEqual(1, len(list(checks.no_xrange(
+            "xrange(45)"))))
+
+        self.assertEqual(0, len(list(checks.no_xrange(
+            "range(45)"))))

releasenotes/source/conf.py

@@ -207,7 +207,7 @@ latex_elements = {
 
 # Grouping the document tree into LaTeX files. List of tuples
 # (source start file, target name, title,
-#  author, documentclass [howto, manual, or own class]).
+#  documentclass [howto, manual, or own class]).
 latex_documents = [
     ('index', 'GlanceReleaseNotes.tex', u'Glance Release Notes Documentation',
      u'Glance Developers', 'manual'),

tox.ini

@@ -36,8 +36,10 @@ commands =
 
 [flake8]
 # E123, E125 skipped as they are invalid PEP-8.
-
-show-source = True
 ignore = E123,E125
+show-source = true
 builtins = _
 exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build
+
+[hacking]
+local-check-factory = octaviaclient.hacking.checks.factory