From 53474391420cdd8c02967a862b0218b37c84b561 Mon Sep 17 00:00:00 2001 From: "Jay S. Bryant" Date: Sat, 2 Aug 2014 18:06:38 -0500 Subject: [PATCH] Improve regex for _ import hacking check Commit 3e2b1117 added a check to make sure that the _ function was being explicitly imported so that translation would work properly. I have discovered that those regexes/code would not work in some cases. Particularly if the import line imported multiple things from gettextutils or i18n. Also the check being used to find lines using the _ function was not right. This commit fixes the issues and adds appropriate tests. It also adds the hacking check to HACKING.rst which should have been done the first time around. Change-Id: I7227bb0051836e537bff2f0f97662c06452d5af6 --- HACKING.rst | 2 +- cinder/hacking/checks.py | 13 +++++++++---- cinder/tests/test_hacking.py | 12 ++++++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 4a50ad564..2c22b63ac 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -9,7 +9,7 @@ Cinder Specific Commandments ---------------------------- - [N319] Validate that debug level logs are not translated - +- [N323] Add check for explicit import of _() to ensure proper translation. General ------- - Use 'raise' instead of 'raise e' to preserve original traceback or exception being reraised:: diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py index 053cf90e5..8078d3070 100644 --- a/cinder/hacking/checks.py +++ b/cinder/hacking/checks.py @@ -31,10 +31,14 @@ Guidelines for writing new hacking checks UNDERSCORE_IMPORT_FILES = [] -log_translation = re.compile( - r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)_\(\s*('|\")") +translated_log = re.compile( + r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)" + "\(\s*_\(\s*('|\")") string_translation = re.compile(r"(.)*_\(\s*('|\")") vi_header_re = re.compile(r"^#\s+vim?:.+") +underscore_import_check = re.compile(r"(.)*import _(.)*") +# We need this for cases where they have created their own _ function. +custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*") def no_vi_headers(physical_line, line_number, lines): @@ -87,9 +91,10 @@ def check_explicit_underscore_import(logical_line, filename): # checking needed once it is found. if filename in UNDERSCORE_IMPORT_FILES: pass - elif logical_line.endswith("import _"): + elif (underscore_import_check.match(logical_line) or + custom_underscore_check.match(logical_line)): UNDERSCORE_IMPORT_FILES.append(filename) - elif(log_translation.match(logical_line) or + elif(translated_log.match(logical_line) or string_translation.match(logical_line)): yield(0, "N323: Found use of _() without explicit import of _ !") diff --git a/cinder/tests/test_hacking.py b/cinder/tests/test_hacking.py index 71793401e..dc13d4366 100644 --- a/cinder/tests/test_hacking.py +++ b/cinder/tests/test_hacking.py @@ -92,3 +92,15 @@ class HackingTestCase(test.TestCase): self.assertEqual(len(list(checks.check_explicit_underscore_import( "msg = _('My message')", "cinder/tests/other_files.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "from cinder.i18n import _, _LW", + "cinder/tests/other_files2.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "msg = _('My message')", + "cinder/tests/other_files2.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "_ = translations.ugettext", + "cinder/tests/other_files3.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "msg = _('My message')", + "cinder/tests/other_files3.py"))), 0) -- 2.45.2