From a144fa3474abc6ab0e7c56c8fa8ffbaa2ed1fb86 Mon Sep 17 00:00:00 2001 From: Andreas Jaeger Date: Sun, 29 Mar 2020 12:06:04 +0200 Subject: [PATCH] Re-enable local hacking checks With the switch to hacking 2.0, the local checks are not enabled anymore, update repo to use the new way of registering local checks. Remove local vi check since hacking has this now as H106. Change-Id: I4d03f4c7eff959017e907cac974c394af0559643 --- HACKING.rst | 1 - cinder/tests/hacking/checks.py | 50 +++++++++++-------------------- cinder/tests/unit/test_hacking.py | 22 -------------- tox.ini | 18 ++++++++++- 4 files changed, 35 insertions(+), 56 deletions(-) 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