openstack/neutron
Code Issues Proposed changes

Merge "pep8: Register checks with their code"

Browse Source
This commit is contained in:
Jenkins 2016-06-29 22:40:56 +00:00 committed by Gerrit Code Review
commit 32cc21bc4a
2 changed files with 84 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
neutron/hacking/checks.py
View File

@ -15,12 +15,12 @@
import os import os
import re import re
from debtcollector import moves
from hacking import core
from neutron_lib.hacking import checks from neutron_lib.hacking import checks
import pep8 import pep8
import six import six
from hacking import core
def flake8ext(f): def flake8ext(f):
"""Decorator to indicate flake8 extension. """Decorator to indicate flake8 extension.
@ -76,6 +76,7 @@ unittest_imports_from = re.compile(r"\bfrom[\s]+unittest\b")
@flake8ext @flake8ext
def validate_log_translations(logical_line, physical_line, filename): def validate_log_translations(logical_line, physical_line, filename):
"""N320 - Log messages require translation."""
# Translations are not required in the test directory # Translations are not required in the test directory
if "neutron/tests" in filename: if "neutron/tests" in filename:
return return
@ -89,6 +90,7 @@ def validate_log_translations(logical_line, physical_line, filename):
@flake8ext @flake8ext
def use_jsonutils(logical_line, filename): def use_jsonutils(logical_line, filename):
"""N321 - Use jsonutils instead of json."""
msg = "N321: jsonutils.%(fun)s must be used instead of json.%(fun)s" msg = "N321: jsonutils.%(fun)s must be used instead of json.%(fun)s"
# Some files in the tree are not meant to be run from inside Neutron # Some files in the tree are not meant to be run from inside Neutron
@ -112,14 +114,13 @@ def use_jsonutils(logical_line, filename):
@flake8ext @flake8ext
def no_translate_debug_logs(logical_line, filename): def no_translate_debug_logs(logical_line, filename):
"""Check for 'LOG.debug(_(' and 'LOG.debug(_Lx(' """N319 - Check for 'LOG.debug(_(' and 'LOG.debug(_Lx('
As per our translation policy, As per our translation policy,
https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
we shouldn't translate debug level logs. we shouldn't translate debug level logs.
* This check assumes that 'LOG' is a logger. * This check assumes that 'LOG' is a logger.
N319
""" """
for hint in _all_hints: for hint in _all_hints:
if logical_line.startswith("LOG.debug(%s(" % hint): if logical_line.startswith("LOG.debug(%s(" % hint):
@ -128,11 +129,12 @@ def no_translate_debug_logs(logical_line, filename):
@flake8ext @flake8ext
def check_assert_called_once_with(logical_line, filename): def check_assert_called_once_with(logical_line, filename):
# Try to detect unintended calls of nonexistent mock methods like: """N322 - Try to detect unintended calls of nonexistent mock methods like:
# assert_called_once assert_called_once
# assertCalledOnceWith assertCalledOnceWith
# assert_has_called assert_has_called
# called_once_with called_once_with
"""
if 'neutron/tests/' in filename: if 'neutron/tests/' in filename:
if '.assert_called_once_with(' in logical_line: if '.assert_called_once_with(' in logical_line:
return return
@ -152,6 +154,7 @@ def check_assert_called_once_with(logical_line, filename):
@flake8ext @flake8ext
def check_no_contextlib_nested(logical_line, filename): def check_no_contextlib_nested(logical_line, filename):
"""N324 - Don't use contextlib.nested."""
msg = ("N324: contextlib.nested is deprecated. With Python 2.7 and later " msg = ("N324: contextlib.nested is deprecated. With Python 2.7 and later "
"the with-statement supports multiple nested objects. See https://" "the with-statement supports multiple nested objects. See https://"
"docs.python.org/2/library/contextlib.html#contextlib.nested for " "docs.python.org/2/library/contextlib.html#contextlib.nested for "
@ -163,6 +166,7 @@ def check_no_contextlib_nested(logical_line, filename):
@flake8ext @flake8ext
def check_python3_xrange(logical_line): def check_python3_xrange(logical_line):
"""N325 - Do not use xrange."""
if re.search(r"\bxrange\s*\(", logical_line): if re.search(r"\bxrange\s*\(", logical_line):
yield(0, "N325: Do not use xrange. Use range, or six.moves.range for " yield(0, "N325: Do not use xrange. Use range, or six.moves.range for "
"large loops.") "large loops.")
@ -170,6 +174,7 @@ def check_python3_xrange(logical_line):
@flake8ext @flake8ext
def check_no_basestring(logical_line): def check_no_basestring(logical_line):
"""N326 - Don't use basestring."""
if re.search(r"\bbasestring\b", logical_line): if re.search(r"\bbasestring\b", logical_line):
msg = ("N326: basestring is not Python3-compatible, use " msg = ("N326: basestring is not Python3-compatible, use "
"six.string_types instead.") "six.string_types instead.")
@ -178,13 +183,15 @@ def check_no_basestring(logical_line):
@flake8ext @flake8ext
def check_python3_no_iteritems(logical_line): def check_python3_no_iteritems(logical_line):
"""N327 - Use six.iteritems()"""
if re.search(r".*\.iteritems\(\)", logical_line): if re.search(r".*\.iteritems\(\)", logical_line):
msg = ("N327: Use six.iteritems() instead of dict.iteritems().") msg = ("N327: Use six.iteritems() instead of dict.iteritems().")
yield(0, msg) yield(0, msg)
@flake8ext @flake8ext
def check_asserttrue(logical_line, filename): def check_asserttruefalse(logical_line, filename):
"""N328 - Don't use assertEqual(True/False, observed)."""
if 'neutron/tests/' in filename: if 'neutron/tests/' in filename:
if re.search(r"assertEqual\(\s*True,[^,]*(,[^,]*)?\)", logical_line): if re.search(r"assertEqual\(\s*True,[^,]*(,[^,]*)?\)", logical_line):
msg = ("N328: Use assertTrue(observed) instead of " msg = ("N328: Use assertTrue(observed) instead of "
@ -194,18 +201,6 @@ def check_asserttrue(logical_line, filename):
msg = ("N328: Use assertTrue(observed) instead of " msg = ("N328: Use assertTrue(observed) instead of "
"assertEqual(True, observed)") "assertEqual(True, observed)")
yield (0, msg) yield (0, msg)
@flake8ext
def no_mutable_default_args(logical_line):
msg = "N329: Method's default argument shouldn't be mutable!"
if checks.mutable_default_args.match(logical_line):
yield (0, msg)
@flake8ext
def check_assertfalse(logical_line, filename):
if 'neutron/tests/' in filename:
if re.search(r"assertEqual\(\s*False,[^,]*(,[^,]*)?\)", logical_line): if re.search(r"assertEqual\(\s*False,[^,]*(,[^,]*)?\)", logical_line):
msg = ("N328: Use assertFalse(observed) instead of " msg = ("N328: Use assertFalse(observed) instead of "
"assertEqual(False, observed)") "assertEqual(False, observed)")
@ -216,8 +211,31 @@ def check_assertfalse(logical_line, filename):
yield (0, msg) yield (0, msg)
check_asserttrue = flake8ext(
moves.moved_function(
check_asserttruefalse, 'check_asserttrue', __name__,
version='Newton', removal_version='Ocata'))
check_assertfalse = flake8ext(
moves.moved_function(
check_asserttruefalse, 'check_assertfalse', __name__,
version='Newton', removal_version='Ocata'))
@flake8ext
def no_mutable_default_args(logical_line):
"""N329 - Don't use mutable default arguments."""
msg = "N329: Method's default argument shouldn't be mutable!"
if checks.mutable_default_args.match(logical_line):
yield (0, msg)
@flake8ext @flake8ext
def check_assertempty(logical_line, filename): def check_assertempty(logical_line, filename):
"""N330 - Enforce using assertEqual parameter ordering in case of empty
objects.
"""
if 'neutron/tests/' in filename: if 'neutron/tests/' in filename:
msg = ("N330: Use assertEqual(*empty*, observed) instead of " msg = ("N330: Use assertEqual(*empty*, observed) instead of "
"assertEqual(observed, *empty*). *empty* contains " "assertEqual(observed, *empty*). *empty* contains "
@ -230,6 +248,7 @@ def check_assertempty(logical_line, filename):
@flake8ext @flake8ext
def check_assertisinstance(logical_line, filename): def check_assertisinstance(logical_line, filename):
"""N331 - Enforce using assertIsInstance."""
if 'neutron/tests/' in filename: if 'neutron/tests/' in filename:
if re.search(r"assertTrue\(\s*isinstance\(\s*[^,]*,\s*[^,]*\)\)", if re.search(r"assertTrue\(\s*isinstance\(\s*[^,]*,\s*[^,]*\)\)",
logical_line): logical_line):
@ -240,6 +259,7 @@ def check_assertisinstance(logical_line, filename):
@flake8ext @flake8ext
def check_assertequal_for_httpcode(logical_line, filename): def check_assertequal_for_httpcode(logical_line, filename):
"""N332 - Enforce correct oredering for httpcode in assertEqual."""
msg = ("N332: Use assertEqual(expected_http_code, observed_http_code) " msg = ("N332: Use assertEqual(expected_http_code, observed_http_code) "
"instead of assertEqual(observed_http_code, expected_http_code)") "instead of assertEqual(observed_http_code, expected_http_code)")
if 'neutron/tests/' in filename: if 'neutron/tests/' in filename:
@ -250,6 +270,7 @@ def check_assertequal_for_httpcode(logical_line, filename):
@flake8ext @flake8ext
def check_log_warn_deprecated(logical_line, filename): def check_log_warn_deprecated(logical_line, filename):
"""N333 - Use LOG.warning."""
msg = "N333: Use LOG.warning due to compatibility with py3" msg = "N333: Use LOG.warning due to compatibility with py3"
if log_warn.match(logical_line): if log_warn.match(logical_line):
yield (0, msg) yield (0, msg)
@ -257,7 +278,7 @@ def check_log_warn_deprecated(logical_line, filename):
@flake8ext @flake8ext
def check_oslo_i18n_wrapper(logical_line, filename, noqa): def check_oslo_i18n_wrapper(logical_line, filename, noqa):
"""Check for neutron.i18n usage. """N340 - Check for neutron.i18n usage.
Okay(neutron/foo/bar.py): from neutron._i18n import _ Okay(neutron/foo/bar.py): from neutron._i18n import _
Okay(neutron_lbaas/foo/bar.py): from neutron_lbaas._i18n import _ Okay(neutron_lbaas/foo/bar.py): from neutron_lbaas._i18n import _
@ -286,7 +307,7 @@ def check_oslo_i18n_wrapper(logical_line, filename, noqa):
@flake8ext @flake8ext
def check_builtins_gettext(logical_line, tokens, filename, lines, noqa): def check_builtins_gettext(logical_line, tokens, filename, lines, noqa):
"""Check usage of builtins gettext _(). """N341 - Check usage of builtins gettext _().
Okay(neutron/foo.py): from neutron._i18n import _\n_('foo') Okay(neutron/foo.py): from neutron._i18n import _\n_('foo')
N341(neutron/foo.py): _('foo') N341(neutron/foo.py): _('foo')
@ -327,6 +348,7 @@ def check_builtins_gettext(logical_line, tokens, filename, lines, noqa):
@core.flake8ext @core.flake8ext
@core.off_by_default @core.off_by_default
def check_unittest_imports(logical_line): def check_unittest_imports(logical_line):
"""N334 - Use unittest2 instead of unittest"""
if (re.match(unittest_imports_from, logical_line) or if (re.match(unittest_imports_from, logical_line) or
re.match(unittest_imports_dot, logical_line)): re.match(unittest_imports_dot, logical_line)):
msg = "N334: '%s' must be used instead of '%s'." % ( msg = "N334: '%s' must be used instead of '%s'." % (
@ -343,9 +365,8 @@ def factory(register):
register(check_python3_xrange) register(check_python3_xrange)
register(check_no_basestring) register(check_no_basestring)
register(check_python3_no_iteritems) register(check_python3_no_iteritems)
register(check_asserttrue) register(check_asserttruefalse)
register(no_mutable_default_args) register(no_mutable_default_args)
register(check_assertfalse)
register(check_assertempty) register(check_assertempty)
register(check_assertisinstance) register(check_assertisinstance)
register(check_assertequal_for_httpcode) register(check_assertequal_for_httpcode)

70
neutron/tests/unit/hacking/test_checks.py
View File

@ -165,29 +165,6 @@ class HackingTestCase(base.BaseTestCase):
self.assertLineFails(f, "d.iteritems()") self.assertLineFails(f, "d.iteritems()")
self.assertLinePasses(f, "six.iteritems(d)") self.assertLinePasses(f, "six.iteritems(d)")
def test_asserttrue(self):
fail_code1 = """
test_bool = True
self.assertEqual(True, test_bool)
"""
fail_code2 = """
test_bool = True
self.assertEqual(test_bool, True)
"""
pass_code = """
test_bool = True
self.assertTrue(test_bool)
"""
self.assertEqual(
1, len(list(checks.check_asserttrue(fail_code1,
"neutron/tests/test_assert.py"))))
self.assertEqual(
1, len(list(checks.check_asserttrue(fail_code2,
"neutron/tests/test_assert.py"))))
self.assertEqual(
0, len(list(checks.check_asserttrue(pass_code,
"neutron/tests/test_assert.py"))))
def test_no_mutable_default_args(self): def test_no_mutable_default_args(self):
self.assertEqual(1, len(list(checks.no_mutable_default_args( self.assertEqual(1, len(list(checks.no_mutable_default_args(
" def fake_suds_context(calls={}):")))) " def fake_suds_context(calls={}):"))))
@ -201,28 +178,55 @@ class HackingTestCase(base.BaseTestCase):
self.assertEqual(0, len(list(checks.no_mutable_default_args( self.assertEqual(0, len(list(checks.no_mutable_default_args(
"defined, undefined = [], {}")))) "defined, undefined = [], {}"))))
def test_assertfalse(self): def test_asserttruefalse(self):
fail_code1 = """ true_fail_code1 = """
test_bool = True
self.assertEqual(True, test_bool)
"""
true_fail_code2 = """
test_bool = True
self.assertEqual(test_bool, True)
"""
true_pass_code = """
test_bool = True
self.assertTrue(test_bool)
"""
false_fail_code1 = """
test_bool = False test_bool = False
self.assertEqual(False, test_bool) self.assertEqual(False, test_bool)
""" """
fail_code2 = """ false_fail_code2 = """
test_bool = False test_bool = False
self.assertEqual(test_bool, False) self.assertEqual(test_bool, False)
""" """
pass_code = """ false_pass_code = """
test_bool = False test_bool = False
self.assertFalse(test_bool) self.assertFalse(test_bool)
""" """
self.assertEqual( self.assertEqual(
1, len(list(checks.check_assertfalse(fail_code1, 1, len(list(
"neutron/tests/test_assert.py")))) checks.check_asserttruefalse(true_fail_code1,
"neutron/tests/test_assert.py"))))
self.assertEqual( self.assertEqual(
1, len(list(checks.check_assertfalse(fail_code2, 1, len(list(
"neutron/tests/test_assert.py")))) checks.check_asserttruefalse(true_fail_code2,
"neutron/tests/test_assert.py"))))
self.assertEqual( self.assertEqual(
0, len(list(checks.check_assertfalse(pass_code, 0, len(list(
"neutron/tests/test_assert.py")))) checks.check_asserttruefalse(true_pass_code,
"neutron/tests/test_assert.py"))))
self.assertEqual(
1, len(list(
checks.check_asserttruefalse(false_fail_code1,
"neutron/tests/test_assert.py"))))
self.assertEqual(
1, len(list(
checks.check_asserttruefalse(false_fail_code2,
"neutron/tests/test_assert.py"))))
self.assertFalse(
list(
checks.check_asserttruefalse(false_pass_code,
"neutron/tests/test_assert.py")))
def test_assertempty(self): def test_assertempty(self):
fail_code = """ fail_code = """