diff --git a/HACKING.rst b/HACKING.rst index 5ce778aa832..cdaec944ee7 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -7,7 +7,6 @@ Cinder Style Commandments Cinder Specific Commandments ---------------------------- -- [N314] Check for vi editor configuration in source files. - [N322] Ensure default arguments are not mutable. - [N323] Add check for explicit import of _() to ensure proper translation. - [N336] Must use a dict comprehension instead of a dict constructor with a diff --git a/cinder/tests/hacking/checks.py b/cinder/tests/hacking/checks.py index f52104506b6..291e65fcca9 100644 --- a/cinder/tests/hacking/checks.py +++ b/cinder/tests/hacking/checks.py @@ -15,6 +15,7 @@ import ast import re +from hacking import core import six """ @@ -41,7 +42,6 @@ translated_log = re.compile( r"(.)*LOG\.(audit|debug|error|info|warn|warning|critical|exception)" r"\(\s*_\(\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_multi = re.compile(r"(.)*i18n\s+import(.)* _, (.)*") # We need this for cases where they have created their own _ function. @@ -102,20 +102,7 @@ class BaseASTChecker(ast.NodeVisitor): return False -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. - - 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" - - +@core.flake8ext def no_translate_logs(logical_line, filename): """Check for 'LOG.*(_(' @@ -132,12 +119,14 @@ def no_translate_logs(logical_line, filename): yield(0, "C312: Log messages should not be translated!") +@core.flake8ext def no_mutable_default_args(logical_line): msg = "N322: Method's default argument shouldn't be mutable!" if mutable_default_args.match(logical_line): yield (0, msg) +@core.flake8ext def check_explicit_underscore_import(logical_line, filename): """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.' LOG_METHODS = [ 'debug', 'info', @@ -232,6 +224,9 @@ class CheckOptRegistrationArgs(BaseASTChecker): 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 ' 'must be a single option or list/tuple of options ' 'respectively. Options must also end with _opt or _opts ' @@ -303,6 +298,7 @@ class CheckOptRegistrationArgs(BaseASTChecker): return super(CheckOptRegistrationArgs, self).generic_visit(node) +@core.flake8ext def check_datetime_now(logical_line, noqa): if noqa: return @@ -316,6 +312,7 @@ def check_datetime_now(logical_line, noqa): _UNICODE_USAGE_REGEX = re.compile(r'\bunicode *\(') +@core.flake8ext def check_unicode_usage(logical_line, noqa): if noqa: return @@ -326,6 +323,7 @@ def check_unicode_usage(logical_line, noqa): yield(0, msg) +@core.flake8ext def check_no_print_statements(logical_line, filename, noqa): # CLI and utils programs do need to use 'print()' so # we shouldn't check those files. @@ -342,6 +340,7 @@ def check_no_print_statements(logical_line, filename, noqa): yield(0, msg) +@core.flake8ext def check_timeutils_strtime(logical_line): msg = ("C306: Found timeutils.strtime(). " "Please use datetime.datetime.isoformat() or datetime.strftime()") @@ -349,6 +348,7 @@ def check_timeutils_strtime(logical_line): yield(0, msg) +@core.flake8ext def dict_constructor_with_list_copy(logical_line): msg = ("N336: Must use a dict comprehension instead of a dict constructor " "with a sequence of key-value pairs.") @@ -356,6 +356,7 @@ def dict_constructor_with_list_copy(logical_line): yield (0, msg) +@core.flake8ext def check_timeutils_isotime(logical_line): msg = ("C308: Found timeutils.isotime(). " "Please use datetime.datetime.isoformat()") @@ -363,6 +364,7 @@ def check_timeutils_isotime(logical_line): yield(0, msg) +@core.flake8ext def no_test_log(logical_line, filename, noqa): if ('cinder/tests' not in filename or noqa): return @@ -371,6 +373,7 @@ def no_test_log(logical_line, filename, noqa): yield (0, msg) +@core.flake8ext def validate_assertTrue(logical_line, filename): # Note: a comparable check cannot be implemented for # assertFalse(), because assertFalse(None) passes. @@ -383,20 +386,3 @@ def validate_assertTrue(logical_line, filename): msg = ("C313: Unit tests should use assertTrue(value) instead" " of using assertEqual(True, value).") 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) diff --git a/cinder/tests/unit/test_hacking.py b/cinder/tests/unit/test_hacking.py index 04e1e47d8c0..1a51bf15095 100644 --- a/cinder/tests/unit/test_hacking.py +++ b/cinder/tests/unit/test_hacking.py @@ -56,28 +56,6 @@ class HackingTestCase(test.TestCase): 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): self.assertEqual(1, len(list(checks.no_translate_logs( "LOG.debug(_('foo'))", "cinder/scheduler/foo.py")))) diff --git a/tox.ini b/tox.ini index c2eb0cbf973..4c2999b8900 100644 --- a/tox.ini +++ b/tox.ini @@ -200,9 +200,25 @@ application-import-names = cinder import-order-style = pep8 [hacking] -local-check-factory = cinder.tests.hacking.checks.factory 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] 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