Enable global hacking checks and remove local checks

In newer hacking version 0.12.0 [1], we can enable some of the
non-default hacking rules (one by one), which are disabled by
default. The enabled rules are the following:

* [H106] Don’t put vim configuration in source files (off by default).
* [H203] Use assertIs(Not)None to check for None (off by default).
* [H904] Delay string interpolations at logging calls (off by default).

Enabled these hacking rules by adding them in the list of
'enable-extensions' in tox.ini [flake8] section. Removed the local
implementation of those hacking rules from hacking/checks.py.

The test-requirements.txt is already updated to use the newer
hacking version 0.12.0 with this commit:
cc44a33f3d

[1] See "Enabling off-by-default checks" section:
    https://pypi.python.org/pypi/hacking/0.12.0

Change-Id: Ieccd5a84ebd80ba3313016c9caeb036eaa37769b
This commit is contained in:
Dinesh Bhor 2017-07-14 12:14:33 +05:30
parent b26ba43f72
commit 66c607c6e8
4 changed files with 6 additions and 146 deletions

View File

@ -15,13 +15,10 @@ Masakari Specific Commandments
calls to datetime.datetime.utcnow() to make it easy to override its return value in tests
- [M303] capitalize help string
Config parameter help strings should have a capitalized first letter
- [M304] vim configuration should not be kept in source files.
- [M305] Change assertTrue(isinstance(A, B)) by optimal assert like
assertIsInstance(A, B).
- [M306] Change assertEqual(type(A), B) by optimal assert like
assertIsInstance(A, B)
- [M307] Change assertEqual(A, None) or assertEqual(None, A) by optimal assert like
assertIsNone(A)
- [M308] Validate that debug level logs are not translated.
- [M309] Don't import translation in tests
- [M310] Setting CONF.* attributes directly in tests is forbidden. Use
@ -45,5 +42,4 @@ Masakari Specific Commandments
- [M327] Python 3: do not use dict.iterkeys.
- [M328] Python 3: do not use dict.itervalues.
- [M329] Deprecated library function os.popen()
- [M330] String interpolation should be delayed at logging calls.
- [M331] LOG.warn is deprecated. Enforce use of LOG.warning.

View File

