From 2d95f8c7d0cc604df2c7fdf447800362900f0caa Mon Sep 17 00:00:00 2001 From: abhishekkekane Date: Mon, 18 Jan 2016 05:01:11 -0800 Subject: [PATCH] Restrict use of iteritems, iterkeys and itervalues Enabled hacking checks to check dict.iteritems, dict.itervalues and dict.iterkeys should not be used in the future and raise error while running pep8 using tox. Change-Id: I1239bb5c651d0193141d5bb830872caf172ec561 --- HACKING.rst | 9 +++++++ heat/hacking/checks.py | 45 +++++++++++++++++++++++++++++++++-- heat/tests/test_hacking.py | 48 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 heat/tests/test_hacking.py diff --git a/HACKING.rst b/HACKING.rst index 7bc9ed75c7..03c51a2925 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -41,3 +41,12 @@ run --parallel` will run it in parallel (this is the default incantation tox uses.) More information about testr can be found at: http://wiki.openstack.org/testr + +Heat Specific Commandments +-------------------------- + +- [Heat301] Use LOG.warning() rather than LOG.warn(). +- [Heat302] Python 3: do not use dict.iteritems. +- [Heat303] Python 3: do not use dict.iterkeys. +- [Heat304] Python 3: do not use dict.itervalues. + diff --git a/heat/hacking/checks.py b/heat/hacking/checks.py index fb2d1d2d4b..dade30bb25 100644 --- a/heat/hacking/checks.py +++ b/heat/hacking/checks.py @@ -13,17 +13,58 @@ # License for the specific language governing permissions and limitations # under the License. +import re + + +""" +Guidelines for writing new hacking checks + +- Use only for Heat specific tests. OpenStack general tests + should be submitted to the common 'hacking' module. +- Pick numbers in the range H3xx. Find the current test with + the highest allocated number and then pick the next value. +- Keep the test method code in the source file ordered based + on the Heat3xx value. +- List the new rule in the top level HACKING.rst file +- Add test cases for each new rule to heat/tests/test_hacking.py + +""" + def no_log_warn(logical_line): """Disallow 'LOG.warn(' https://bugs.launchpad.net/tempest/+bug/1508442 - H001 + Heat301 """ if logical_line.startswith('LOG.warn('): - yield(0, 'H001 Use LOG.warning() rather than LOG.warn()') + yield(0, 'Heat301 Use LOG.warning() rather than LOG.warn()') + + +def check_python3_no_iteritems(logical_line): + msg = ("Heat302: Use dict.items() instead of dict.iteritems().") + + if re.search(r".*\.iteritems\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_iterkeys(logical_line): + msg = ("Heat303: Use dict.keys() instead of dict.iterkeys().") + + if re.search(r".*\.iterkeys\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_itervalues(logical_line): + msg = ("Heat304: Use dict.values() instead of dict.itervalues().") + + if re.search(r".*\.itervalues\(\)", logical_line): + yield(0, msg) def factory(register): register(no_log_warn) + register(check_python3_no_iteritems) + register(check_python3_no_iterkeys) + register(check_python3_no_itervalues) diff --git a/heat/tests/test_hacking.py b/heat/tests/test_hacking.py new file mode 100644 index 0000000000..3c3813fb5a --- /dev/null +++ b/heat/tests/test_hacking.py @@ -0,0 +1,48 @@ +# Copyright 2016 NTT DATA. +# +# 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. + +from heat.hacking import checks +from heat.tests import common + + +class HackingTestCase(common.HeatTestCase): + 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( + "obj.items()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iteritems( + "six.iteritems(ob))")))) + + 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( + "obj.keys()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iterkeys( + "six.iterkeys(ob))")))) + + 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( + "obj.values()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_itervalues( + "six.itervalues(ob))"))))