Add hacking rule for api_version
As an implementation limitation, the 'api_version' decorator must be the first decorator if applied on a method. This patch adds a pep8 check for this to be enforced. partial-bp: api-microversioning Change-Id: Ib03ad91e492175d23dde8b3a30dae9a077baffb9
This commit is contained in:
parent
e0e5250d57
commit
9742996102
|
@ -13,6 +13,7 @@ Senlin Specific Commandments
|
||||||
- [S319] Use ``jsonutils`` functions rather than using the ``json`` package
|
- [S319] Use ``jsonutils`` functions rather than using the ``json`` package
|
||||||
directly.
|
directly.
|
||||||
- [S320] Default arguments of a method should not be mutable.
|
- [S320] Default arguments of a method should not be mutable.
|
||||||
|
- [S321] The api_version decorator has to be the first decorator on a method.
|
||||||
|
|
||||||
Working on APIs
|
Working on APIs
|
||||||
---------------
|
---------------
|
||||||
|
|
|
@ -15,6 +15,8 @@ import re
|
||||||
asse_equal_end_with_none_re = re.compile(r"assertEqual\(.*?,\s+None\)$")
|
asse_equal_end_with_none_re = re.compile(r"assertEqual\(.*?,\s+None\)$")
|
||||||
asse_equal_start_with_none_re = re.compile(r"assertEqual\(None,")
|
asse_equal_start_with_none_re = re.compile(r"assertEqual\(None,")
|
||||||
mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
|
mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
|
||||||
|
api_version_dec = re.compile(r"@.*api_version")
|
||||||
|
decorator_re = re.compile(r"@.*")
|
||||||
|
|
||||||
|
|
||||||
def assert_equal_none(logical_line):
|
def assert_equal_none(logical_line):
|
||||||
|
@ -46,7 +48,17 @@ def no_mutable_default_args(logical_line):
|
||||||
yield (0, msg)
|
yield (0, msg)
|
||||||
|
|
||||||
|
|
||||||
|
def check_api_version_decorator(logical_line, previous_logical, blank_before,
|
||||||
|
filename):
|
||||||
|
msg = ("S321: The api_version decorator must be the first decorator on "
|
||||||
|
"a method.")
|
||||||
|
if (blank_before == 0 and re.match(api_version_dec, logical_line) and
|
||||||
|
re.match(decorator_re, previous_logical)):
|
||||||
|
yield(0, msg)
|
||||||
|
|
||||||
|
|
||||||
def factory(register):
|
def factory(register):
|
||||||
register(assert_equal_none)
|
register(assert_equal_none)
|
||||||
register(use_jsonutils)
|
register(use_jsonutils)
|
||||||
register(no_mutable_default_args)
|
register(no_mutable_default_args)
|
||||||
|
register(check_api_version_decorator)
|
||||||
|
|
|
@ -10,6 +10,10 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
import mock
|
||||||
|
import pep8
|
||||||
|
import textwrap
|
||||||
|
|
||||||
from senlin.hacking import checks
|
from senlin.hacking import checks
|
||||||
from senlin.tests.unit.common import base
|
from senlin.tests.unit.common import base
|
||||||
|
|
||||||
|
@ -48,3 +52,49 @@ class HackingTestCase(base.SenlinTestCase):
|
||||||
|
|
||||||
self.assertEqual(0, len(list(checks.no_mutable_default_args(
|
self.assertEqual(0, len(list(checks.no_mutable_default_args(
|
||||||
"defined, undefined = [], {}"))))
|
"defined, undefined = [], {}"))))
|
||||||
|
|
||||||
|
@mock.patch("pep8._checks",
|
||||||
|
{'physical_line': {}, 'logical_line': {}, 'tree': {}})
|
||||||
|
def test_api_version_decorator(self):
|
||||||
|
code = """
|
||||||
|
@some_other_decorator
|
||||||
|
@wsgi.api_version("2.2")
|
||||||
|
def my_method():
|
||||||
|
pass
|
||||||
|
"""
|
||||||
|
|
||||||
|
lines = textwrap.dedent(code).strip().splitlines(True)
|
||||||
|
|
||||||
|
pep8.register_check(checks.check_api_version_decorator)
|
||||||
|
checker = pep8.Checker(filename=None, lines=lines)
|
||||||
|
checker.check_all()
|
||||||
|
checker.report._deferred_print.sort()
|
||||||
|
|
||||||
|
actual_error = checker.report._deferred_print[0]
|
||||||
|
|
||||||
|
self.assertEqual(2, actual_error[0])
|
||||||
|
self.assertEqual(0, actual_error[1])
|
||||||
|
self.assertEqual('S321', actual_error[2])
|
||||||
|
self.assertEqual(' The api_version decorator must be the first '
|
||||||
|
'decorator on a method.',
|
||||||
|
actual_error[3])
|
||||||
|
|
||||||
|
@mock.patch("pep8._checks",
|
||||||
|
{'physical_line': {}, 'logical_line': {}, 'tree': {}})
|
||||||
|
def test_api_version_decorator_good(self):
|
||||||
|
code = """
|
||||||
|
class SomeController():
|
||||||
|
@wsgi.api_version("2.2")
|
||||||
|
def my_method():
|
||||||
|
pass
|
||||||
|
|
||||||
|
"""
|
||||||
|
lines = textwrap.dedent(code).strip().splitlines(True)
|
||||||
|
|
||||||
|
pep8.register_check(checks.check_api_version_decorator)
|
||||||
|
checker = pep8.Checker(filename=None, lines=lines)
|
||||||
|
checker.check_all()
|
||||||
|
checker.report._deferred_print.sort()
|
||||||
|
|
||||||
|
actual_error = checker.report._deferred_print
|
||||||
|
self.assertEqual(0, len(actual_error))
|
||||||
|
|
Loading…
Reference in New Issue