Add hacking check for explicit import of _()

To ensure the right message catalog is used when translating
messages we need to make sure to explicitly import '_' in
any files that use that function.  We cannot count on
unit test to catch cases where the user has forgotten to
import the _() function.

This hacking check ensures that the function has been imported
anywhere that it is used.  Unit tests for the hacking check are
included.

Change-Id: I9d8101916bcb449345d3123617c2ac75776d053e

HACKING.rst

@@ -35,6 +35,7 @@ Nova Specific Commandments
   self.flags(option=value) instead.
 - [N321] Validate that LOG messages, except debug ones, have translations
 - [N322] Method's default argument shouldn't be mutable
+- [N323] Ensure that the _() function is explicitly imported to ensure proper translations.
 
 Creating Unit Tests
 -------------------

nova/hacking/checks.py

@@ -31,6 +31,8 @@ Guidelines for writing new hacking checks
 
 """
 
+UNDERSCORE_IMPORT_FILES = []
+
 session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]")
 cfg_re = re.compile(r".*\scfg\.")
 vi_header_re = re.compile(r"^#\s+vim?:.+")
@@ -54,7 +56,14 @@ asse_equal_start_with_none_re = re.compile(
 conf_attribute_set_re = re.compile(r"CONF\.[a-z0-9_.]+\s*=\s*\w")
 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*('|\")")
 mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
+string_translation = re.compile(r"[^_]*_\(\s*('|\")")
+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 import_no_db_in_virt(logical_line, filename):
@@ -256,6 +265,27 @@ def no_mutable_default_args(logical_line):
         yield (0, msg)
 
 
+def check_explicit_underscore_import(logical_line, filename):
+    """Check for explicit import of the _ function
+
+    We need to ensure that any files that are using the _() function
+    to translate logs are explicitly importing the _ function.  We
+    can't trust unit test to catch whether the import has been
+    added so we need to check for it here.
+    """
+
+    # Build a list of the files that have _ imported.  No further
+    # checking needed once it is found.
+    if filename in UNDERSCORE_IMPORT_FILES:
+        pass
+    elif (underscore_import_check.match(logical_line) or
+          custom_underscore_check.match(logical_line)):
+        UNDERSCORE_IMPORT_FILES.append(filename)
+    elif (translated_log.match(logical_line) or
+         string_translation.match(logical_line)):
+        yield(0, "N323: Found use of _() without explicit import of _ !")
+
+
 def factory(register):
     register(import_no_db_in_virt)
     register(no_db_session_in_public_api)
@@ -272,3 +302,4 @@ def factory(register):
     register(no_setting_conf_directly_in_tests)
     register(validate_log_translations)
     register(no_mutable_default_args)
+    register(check_explicit_underscore_import)

nova/tests/api/openstack/test_faults.py

@@ -26,6 +26,7 @@ from nova.api.openstack import common
 from nova.api.openstack import wsgi
 from nova import exception
 from nova import i18n
+from nova.i18n import _
 from nova.openstack.common import jsonutils
 from nova import test
 
@@ -43,7 +44,7 @@ class TestFaultWrapper(test.NoDBTestCase):
 
         # Create an exception, passing a translatable message with a
         # known value we can test for later.
-        safe_exception = exception.NotFound(i18n._('Should be translated.'))
+        safe_exception = exception.NotFound(_('Should be translated.'))
         safe_exception.safe = True
         safe_exception.code = 404
 

nova/tests/test_hacking.py

@@ -188,3 +188,32 @@ class HackingTestCase(test.NoDBTestCase):
 
         self.assertEqual(0, len(list(checks.no_mutable_default_args(
             "defined, undefined = [], {}"))))
+
+    def test_check_explicit_underscore_import(self):
+        self.assertEqual(len(list(checks.check_explicit_underscore_import(
+            "LOG.info(_('My info message'))",
+            "cinder/tests/other_files.py"))), 1)
+        self.assertEqual(len(list(checks.check_explicit_underscore_import(
+            "msg = _('My message')",
+            "cinder/tests/other_files.py"))), 1)
+        self.assertEqual(len(list(checks.check_explicit_underscore_import(
+            "from cinder.i18n import _",
+            "cinder/tests/other_files.py"))), 0)
+        self.assertEqual(len(list(checks.check_explicit_underscore_import(
+            "LOG.info(_('My info message'))",
+            "cinder/tests/other_files.py"))), 0)
+        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)