From 024e9b1fd9acd74724dfa00392721de521c366d4 Mon Sep 17 00:00:00 2001 From: Eyal Date: Sun, 12 Nov 2017 16:41:18 +0200 Subject: [PATCH] add hacking module Change-Id: I7f89cade9fbb9e24a3acc7783ad67820d8cfcabb --- tox.ini | 3 + .../collectd/collectd_vitrage/plugin.py | 2 +- vitrage/evaluator/template_data.py | 4 +- vitrage/graph/utils.py | 6 +- vitrage/hacking/__init__.py | 0 vitrage/hacking/checks.py | 150 ++++++++++++++++ vitrage/tests/mocks/utils.py | 3 +- vitrage/tests/unit/hacking/__init__.py | 0 vitrage/tests/unit/hacking/test_hacking.py | 168 ++++++++++++++++++ 9 files changed, 327 insertions(+), 9 deletions(-) create mode 100644 vitrage/hacking/__init__.py create mode 100644 vitrage/hacking/checks.py create mode 100644 vitrage/tests/unit/hacking/__init__.py create mode 100644 vitrage/tests/unit/hacking/test_hacking.py diff --git a/tox.ini b/tox.ini index 29975e9f8..5fb6ee884 100644 --- a/tox.ini +++ b/tox.ini @@ -48,6 +48,9 @@ enable-extensions=H106,H203 builtins = _ exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build +[hacking] +local-check-factory = vitrage.hacking.checks.factory +import_exceptions = vitrage.i18n [testenv:releasenotes] commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html diff --git a/vitrage/datasources/collectd/collectd_vitrage/plugin.py b/vitrage/datasources/collectd/collectd_vitrage/plugin.py index 6180da640..44752b1ae 100644 --- a/vitrage/datasources/collectd/collectd_vitrage/plugin.py +++ b/vitrage/datasources/collectd/collectd_vitrage/plugin.py @@ -51,7 +51,7 @@ class CollectDPlugin(object): else: return config.key, config.values[0] - return dict([config_to_tuple(config)]) + return {k: v for k, v in [config_to_tuple(config)]} def error(self, message): """Log an error message to the collectd logger. """ diff --git a/vitrage/evaluator/template_data.py b/vitrage/evaluator/template_data.py index 046016ab7..97184f9ff 100644 --- a/vitrage/evaluator/template_data.py +++ b/vitrage/evaluator/template_data.py @@ -228,8 +228,8 @@ class TemplateData(object): def _extract_properties(var_dict): ignore_ids = [TFields.TEMPLATE_ID, TFields.SOURCE, TFields.TARGET] - return dict((key, var_dict[key]) for key in var_dict - if key not in ignore_ids) + return \ + {key: var_dict[key] for key in var_dict if key not in ignore_ids} @staticmethod def _convert_props_with_set(properties): diff --git a/vitrage/graph/utils.py b/vitrage/graph/utils.py index 307e7bc18..9e63794d6 100644 --- a/vitrage/graph/utils.py +++ b/vitrage/graph/utils.py @@ -72,8 +72,7 @@ def create_vertex(vitrage_id, } if metadata: properties.update(metadata) - properties = dict( - (k, v) for k, v in properties.items() if v is not None) + properties = {k: v for k, v in properties.items() if v is not None} vertex = Vertex(vertex_id=vitrage_id, properties=properties) return vertex @@ -107,8 +106,7 @@ def create_edge(source_id, } if metadata: properties.update(metadata) - properties = dict( - (k, v) for k, v in properties.items() if v is not None) + properties = {k: v for k, v in properties.items() if v is not None} edge = Edge(source_id=source_id, target_id=target_id, label=relationship_type, diff --git a/vitrage/hacking/__init__.py b/vitrage/hacking/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/vitrage/hacking/checks.py b/vitrage/hacking/checks.py new file mode 100644 index 000000000..5925a239c --- /dev/null +++ b/vitrage/hacking/checks.py @@ -0,0 +1,150 @@ +# Copyright 2017 - Nokia +# 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 + +mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") + +asse_trueinst_re = re.compile( + r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, " + "(\w|\.|\'|\"|\[|\])+\)\)") +asse_equal_type_re = re.compile( + r"(.)*assertEqual\(type\((\w|\.|\'|\"|\[|\])+\), " + "(\w|\.|\'|\"|\[|\])+\)") +asse_equal_end_with_none_re = re.compile( + r"(.)*assertEqual\((\w|\.|\'|\"|\[|\])+, None\)") +asse_equal_start_with_none_re = re.compile( + r"(.)*assertEqual\(None, (\w|\.|\'|\"|\[|\])+\)") +unicode_func_re = re.compile(r"(\s|\W|^)unicode\(") + +_all_log_levels = {'debug', 'error', 'info', 'warning', + 'critical', 'exception'} +# Since _Lx have been removed, we just need to check _() +translated_logs = re.compile( + r"(.)*LOG\.(%(level)s)\(\s*_\(" % {'level': '|'.join(_all_log_levels)}) + +dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") + + +def assert_true_instance(logical_line): + """Check for assertTrue(isinstance(a, b)) sentences + + V316 + """ + if asse_trueinst_re.match(logical_line): + yield (0, "V316: assertTrue(isinstance(a, b)) sentences not allowed") + + +def assert_equal_type(logical_line): + """Check for assertEqual(type(A), B) sentences + + V317 + """ + if asse_equal_type_re.match(logical_line): + yield (0, "V317: assertEqual(type(A), B) sentences not allowed") + + +def assert_equal_none(logical_line): + """Check for assertEqual(A, None) or assertEqual(None, A) sentences + + V318 + """ + res = (asse_equal_start_with_none_re.match(logical_line) or + asse_equal_end_with_none_re.match(logical_line)) + if res: + yield (0, "V318: assertEqual(A, None) or assertEqual(None, A) " + "sentences not allowed") + + +def no_translate_logs(logical_line): + """Check for use of LOG.*(_( + + V319 + """ + if translated_logs.match(logical_line): + yield (0, "V319: Don't translate logs") + + +def no_direct_use_of_unicode_function(logical_line): + """Check for use of unicode() builtin + + V320 + """ + if unicode_func_re.match(logical_line): + yield(0, "V320: Use six.text_type() instead of unicode()") + + +def check_no_contextlib_nested(logical_line): + msg = ("V327: contextlib.nested is deprecated since Python 2.7. See " + "https://docs.python.org/2/library/contextlib.html#contextlib." + "nested for more information.") + if ("with contextlib.nested(" in logical_line or + "with nested(" in logical_line): + yield(0, msg) + + +def dict_constructor_with_list_copy(logical_line): + msg = ("V328: Must use a dict comprehension instead of a dict constructor " + "with a sequence of key-value pairs.") + if dict_constructor_with_list_copy_re.match(logical_line): + yield (0, msg) + + +def check_python3_xrange(logical_line): + if re.search(r"\bxrange\s*\(", logical_line): + yield(0, "V329: Do not use xrange. Use range, or six.moves.range for " + "large loops.") + + +def check_python3_no_iteritems(logical_line): + msg = ("V330: Use six.iteritems() or dict.items() instead of " + "dict.iteritems().") + if re.search(r".*\.iteritems\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_iterkeys(logical_line): + msg = ("V331: Use six.iterkeys() or dict.keys() instead of " + "dict.iterkeys().") + if re.search(r".*\.iterkeys\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_itervalues(logical_line): + msg = ("V332: Use six.itervalues() or dict.values instead of " + "dict.itervalues().") + if re.search(r".*\.itervalues\(\)", logical_line): + yield(0, msg) + + +def no_mutable_default_args(logical_line): + msg = "V529: Method's default argument shouldn't be mutable!" + if mutable_default_args.match(logical_line): + yield (0, msg) + + +def factory(register): + register(assert_true_instance) + register(assert_equal_type) + register(assert_equal_none) + register(no_translate_logs) + register(no_direct_use_of_unicode_function) + register(no_mutable_default_args) + register(check_no_contextlib_nested) + register(dict_constructor_with_list_copy) + register(check_python3_xrange) + register(check_python3_no_iteritems) + register(check_python3_no_iterkeys) + register(check_python3_no_itervalues) diff --git a/vitrage/tests/mocks/utils.py b/vitrage/tests/mocks/utils.py index 6e2d15d25..38b7bc265 100644 --- a/vitrage/tests/mocks/utils.py +++ b/vitrage/tests/mocks/utils.py @@ -105,8 +105,7 @@ def generate_vals(param_specs): """ if isinstance(param_specs, dict): - current_info = \ - dict((k, generate_vals(v)) for k, v in param_specs.items()) + current_info = {k: generate_vals(v) for k, v in param_specs.items()} elif isinstance(param_specs, list) or isinstance(param_specs, tuple): # convert tuples to lists current_info = [generate_vals(param) for param in param_specs] diff --git a/vitrage/tests/unit/hacking/__init__.py b/vitrage/tests/unit/hacking/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/vitrage/tests/unit/hacking/test_hacking.py b/vitrage/tests/unit/hacking/test_hacking.py new file mode 100644 index 000000000..9d9a0642a --- /dev/null +++ b/vitrage/tests/unit/hacking/test_hacking.py @@ -0,0 +1,168 @@ +# Copyright 2016 - Nokia +# Copyright 2014 IBM Corp. +# +# 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 inspect + +from vitrage.hacking import checks +from vitrage.tests import base + + +class HackingTestCase(base.BaseTest): + 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_type(self): + self.assertEqual(1, len(list(checks.assert_equal_type( + "self.assertEqual(type(als['QuicAssist']), list)")))) + + self.assertEqual( + 0, len(list(checks.assert_equal_type("self.assertTrue()")))) + + def test_assert_equal_none(self): + self.assertEqual(1, len(list(checks.assert_equal_none( + "self.assertEqual(A, None)")))) + + self.assertEqual(1, len(list(checks.assert_equal_none( + "self.assertEqual(None, A)")))) + + self.assertEqual( + 0, len(list(checks.assert_equal_none("self.assertIsNone()")))) + + def test_no_translate_logs(self): + for log in checks._all_log_levels: + bad = 'LOG.%s(_("Bad"))' % log + self.assertEqual(1, len(list(checks.no_translate_logs(bad)))) + # Catch abuses when used with a variable and not a literal + bad = 'LOG.%s(_(msg))' % log + self.assertEqual(1, len(list(checks.no_translate_logs(bad)))) + + def test_no_direct_use_of_unicode_function(self): + self.assertEqual(1, len(list(checks.no_direct_use_of_unicode_function( + "unicode('the party don't start til the unicode walks in')")))) + self.assertEqual(1, len(list(checks.no_direct_use_of_unicode_function( + """unicode('something ' + 'something else""")))) + self.assertEqual(0, len(list(checks.no_direct_use_of_unicode_function( + "six.text_type('party over')")))) + self.assertEqual(0, len(list(checks.no_direct_use_of_unicode_function( + "not_actually_unicode('something completely different')")))) + + def test_no_contextlib_nested(self): + self.assertEqual(1, len(list(checks.check_no_contextlib_nested( + "with contextlib.nested(")))) + + self.assertEqual(1, len(list(checks.check_no_contextlib_nested( + "with nested(")))) + + self.assertEqual(0, len(list(checks.check_no_contextlib_nested( + "with foo as bar")))) + + def test_dict_constructor_with_list_copy(self): + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dict([(i, connect_info[i])")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " attrs = dict([(k, _from_json(v))")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " type_names = dict((value, key) for key, value in")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dict((value, key) for key, value in")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + "foo(param=dict((k, v) for k, v in bar.items()))")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dict([[i,i] for i in range(3)])")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dd = dict([i,i] for i in range(3))")))) + + self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy( + " create_kwargs = dict(snapshot=snapshot,")))) + + self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy( + " self._render_dict(xml, data_el, data.__dict__)")))) + + def test_check_python3_xrange(self): + func = checks.check_python3_xrange + self.assertEqual(1, len(list(func('for i in xrange(10)')))) + self.assertEqual(1, len(list(func('for i in xrange (10)')))) + self.assertEqual(0, len(list(func('for i in range(10)')))) + self.assertEqual(0, len(list(func('for i in six.moves.range(10)')))) + self.assertEqual(0, len(list(func('testxrange(10)')))) + + def test_dict_iteritems(self): + self.assertEqual(1, len(list(checks.check_python3_no_iteritems( + "obj.iteritems()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iteritems( + "six.iteritems(obj)")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iteritems( + "obj.items()")))) + + def test_dict_iterkeys(self): + self.assertEqual(1, len(list(checks.check_python3_no_iterkeys( + "obj.iterkeys()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iterkeys( + "six.iterkeys(obj)")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iterkeys( + "obj.keys()")))) + + def test_dict_itervalues(self): + self.assertEqual(1, len(list(checks.check_python3_no_itervalues( + "obj.itervalues()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_itervalues( + "six.itervalues(ob)")))) + + self.assertEqual(0, len(list(checks.check_python3_no_itervalues( + "obj.values()")))) + + 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_factory(self): + class Register(object): + def __init__(self): + self.funcs = [] + + def __call__(self, func): + self.funcs.append(func) + + register = Register() + checks.factory(register) + for name, func in inspect.getmembers(checks, inspect.isfunction): + if name != 'factory': + self.assertIn(func, register.funcs)