Adds hacking check for api_version decorator

Because of the implementation of this decorator and the controller's metaclass,
the api_version decorator must be the outermost (ie first) decorator wherever
it is used. This patch adds a hacking check to ensure that this is the case.

This decorator is intended to be used on multiple methods with the same name
which offends F811, so '#noqa' is needed too. This will be fixed in a separate
patch.

Bad:
    @some_decorator # noqa  <-- '# noqa' to avoid F811
    @api_version("2.5")     <-- Error, needs to be the first decorator
    def my_api_method...

Good:
    @api_version("2.5") # noqa   <-- this line still needs '# noqa' for F811
    @some_decorator
    def my_api_method...

Change-Id: I579c0061f03d788c477c5424d4d00ec7a6e721e1

HACKING.rst

@@ -44,6 +44,7 @@ Nova Specific Commandments
 - [N329] Validate that LOG.exception messages use _LE.
 - [N330] Validate that LOG.warning messages use _LW.
 - [N331] Change LOG.warn on LOG.warning.
+- [N332] Check that the api_version decorator is the first decorator on a method
 
 Creating Unit Tests
 -------------------

nova/api/openstack/wsgi.py

@@ -1242,8 +1242,6 @@ class Controller(object):
 
     # NOTE(cyeoh): This decorator MUST appear first (the outermost
     # decorator) on an API method for it to work correctly
-    # TODO(cyeoh): Would be much better if this was not a requirement
-    # and if so then checked with probably a hacking check
     @classmethod
     def api_version(cls, min_ver, max_ver=None):
         """Decorator for versioning api methods.

nova/hacking/checks.py

@@ -73,6 +73,7 @@ string_translation = re.compile(r"[^_]*_\(\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*(.)*")
+api_version_re = re.compile(r"@.*api_version")
 
 
 class BaseASTChecker(ast.NodeVisitor):
@@ -389,6 +390,13 @@ def check_assert_called_once(logical_line, filename):
             yield (pos, msg)
 
 
+def check_api_version_decorator(logical_line, blank_before, filename):
+    msg = ("N332: the api_version decorator must be the first decorator"
+           " on a method.")
+    if blank_before == 0 and re.match(api_version_re, logical_line):
+        yield(0, msg)
+
+
 class CheckForStrUnicodeExc(BaseASTChecker):
     """Checks for the use of str() or unicode() on an exception.
 
@@ -467,5 +475,6 @@ def factory(register):
     register(check_explicit_underscore_import)
     register(use_jsonutils)
     register(check_assert_called_once)
+    register(check_api_version_decorator)
     register(CheckForStrUnicodeExc)
     register(CheckForTransAdd)

nova/tests/unit/test_hacking.py

@@ -369,6 +369,16 @@ class HackingTestCase(test.NoDBTestCase):
         errors = [(8, 20, 'N325'), (8, 33, 'N325'), (9, 16, 'N325')]
         self._assert_has_errors(code, checker, expected_errors=errors)
 
+    def test_api_version_decorator_check(self):
+        code = """
+               @some_other_decorator
+               @wsgi.api_version("2.5")
+               def my_method():
+                   pass
+               """
+        self._assert_has_errors(code, checks.check_api_version_decorator,
+                                expected_errors=[(2, 0, "N332")])
+
     def test_trans_add(self):
 
         checker = checks.CheckForTransAdd