@ -35,7 +35,6 @@ UNDERSCORE_IMPORT_FILES = []
session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]")
cfg_re = re.compile(r".*\scfg\.")
cfg_opt_re = re.compile(r".*[\s\[]cfg\.[a-zA-Z]*Opt\(")
vi_header_re = re.compile(r"^#\s+vim?:.+")
asse_trueinst_re = re.compile(
r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
"(\w|\.|\'|\"|\[|\])+\)\)")
@ -86,9 +85,6 @@ spawn_re = re.compile(
contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(")
doubled_words_re = re.compile(
r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b")
log_string_interpolation = re.compile(r".*LOG\.(error|warning|info"
r"|critical|exception|debug)"
r"\([^,]*%[^,]*[,)]")
def no_db_session_in_public_api(logical_line, filename):
@ -124,20 +120,6 @@ def capital_cfg_help(logical_line, tokens):
yield(0, msg)
def no_vi_headers(physical_line, line_number, lines):
"""Check for vi editor configuration in source files.
By default vi modelines can only appear in the first or
last 5 lines of a source file.
M304
"""
# NOTE(abhishekk): line_number is 1-indexed
if line_number <= 5 or line_number > len(lines) - 5:
if vi_header_re.match(physical_line):
return 0, "M304: Don't put vi configuration in source files"
def assert_true_instance(logical_line):
"""Check for assertTrue(isinstance(a, b)) sentences
@ -157,18 +139,6 @@ def assert_equal_type(logical_line):
yield (0, "M306: assertEqual(type(A), B) sentences not allowed")
def assert_equal_none(logical_line):
"""Check for assertEqual(A, None) or assertEqual(None, A) sentences
M307
"""
res = (asse_equal_start_with_none_re.search(logical_line) or
asse_equal_end_with_none_re.search(logical_line))
if res:
yield (0, "M307: assertEqual(A, None) or assertEqual(None, A) "
"sentences not allowed")
def no_translate_debug_logs(logical_line, filename):
"""Check for 'LOG.debug(_('
@ -392,27 +362,6 @@ def no_os_popen(logical_line):
'Replace it using subprocess module. ')
def check_delayed_string_interpolation(logical_line, filename, noqa):
"""M330 String interpolation should be delayed at logging calls.
M330: LOG.debug('Example: %s' % 'bad')
Okay: LOG.debug('Example: %s', 'good')
"""
msg = ("M330 String interpolation should be delayed to be "
"handled by the logging code, rather than being done "
"at the point of the logging call. "
"Use ',' instead of '%'.")
if noqa:
return
if '/tests/' in filename:
return
if log_string_interpolation.match(logical_line):
yield(logical_line.index('%'), msg)
def no_log_warn(logical_line):
"""Disallow 'LOG.warn('
@ -431,11 +380,9 @@ def factory(register):
register(no_db_session_in_public_api)
register(use_timeutils_utcnow)
register(capital_cfg_help)
register(no_vi_headers)
register(no_import_translation_in_tests)
register(assert_true_instance)
register(assert_equal_type)
register(assert_equal_none)
register(assert_raises_regexp)
register(no_translate_debug_logs)
register(no_setting_conf_directly_in_tests)
@ -453,5 +400,4 @@ def factory(register):
register(check_python3_no_iterkeys)
register(check_python3_no_itervalues)
register(no_os_popen)
register(check_delayed_string_interpolation)
register(no_log_warn)

View File

@ -53,30 +53,6 @@ class HackingTestCase(test.NoDBTestCase):
just assertTrue if the check is expected to fail and assertFalse if it
should pass.
"""
def test_no_vi_headers(self):
lines = ['Line 1\n', 'Line 2\n', 'Line 3\n', 'Line 4\n', 'Line 5\n',
'Line 6\n', 'Line 7\n', 'Line 8\n', 'Line 9\n', 'Line 10\n',
'Line 11\n', 'Line 12\n', 'Line 13\n', 'Line14\n', 'Line15\n']
self.assertIsNone(checks.no_vi_headers(
"Test string foo", 1, lines))
self.assertEqual(len(list(checks.no_vi_headers(
"# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
2, lines))), 2)
self.assertIsNone(checks.no_vi_headers(
"# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
6, lines))
self.assertIsNone(checks.no_vi_headers(
"# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
9, lines))
self.assertEqual(len(list(checks.no_vi_headers(
"# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
14, lines))), 2)
self.assertIsNone(checks.no_vi_headers(
"Test end string for vi",
15, lines))
def test_assert_true_instance(self):
self.assertEqual(len(list(checks.assert_true_instance(
"self.assertTrue(isinstance(e, "
@ -129,19 +105,7 @@ class HackingTestCase(test.NoDBTestCase):
self.assertEqual(len(list(checks.assert_equal_in(
"self.assertEqual(False, any(a==1 for a in b))"))), 0)
def test_assert_equal_none(self):
self.assertEqual(len(list(checks.assert_equal_none(
"self.assertEqual(A, None)"))), 1)
self.assertEqual(len(list(checks.assert_equal_none(
"self.assertEqual(None, A)"))), 1)
self.assertEqual(
len(list(checks.assert_equal_none("self.assertIsNone()"))), 0)
def test_assert_true_or_false_with_in_or_not_in(self):
self.assertEqual(len(list(checks.assert_equal_none(
"self.assertEqual(A, None)"))), 1)
self.assertEqual(len(list(checks.assert_true_or_false_with_in(
"self.assertTrue(A in B)"))), 1)
@ -456,58 +420,6 @@ class HackingTestCase(test.NoDBTestCase):
self._assert_has_errors(code, checks.no_os_popen,
expected_errors=errors)
def test_check_delayed_string_interpolation(self):
checker = checks.check_delayed_string_interpolation
code = """
msg_w = ('Test string (%s)')
msg_i = 'Test string (%s)'
value = 'test'
LOG.error(("Test string (%s)") % value)
LOG.warning(msg_w % 'test%string')
LOG.info(msg_i %
"test%string%info")
LOG.critical(
('Test string (%s)') % value,
instance=instance)
LOG.exception((" 'Test quotation %s' \"Test\"") % 'test')
LOG.debug(' "Test quotation %s" \'Test\'' % "test")
LOG.debug('Tesing %(test)s' %
{'test': ','.join(
['%s=%s' % (name, value)
for name, value in test.items()])})
"""
expected_errors = [(5, 31, 'M330'), (6, 18, 'M330'), (7, 15, 'M330'),
(10, 25, 'M330'), (12, 46, 'M330'),
(13, 40, 'M330'), (14, 28, 'M330')]
self._assert_has_errors(code, checker, expected_errors=expected_errors)
self._assert_has_no_errors(
code, checker, filename='masakari/tests/unit/test_hacking.py')
code = """
msg_w = ('Test string (%s)')
msg_i = 'Test string (%s)'
value = 'test'
LOG.error(("Test string (%s)"), value)
LOG.error(("Test string (%s)") % value) # noqa
LOG.warn(('Test string (%s)'),
value)
LOG.info(msg_i,
"test%string%info")
LOG.critical(
('Test string (%s)'), value,
instance=instance)
LOG.exception((" 'Test quotation %s' \"Test\""), 'test')
LOG.debug(' "Test quotation %s" \'Test\'', "test")
LOG.debug('Tesing %(test)s',
{'test': ','.join(
['%s=%s' % (name, value)
for name, value in test.items()])})
"""
self._assert_has_no_errors(code, checker)
def test_no_log_warn(self):
code = """
LOG.warn("LOG.warn is deprecated")

View File

@ -61,6 +61,12 @@ commands = oslo_debug_helper {posargs}
# E123, E125 skipped as they are invalid PEP-8.
show-source = True
# The below hacking rules by default are disabled should be enabled:
# [H106] Don't put vim configuration in source files.
# [H203] Use assertIs(Not)None to check for None.
# [H904] Delay string interpolations at logging calls.
enable-extensions = H106,H203,H904
ignore = E123,E125,E128,H405
builtins = _
exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build