openstack/neutron-lib
Code Issues Proposed changes

Revert "Update hacking check consumption"

Browse Source 
This reverts commit 4f318bc253.

Conflicts:
        neutron_lib/hacking/checks.py
        neutron_lib/tests/unit/hacking/test_checks.py
        test-requirements.txt

Change-Id: I383e2cfee3edac7201498f685e4c3b16f38af237
This commit is contained in:
Henry Gessau
2016-08-09 21:52:45 -04:00
parent 4f96e0e20e
commit 27cfad9210
6 changed files with 16 additions and 335 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
doc/source/usage.rst
View File

@@ -5,67 +5,3 @@ Usage
To use neutron-lib in a project:: To use neutron-lib in a project::
import neutron_lib import neutron_lib
Hacking Checks
--------------
The ``neutron_lib.hacking`` package implements a number of public
`hacking checks <https://github.com/openstack-dev/hacking>`_ that
can be categorized as follows:
Project specific hacking checks
-------------------------------
These hacking checks are intended for validating neutron-lib source
code as part of our pep8 checks. Adopters need not run these checks
and thus a private hacking check factory is used within neutron-lib's
hacking ``tox.ini`` configuration.
General purpose hacking checks
------------------------------
Hacking checks that are shared by two or more consuming projects often
end up in neutron-lib as a general purpose shared hacking check so that
there's a single source for consumption.
All hacking checks in neutron-lib are registered via entry points and are
therefore available via ``flake8`` directly in any environment where
neutron-lib is installed. However, these checks registered via entry points
are disabled by default and therefore must be selectively enabled by
consumers wishing to utilize them.
To selectively enable checks consumers must use ``flake8`` ``select`` to
signify the checks to enable and run.
For example in your ``tox.ini``::
[flake8]
select = N530,N531
Via CLI::
flake8 --select N530,N531 /path/to/src
Adopter hacking checks
----------------------
A subset of checks provided by neutron-lib are intended to validate the
"compliance" of neutron-lib adopter's source code. Consumers can configure
to run the latest set of compliance hacking checks by configuring their
``tox.ini`` as follows::
[hacking]
local-check-factory = neutron_lib.hacking.checks.latest_adopter_hacking_checks
The set of hacking checks registered via ``latest_adopter_hacking_checks``
is dynamic and may change from release to release. Consumer's who are not fully
complaint and therefore cannot pass all adopter hacking checks can selectively
enable checks as described in the `General purpose hacking checks`_ section herein.
Hacking Checks Implemented
--------------------------
.. include:: ../../HACKING.rst

159
neutron_lib/hacking/checks.py
View File

