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

diff --git a/HACKING.rst b/HACKING.rst
index 4a50ad5643cacd45e61f59e4d902632ab2cddaa0..2c22b63ac2a1636bff840147189345d5866de97b 100644 (file)
--- 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 053cf90e525e49f8a35d686e51b99c1c73d19d2c..8078d307099b09bdf21136f5bc37da2c079ecc97 100644 (file)
--- 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 71793401e3c70b6a50ed8627d5315b1b7b0c0bfe..dc13d436659fb601291799119ce93e7aa8a4d962 100644 (file)
--- 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)