From 74e7df482c22c3e39898bd0c55dd24352740dbe4 Mon Sep 17 00:00:00 2001 From: Henry Gessau Date: Sun, 24 Apr 2016 15:19:20 +0000 Subject: [PATCH] Revert "Switch to inheriting hacking checks from neutron-lib" This reverts commit 7e1601fcb8187ad42cdeb5f0b899253cc32458ba. Change-Id: Ib2420370d29dd40c187197c611d05d1b4a13eaae --- neutron/hacking/checks.py | 73 ++++++++++++++++++++++- neutron/tests/unit/hacking/test_checks.py | 58 +++++++++++++++++- tox.ini | 4 +- 3 files changed, 129 insertions(+), 6 deletions(-) diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index dc09f144bb5..00d49e835d8 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -15,7 +15,6 @@ import os import re -import neutron_lib.hacking.checks import pep8 import six @@ -53,6 +52,7 @@ _all_log_levels = { 'exception': '_LE', } _all_hints = set(_all_log_levels.values()) +mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") def _regex_for_level(level, hint): @@ -68,6 +68,7 @@ log_translation_hint = re.compile( log_warn = re.compile( r"(.)*LOG\.(warn)\(\s*('|\"|_)") +contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(") @flake8ext @@ -83,6 +84,29 @@ def validate_log_translations(logical_line, physical_line, filename): yield (0, msg) +@flake8ext +def use_jsonutils(logical_line, filename): + 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 + # itself, so we should not complain about them not using jsonutils + json_check_skipped_patterns = [ + "neutron/plugins/ml2/drivers/openvswitch/agent/xenapi/etc/xapi.d/" + "plugins/netwrap", + ] + + for pattern in json_check_skipped_patterns: + if pattern in filename: + return + + if "json." in logical_line: + json_funcs = ['dumps(', 'dump(', 'loads(', 'load('] + for f in json_funcs: + pos = logical_line.find('json.%s' % f) + if pos != -1: + yield (pos, msg % {'fun': f[:-1]}) + + @flake8ext def no_translate_debug_logs(logical_line, filename): """Check for 'LOG.debug(_(' and 'LOG.debug(_Lx(' @@ -123,6 +147,39 @@ def check_assert_called_once_with(logical_line, filename): yield (0, msg) +@flake8ext +def check_no_contextlib_nested(logical_line, filename): + msg = ("N324: contextlib.nested is deprecated. With Python 2.7 and later " + "the with-statement supports multiple nested objects. See https://" + "docs.python.org/2/library/contextlib.html#contextlib.nested for " + "more information.") + + if contextlib_nested.match(logical_line): + yield(0, msg) + + +@flake8ext +def check_python3_xrange(logical_line): + if re.search(r"\bxrange\s*\(", logical_line): + yield(0, "N325: Do not use xrange. Use range, or six.moves.range for " + "large loops.") + + +@flake8ext +def check_no_basestring(logical_line): + if re.search(r"\bbasestring\b", logical_line): + msg = ("N326: basestring is not Python3-compatible, use " + "six.string_types instead.") + yield(0, msg) + + +@flake8ext +def check_python3_no_iteritems(logical_line): + if re.search(r".*\.iteritems\(\)", logical_line): + msg = ("N327: Use six.iteritems() instead of dict.iteritems().") + yield(0, msg) + + @flake8ext def check_asserttrue(logical_line, filename): if 'neutron/tests/' in filename: @@ -136,6 +193,13 @@ def check_asserttrue(logical_line, filename): yield (0, msg) +@flake8ext +def no_mutable_default_args(logical_line): + msg = "N329: Method's default argument shouldn't be mutable!" + if mutable_default_args.match(logical_line): + yield (0, msg) + + @flake8ext def check_assertfalse(logical_line, filename): if 'neutron/tests/' in filename: @@ -258,11 +322,16 @@ def check_builtins_gettext(logical_line, tokens, filename, lines, noqa): def factory(register): - neutron_lib.hacking.checks.factory(register) register(validate_log_translations) + register(use_jsonutils) register(check_assert_called_once_with) register(no_translate_debug_logs) + register(check_no_contextlib_nested) + register(check_python3_xrange) + register(check_no_basestring) + register(check_python3_no_iteritems) register(check_asserttrue) + register(no_mutable_default_args) register(check_assertfalse) register(check_assertempty) register(check_assertisinstance) diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index 49c22fe9afb..8b5966309a6 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -72,6 +72,32 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual( 1, len(list(checks.no_translate_debug_logs(bad, 'f')))) + def test_use_jsonutils(self): + def __get_msg(fun): + msg = ("N321: jsonutils.%(fun)s must be used instead of " + "json.%(fun)s" % {'fun': fun}) + return [(0, msg)] + + for method in ('dump', 'dumps', 'load', 'loads'): + self.assertEqual( + __get_msg(method), + list(checks.use_jsonutils("json.%s(" % method, + "./neutron/common/rpc.py"))) + + self.assertEqual(0, + len(list(checks.use_jsonutils("jsonx.%s(" % method, + "./neutron/common/rpc.py")))) + + self.assertEqual(0, + len(list(checks.use_jsonutils("json.%sx(" % method, + "./neutron/common/rpc.py")))) + + self.assertEqual(0, + len(list(checks.use_jsonutils( + "json.%s" % method, + "./neutron/plugins/ml2/drivers/openvswitch/agent/xenapi/" + "etc/xapi.d/plugins/netwrap")))) + def test_assert_called_once_with(self): fail_code1 = """ mock = Mock() @@ -122,6 +148,23 @@ class HackingTestCase(base.BaseTestCase): 0, len(list(checks.check_assert_called_once_with(pass_code2, "neutron/tests/test_assert.py")))) + def test_check_python3_xrange(self): + f = checks.check_python3_xrange + self.assertLineFails(f, 'a = xrange(1000)') + self.assertLineFails(f, 'b =xrange ( 42 )') + self.assertLineFails(f, 'c = xrange(1, 10, 2)') + self.assertLinePasses(f, 'd = range(1000)') + self.assertLinePasses(f, 'e = six.moves.range(1337)') + + def test_no_basestring(self): + self.assertEqual(1, + len(list(checks.check_no_basestring("isinstance(x, basestring)")))) + + def test_check_python3_iteritems(self): + f = checks.check_python3_no_iteritems + self.assertLineFails(f, "d.iteritems()") + self.assertLinePasses(f, "six.iteritems(d)") + def test_asserttrue(self): fail_code1 = """ test_bool = True @@ -145,6 +188,19 @@ class HackingTestCase(base.BaseTestCase): 0, len(list(checks.check_asserttrue(pass_code, "neutron/tests/test_assert.py")))) + def test_no_mutable_default_args(self): + self.assertEqual(1, len(list(checks.no_mutable_default_args( + " def fake_suds_context(calls={}):")))) + + self.assertEqual(1, len(list(checks.no_mutable_default_args( + "def get_info_from_bdm(virt_type, bdm, mapping=[])")))) + + self.assertEqual(0, len(list(checks.no_mutable_default_args( + "defined = []")))) + + self.assertEqual(0, len(list(checks.no_mutable_default_args( + "defined, undefined = [], {}")))) + def test_assertfalse(self): fail_code1 = """ test_bool = False @@ -288,7 +344,7 @@ def load_tests(loader, tests, pattern): flake8_style = engine.get_style_guide(parse_argv=False, # Ignore H104 otherwise it's # raised on doctests. - ignore=('F', 'H104', 'N530')) + ignore=('F', 'H104')) options = flake8_style.options for name, check in checks.__dict__.items(): diff --git a/tox.ini b/tox.ini index ce609602fb7..0b3d0838cc9 100644 --- a/tox.ini +++ b/tox.ini @@ -123,9 +123,7 @@ commands = sphinx-build -W -b html doc/source doc/build/html # E265 block comment should start with ‘# ‘ # H404 multi line docstring should start with a summary # H405 multi line docstring summary not separated with an empty line -# The following is a permanent exception, as that rule is for subprojects -# N530 direct neutron imports not allowed (oh yes, neutron is allowed!) -ignore = E125,E126,E128,E129,E265,H404,H405,N530 +ignore = E125,E126,E128,E129,E265,H404,H405 show-source = true exclude = ./.*,build,dist,neutron/openstack/common/*