@@ -14,10 +14,7 @@
import re import re
from debtcollector import moves
from debtcollector import removals from debtcollector import removals
from hacking import core
import pep8
from neutron_lib.hacking import translation_checks from neutron_lib.hacking import translation_checks
@@ -41,10 +38,7 @@ namespace_imports_from_root = re.compile(r"from[\s]+([\w]+)[\s]+import[\s]+")
contextlib_nested = re.compile(r"^\s*with (contextlib\.)?nested\(") contextlib_nested = re.compile(r"^\s*with (contextlib\.)?nested\(")
@core.flake8ext
@core.off_by_default
def use_jsonutils(logical_line, filename): def use_jsonutils(logical_line, filename):
"""N521 - jsonutils must be used instead of json."""
msg = "N521: jsonutils.%(fun)s must be used instead of json.%(fun)s" msg = "N521: 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
@@ -101,19 +95,13 @@ def _check_namespace_imports(failure_code, namespace, new_ns, logical_line,
@removals.remove(removal_version='P release') @removals.remove(removal_version='P release')
@core.flake8ext
@core.off_by_default
def check_oslo_namespace_imports(logical_line): def check_oslo_namespace_imports(logical_line):
"""N523 - Import oslo_ rather than oslo."""
x = _check_namespace_imports('N523', 'oslo', 'oslo_', logical_line) x = _check_namespace_imports('N523', 'oslo', 'oslo_', logical_line)
if x is not None: if x is not None:
yield x yield x
@core.flake8ext
@core.off_by_default
def check_no_contextlib_nested(logical_line, filename): def check_no_contextlib_nested(logical_line, filename):
"""N524 - Use of contextlib.nested is deprecated."""
msg = ("N524: contextlib.nested is deprecated. With Python 2.7 and later " msg = ("N524: 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 "
@@ -123,29 +111,20 @@ def check_no_contextlib_nested(logical_line, filename):
yield(0, msg) yield(0, msg)
@core.flake8ext
@core.off_by_default
def check_python3_xrange(logical_line): def check_python3_xrange(logical_line):
"""N525 - Do not use xrange."""
if re.search(r"\bxrange\s*\(", logical_line): if re.search(r"\bxrange\s*\(", logical_line):
yield(0, "N525: Do not use xrange. Use range, or six.moves.range for " yield(0, "N525: Do not use xrange. Use range, or six.moves.range for "
"large loops.") "large loops.")
@core.flake8ext
@core.off_by_default
def check_no_basestring(logical_line): def check_no_basestring(logical_line):
"""N526 - basestring is not Python3-compatible."""
if re.search(r"\bbasestring\b", logical_line): if re.search(r"\bbasestring\b", logical_line):
msg = ("N526: basestring is not Python3-compatible, use " msg = ("N526: basestring is not Python3-compatible, use "
"six.string_types instead.") "six.string_types instead.")
yield(0, msg) yield(0, msg)
@core.flake8ext
@core.off_by_default
def check_python3_no_iteritems(logical_line): def check_python3_no_iteritems(logical_line):
"""N527 - Use dict.items() instead of dict.iteritems()."""
if re.search(r".*\.iteritems\(\)", logical_line): if re.search(r".*\.iteritems\(\)", logical_line):
msg = ("N527: Use dict.items() instead of dict.iteritems() to be " msg = ("N527: Use dict.items() instead of dict.iteritems() to be "
"compatible with both Python 2 and Python 3. In Python 2, " "compatible with both Python 2 and Python 3. In Python 2, "
@@ -155,10 +134,7 @@ def check_python3_no_iteritems(logical_line):
yield(0, msg) yield(0, msg)
@core.flake8ext
@core.off_by_default
def no_mutable_default_args(logical_line): def no_mutable_default_args(logical_line):
"""N529 - Method's default argument shouldn't be mutable."""
msg = "N529: Method's default argument shouldn't be mutable!" msg = "N529: Method's default argument shouldn't be mutable!"
if mutable_default_args.match(logical_line): if mutable_default_args.match(logical_line):
yield (0, msg) yield (0, msg)
@@ -166,10 +142,8 @@ def no_mutable_default_args(logical_line):
# Chances are that most projects will need to put an ignore on this rule # Chances are that most projects will need to put an ignore on this rule
# until they can fully migrate to the lib. # until they can fully migrate to the lib.
@core.flake8ext
@core.off_by_default
def check_neutron_namespace_imports(logical_line): def check_neutron_namespace_imports(logical_line):
"""N530 - Direct neutron imports not allowed."""
x = _check_namespace_imports( x = _check_namespace_imports(
'N530', 'neutron', 'neutron_lib.', logical_line, 'N530', 'neutron', 'neutron_lib.', logical_line,
message_override="direct neutron imports not allowed") message_override="direct neutron imports not allowed")
@@ -177,8 +151,6 @@ def check_neutron_namespace_imports(logical_line):
yield x yield x
@core.flake8ext
@core.off_by_default
def check_no_eventlet_imports(logical_line): def check_no_eventlet_imports(logical_line):
"""N535 - Usage of Python eventlet module not allowed.""" """N535 - Usage of Python eventlet module not allowed."""
if re.match(r'(import|from)\s+[(]?eventlet', logical_line): if re.match(r'(import|from)\s+[(]?eventlet', logical_line):
@@ -186,119 +158,16 @@ def check_no_eventlet_imports(logical_line):
yield logical_line.index('eventlet'), msg yield logical_line.index('eventlet'), msg
ALL_CHECKS = set([use_jsonutils, def factory(register):
check_no_contextlib_nested, register(use_jsonutils)
check_python3_xrange, register(check_no_contextlib_nested)
check_no_basestring, register(check_python3_xrange)
check_python3_no_iteritems, register(check_no_basestring)
no_mutable_default_args, register(check_python3_no_iteritems)
check_neutron_namespace_imports, register(no_mutable_default_args)
translation_checks.validate_log_translations, register(check_neutron_namespace_imports)
translation_checks.no_translate_debug_logs, register(check_no_eventlet_imports)
translation_checks.check_log_warn_deprecated, register(translation_checks.validate_log_translations)
translation_checks.check_raised_localized_exceptions, register(translation_checks.no_translate_debug_logs)
check_no_eventlet_imports]) register(translation_checks.check_log_warn_deprecated)
register(translation_checks.check_raised_localized_exceptions)
_LIB_PROJECT_CHECKS = ALL_CHECKS
ADOPTER_CHECKS = ALL_CHECKS - set([check_no_eventlet_imports])
def _get_pep8_checks():
check_types = ['physical_line', 'logical_line', 'tree']
style_guide = pep8.StyleGuide()
check_reg = {}
for check_type in check_types:
for registered_check in style_guide.get_checks(check_type):
check_reg[registered_check[0]] = registered_check
return check_reg
def _register_and_enable_checks(register, checks):
"""Call register for each check; ensuring its enabled."""
check_reg = _get_pep8_checks()
for check in checks:
# NOTE(boden): checks registered via entry points already exist
# and must be enabled programmatically
check = (check_reg[check.__name__][1]
if check.__name__ in check_reg
else check)
setattr(check, 'off_by_default', False)
register(check)
def latest_adopter_hacking_checks(register):
"""Hacking check factory for neutron-lib adopter compliant checks.
This factory registers all checks neutron-lib adopters should seek to
pass. The set of checks registered is the latest set of adopter checks
and is thus subject to change from release to release.
As neutron-lib hacking checks are registered as entry points and default
to disabled, consumers have more granular control over checks by not using
this factory function and instead selecting individual checks via their
flake8/tox configuration.
This function should only be used with tox flake8 hacking targets.
:param register: The register function to call for each check.
:return: None
"""
_register_and_enable_checks(register, ADOPTER_CHECKS)
# TODO(boden): update removal_version once naming determined
factory = moves.moved_function(latest_adopter_hacking_checks,
'factory', __name__,
message='function renamed to reflect '
'explicit usage',
version='newton',
removal_version='P release')
def _neutron_lib_project_hacking_checks(register):
"""neutron-lib project specific hacking checks."""
_register_and_enable_checks(register, _LIB_PROJECT_CHECKS)
class _ProxyHackingChecks(core.GlobalCheck):
"""Flake8 extension to ensure latest off_by_default is used.
Hacking checks registered via entry point are typically set
off_by_default to True so that consumers can selectively enable them.
Subsequent factory method calls to register and enable hacking checks
go unnoticed; the check registered via entry point takes precedence by
default.
This flake8 extension is registered via entry point and performs option
handling to ensure any changes to hacking check off_by_default are
reflected in the checks ignored in the options. This allows consumers
to use our hacking check factory methods to enable checks pragmatically.
Note that if consumers use flake8 CLI with the --ignore option, the ignored
checks are not even in the list of checks returned by pep8. Therefore CLI
select/ignore still functions as expected regardless of the off_by_default
logic contained herein.
"""
name = 'enabled-hacking-check-proxy'
@classmethod
def parse_options(cls, opts):
ignore = list(opts.ignore)
# NOTE(boden): make sure options.ignore has the latest off_by_default
# from pep8 registered checks that may be set post entry-point loading
for fn_name, check_data in _get_pep8_checks().items():
check_fn = check_data[1]
enabled = not getattr(check_fn, 'off_by_default', False)
if enabled:
check_codes = pep8.ERRORCODE_REGEX.findall(
check_fn.__doc__ or '')
# Remove check's codes from default ignore list
for code in check_codes:
if code in ignore:
ignore.remove(code)
opts.ignore = tuple(ignore)

19
neutron_lib/hacking/translation_checks.py
View File

@@ -14,10 +14,8 @@
import re import re
from hacking import core
import pep8 import pep8
_all_log_levels = { _all_log_levels = {
'critical': '_LC', 'critical': '_LC',
'error': '_LE', 'error': '_LE',
@@ -50,11 +48,7 @@ def _translation_is_not_expected(filename):
return any(pat in filename for pat in ["/tests/", "rally-jobs/plugins/"]) return any(pat in filename for pat in ["/tests/", "rally-jobs/plugins/"])
@core.flake8ext
@core.off_by_default
def validate_log_translations(logical_line, physical_line, filename): def validate_log_translations(logical_line, physical_line, filename):
"""N531 - Log messages require translation hints."""
# Do not do these validations on tests
if _translation_is_not_expected(filename): if _translation_is_not_expected(filename):
return return
@@ -66,21 +60,14 @@ def validate_log_translations(logical_line, physical_line, filename):
yield (0, msg) yield (0, msg)
@core.flake8ext
@core.off_by_default
def check_log_warn_deprecated(logical_line, filename): def check_log_warn_deprecated(logical_line, filename):
"""N532 - Use LOG.warning due to compatibility with py3."""
msg = "N532: Use LOG.warning due to compatibility with py3" msg = "N532: 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)
@core.flake8ext
@core.off_by_default
def no_translate_debug_logs(logical_line, filename): def no_translate_debug_logs(logical_line, filename):
"""N533 - Don't translate debug level logs. """Check for 'LOG.debug(_(' and 'LOG.debug(_Lx('
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
@@ -94,11 +81,7 @@ def no_translate_debug_logs(logical_line, filename):
yield(0, "N533 Don't translate debug level logs") yield(0, "N533 Don't translate debug level logs")
@core.flake8ext
@core.off_by_default
def check_raised_localized_exceptions(logical_line, filename): def check_raised_localized_exceptions(logical_line, filename):
"""N534 - Untranslated exception message."""
# NOTE(boden): tox.ini doesn't permit per check exclusion
if _translation_is_not_expected(filename): if _translation_is_not_expected(filename):
return return

91
neutron_lib/tests/unit/hacking/test_checks.py
View File

@@ -10,11 +10,6 @@
# 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 contextlib
import math
import mock
import pep8
import testtools import testtools
from neutron_lib.hacking import checks from neutron_lib.hacking import checks
@@ -184,89 +179,3 @@ class HackingTestCase(base.BaseTestCase):
f = tc.check_raised_localized_exceptions f = tc.check_raised_localized_exceptions
self.assertLinePasses(f, "raise KeyError('Error text')", self.assertLinePasses(f, "raise KeyError('Error text')",
'neutron_lib/tests/unit/mytest.py') 'neutron_lib/tests/unit/mytest.py')
@contextlib.contextmanager
def _mocked_style_guide_checks(self, pep_checks):
# chunk checks into 3 sub-lists so that each type of check returns
# approximately 1/3 of the checks
list_size = int(math.ceil(float(len(pep_checks)) / 3.0))
chunks = [pep_checks[i:i + list_size]
for i in range(0, len(pep_checks), list_size)]
indexed = [[], [], []]
for i in range(3):
for c in range(len(chunks[i])):
indexed[i].append([chunks[i][c].__name__, chunks[i][c]])
# each type of check returns about 1/3 of the pep checks
check_dict = {
'physical_line': indexed[0],
'logical_line': indexed[1],
'tree': indexed[2]
}
try:
class _MockSG(object):
def get_checks(self, check_type):
return check_dict[check_type]
mock_sg = mock.patch.object(checks.pep8, 'StyleGuide', new=_MockSG)
mock_sg.start()
yield mock_sg
finally:
mock_sg.stop()
def _test_register_checks(self, to_register, factory):
for check in to_register:
setattr(check, 'off_by_default', True)
with self._mocked_style_guide_checks(to_register):
reg = []
factory(reg.append)
for check in to_register:
self.assertIn(check, reg)
self.assertFalse(getattr(check, 'off_by_default', True))
def test_latest_adopter_hacking_checks(self):
self._test_register_checks(list(checks.ADOPTER_CHECKS),
checks.latest_adopter_hacking_checks)
def test_neutron_lib_project_hacking_checks(self):
self._test_register_checks(list(checks._LIB_PROJECT_CHECKS),
checks._neutron_lib_project_hacking_checks)
def test_hacking_check_proxy(self):
class _MockOpts(object):
def __init__(self, ignore):
self.ignore = ignore or []
all_checks = list(checks.ALL_CHECKS)
with self._mocked_style_guide_checks(all_checks):
reg = []
all_codes = set([])
for codes in [pep8.ERRORCODE_REGEX.findall(f.__doc__ or '')
for f in all_checks]:
for code in codes:
all_codes.add(code)
self.assertTrue(len(all_codes) > 0)
opts = _MockOpts(all_codes)
checks._register_and_enable_checks(reg.append, all_checks)
checks._ProxyHackingChecks.parse_options(opts)
# make sure all registered checks are not ignored
self.assertEqual([], list(opts.ignore))
def test_check_eventlet_imports(self):
f = checks.check_no_eventlet_imports
self.assertLineFails(f, "import eventlet")
self.assertLineFails(f, "import eventlet.timeout")
self.assertLineFails(f, "from eventlet import timeout")
self.assertLineFails(f, "from eventlet.timeout import Timeout")
self.assertLineFails(f, "from eventlet.timeout import (Timeout, X)")
self.assertLinePasses(f, "import is.not.eventlet")
self.assertLinePasses(f, "from is.not.eventlet")
self.assertLinePasses(f, "from mymod import eventlet")
self.assertLinePasses(f, "from mymod.eventlet import amod")
self.assertLinePasses(f, 'print("eventlet not here")')
self.assertLinePasses(f, 'print("eventlet.timeout")')
self.assertLinePasses(f, "from mymod.timeout import (eventlet, X)")

16
setup.cfg
View File

@@ -45,19 +45,3 @@ input_file = neutron_lib/locale/neutron_lib.pot
keywords = _ gettext ngettext l_ lazy_gettext keywords = _ gettext ngettext l_ lazy_gettext
mapping_file = babel.cfg mapping_file = babel.cfg
output_file = neutron_lib/locale/neutron_lib.pot output_file = neutron_lib/locale/neutron_lib.pot
[entry_points]
flake8.extension =
D000 = neutron_lib.hacking.checks:_ProxyHackingChecks
N521 = neutron_lib.hacking.checks:use_jsonutils
N523 = neutron_lib.hacking.checks:check_oslo_namespace_imports
N524 = neutron_lib.hacking.checks:check_no_contextlib_nested
N525 = neutron_lib.hacking.checks:check_python3_xrange
N526 = neutron_lib.hacking.checks:check_no_basestring
N527 = neutron_lib.hacking.checks:check_python3_no_iteritems
N529 = neutron_lib.hacking.checks:no_mutable_default_args
N530 = neutron_lib.hacking.checks:check_neutron_namespace_imports
N531 = neutron_lib.hacking.translation_checks:validate_log_translations
N532 = neutron_lib.hacking.translation_checks:check_log_warn_deprecated
N533 = neutron_lib.hacking.translation_checks:no_translate_debug_logs
N534 = neutron_lib.hacking.translation_checks:check_raised_localized_exceptions

2
tox.ini
View File

@@ -50,4 +50,4 @@ exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build
[hacking] [hacking]
import_exceptions = neutron_lib._i18n import_exceptions = neutron_lib._i18n
local-check-factory = neutron_lib.hacking.checks._neutron_lib_project_hacking_checks local-check-factory = neutron_lib.hacking.checks.factory