Merge "Re-enable local hacking checks"

This commit is contained in:
Zuul 2020-03-31 15:18:03 +00:00 committed by Gerrit Code Review
commit d4ea848e22
4 changed files with 35 additions and 56 deletions

View File

@ -7,7 +7,6 @@ Cinder Style Commandments
Cinder Specific Commandments Cinder Specific Commandments
---------------------------- ----------------------------
- [N314] Check for vi editor configuration in source files.
- [N322] Ensure default arguments are not mutable. - [N322] Ensure default arguments are not mutable.
- [N323] Add check for explicit import of _() to ensure proper translation. - [N323] Add check for explicit import of _() to ensure proper translation.
- [N336] Must use a dict comprehension instead of a dict constructor with a - [N336] Must use a dict comprehension instead of a dict constructor with a

View File

@ -15,6 +15,7 @@
import ast import ast
import re import re
from hacking import core
import six import six
""" """
@ -41,7 +42,6 @@ translated_log = re.compile(
r"(.)*LOG\.(audit|debug|error|info|warn|warning|critical|exception)" r"(.)*LOG\.(audit|debug|error|info|warn|warning|critical|exception)"
r"\(\s*_\(\s*('|\")") r"\(\s*_\(\s*('|\")")
string_translation = re.compile(r"(.)*_\(\s*('|\")") string_translation = re.compile(r"(.)*_\(\s*('|\")")
vi_header_re = re.compile(r"^#\s+vim?:.+")
underscore_import_check = re.compile(r"(.)*i18n\s+import(.)* _$") underscore_import_check = re.compile(r"(.)*i18n\s+import(.)* _$")
underscore_import_check_multi = re.compile(r"(.)*i18n\s+import(.)* _, (.)*") underscore_import_check_multi = re.compile(r"(.)*i18n\s+import(.)* _, (.)*")
# We need this for cases where they have created their own _ function. # We need this for cases where they have created their own _ function.
@ -102,20 +102,7 @@ class BaseASTChecker(ast.NodeVisitor):
return False return False
def no_vi_headers(physical_line, line_number, lines): @core.flake8ext
"""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.
N314
"""
# NOTE(gilliard): line_number is 1-indexed
if line_number <= 5 or line_number > len(lines) - 5:
if vi_header_re.match(physical_line):
return 0, "N314: Don't put vi configuration in source files"
def no_translate_logs(logical_line, filename): def no_translate_logs(logical_line, filename):
"""Check for 'LOG.*(_(' """Check for 'LOG.*(_('
@ -132,12 +119,14 @@ def no_translate_logs(logical_line, filename):
yield(0, "C312: Log messages should not be translated!") yield(0, "C312: Log messages should not be translated!")
@core.flake8ext
def no_mutable_default_args(logical_line): def no_mutable_default_args(logical_line):
msg = "N322: Method's default argument shouldn't be mutable!" msg = "N322: 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)
@core.flake8ext
def check_explicit_underscore_import(logical_line, filename): def check_explicit_underscore_import(logical_line, filename):
"""Check for explicit import of the _ function """Check for explicit import of the _ function
@ -170,6 +159,9 @@ class CheckLoggingFormatArgs(BaseASTChecker):
""" """
name = 'check_logging_format_args'
version = '1.0'
CHECK_DESC = 'C310 Log method arguments should not be a tuple.' CHECK_DESC = 'C310 Log method arguments should not be a tuple.'
LOG_METHODS = [ LOG_METHODS = [
'debug', 'info', 'debug', 'info',
@ -232,6 +224,9 @@ class CheckOptRegistrationArgs(BaseASTChecker):
opts when register_opt() or register_opts() are being called. opts when register_opt() or register_opts() are being called.
""" """
name = 'check_opt_registrationg_args'
version = '1.0'
CHECK_DESC = ('C311: Arguments being passed to register_opt/register_opts ' CHECK_DESC = ('C311: Arguments being passed to register_opt/register_opts '
'must be a single option or list/tuple of options ' 'must be a single option or list/tuple of options '
'respectively. Options must also end with _opt or _opts ' 'respectively. Options must also end with _opt or _opts '
@ -303,6 +298,7 @@ class CheckOptRegistrationArgs(BaseASTChecker):
return super(CheckOptRegistrationArgs, self).generic_visit(node) return super(CheckOptRegistrationArgs, self).generic_visit(node)
@core.flake8ext
def check_datetime_now(logical_line, noqa): def check_datetime_now(logical_line, noqa):
if noqa: if noqa:
return return
@ -316,6 +312,7 @@ def check_datetime_now(logical_line, noqa):
_UNICODE_USAGE_REGEX = re.compile(r'\bunicode *\(') _UNICODE_USAGE_REGEX = re.compile(r'\bunicode *\(')
@core.flake8ext
def check_unicode_usage(logical_line, noqa): def check_unicode_usage(logical_line, noqa):
if noqa: if noqa:
return return
@ -326,6 +323,7 @@ def check_unicode_usage(logical_line, noqa):
yield(0, msg) yield(0, msg)
@core.flake8ext
def check_no_print_statements(logical_line, filename, noqa): def check_no_print_statements(logical_line, filename, noqa):
# CLI and utils programs do need to use 'print()' so # CLI and utils programs do need to use 'print()' so
# we shouldn't check those files. # we shouldn't check those files.
@ -342,6 +340,7 @@ def check_no_print_statements(logical_line, filename, noqa):
yield(0, msg) yield(0, msg)
@core.flake8ext
def check_timeutils_strtime(logical_line): def check_timeutils_strtime(logical_line):
msg = ("C306: Found timeutils.strtime(). " msg = ("C306: Found timeutils.strtime(). "
"Please use datetime.datetime.isoformat() or datetime.strftime()") "Please use datetime.datetime.isoformat() or datetime.strftime()")
@ -349,6 +348,7 @@ def check_timeutils_strtime(logical_line):
yield(0, msg) yield(0, msg)
@core.flake8ext
def dict_constructor_with_list_copy(logical_line): def dict_constructor_with_list_copy(logical_line):
msg = ("N336: Must use a dict comprehension instead of a dict constructor " msg = ("N336: Must use a dict comprehension instead of a dict constructor "
"with a sequence of key-value pairs.") "with a sequence of key-value pairs.")
@ -356,6 +356,7 @@ def dict_constructor_with_list_copy(logical_line):
yield (0, msg) yield (0, msg)
@core.flake8ext
def check_timeutils_isotime(logical_line): def check_timeutils_isotime(logical_line):
msg = ("C308: Found timeutils.isotime(). " msg = ("C308: Found timeutils.isotime(). "
"Please use datetime.datetime.isoformat()") "Please use datetime.datetime.isoformat()")
@ -363,6 +364,7 @@ def check_timeutils_isotime(logical_line):
yield(0, msg) yield(0, msg)
@core.flake8ext
def no_test_log(logical_line, filename, noqa): def no_test_log(logical_line, filename, noqa):
if ('cinder/tests' not in filename or noqa): if ('cinder/tests' not in filename or noqa):
return return
@ -371,6 +373,7 @@ def no_test_log(logical_line, filename, noqa):
yield (0, msg) yield (0, msg)
@core.flake8ext
def validate_assertTrue(logical_line, filename): def validate_assertTrue(logical_line, filename):
# Note: a comparable check cannot be implemented for # Note: a comparable check cannot be implemented for
# assertFalse(), because assertFalse(None) passes. # assertFalse(), because assertFalse(None) passes.
@ -383,20 +386,3 @@ def validate_assertTrue(logical_line, filename):
msg = ("C313: Unit tests should use assertTrue(value) instead" msg = ("C313: Unit tests should use assertTrue(value) instead"
" of using assertEqual(True, value).") " of using assertEqual(True, value).")
yield(0, msg) yield(0, msg)
def factory(register):
register(no_vi_headers)
register(no_translate_logs)
register(no_mutable_default_args)
register(check_explicit_underscore_import)
register(CheckLoggingFormatArgs)
register(CheckOptRegistrationArgs)
register(check_datetime_now)
register(check_timeutils_strtime)
register(check_timeutils_isotime)
register(check_unicode_usage)
register(check_no_print_statements)
register(dict_constructor_with_list_copy)
register(no_test_log)
register(validate_assertTrue)

View File

@ -56,28 +56,6 @@ class HackingTestCase(test.TestCase):
should pass. 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']
self.assertIsNone(checks.no_vi_headers(
"Test string foo", 1, lines))
self.assertEqual(2, len(list(checks.no_vi_headers(
"# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
2, lines))))
self.assertEqual(2, len(list(checks.no_vi_headers(
"# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
8, lines))))
self.assertIsNone(checks.no_vi_headers(
"Test end string for vi",
9, lines))
# vim header outside of boundary (first/last 5 lines)
self.assertIsNone(checks.no_vi_headers(
"# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
6, lines))
def test_no_translate_logs(self): def test_no_translate_logs(self):
self.assertEqual(1, len(list(checks.no_translate_logs( self.assertEqual(1, len(list(checks.no_translate_logs(
"LOG.debug(_('foo'))", "cinder/scheduler/foo.py")))) "LOG.debug(_('foo'))", "cinder/scheduler/foo.py"))))

18
tox.ini
View File

@ -200,9 +200,25 @@ application-import-names = cinder
import-order-style = pep8 import-order-style = pep8
[hacking] [hacking]
local-check-factory = cinder.tests.hacking.checks.factory
import_exceptions = cinder.i18n import_exceptions = cinder.i18n
[flake8:local-plugins]
extension =
C312 = checks:no_translate_logs
N322 = checks:no_mutable_default_args
N323 = checks:check_explicit_underscore_import
C310 = checks:CheckLoggingFormatArgs
C311 = checks:CheckOptRegistrationArgs
C301 = checks:check_datetime_now
C306 = checks:check_timeutils_strtime
C308 = checks:check_timeutils_isotime
C302 = checks:check_unicode_usage
C303 = checks:check_no_print_statements
C336 = checks:dict_constructor_with_list_copy
C309 = checks:no_test_log
C313 = checks:validate_assertTrue
paths = ./cinder/tests/hacking
[doc8] [doc8]
ignore-path=.tox,*.egg-info,doc/src/api,doc/source/drivers.rst,doc/build,.eggs/*/EGG-INFO/*.txt,doc/source/configuration/tables,./*.txt ignore-path=.tox,*.egg-info,doc/src/api,doc/source/drivers.rst,doc/build,.eggs/*/EGG-INFO/*.txt,doc/source/configuration/tables,./*.txt
extension=.txt,.rst,.inc extension=.txt,.rst,.inc