Add hacking check for explicit import of _

Unit tests are not guaranteed to discover whether
the _() function has been explicitly imported to a file.

To ensure that people do not add messages with _() translations
without importing the function, I am adding this hacking check
to enforce the import of _ when it is needed.

The commit includes fixes for offenders found by the new
check.

Change-Id: I616dce6a9da461b3ad2cfaf5e114a2aaa8ced9fa
Closes-Bug: #1348129

diff --git a/cinder/api/middleware/auth.py b/cinder/api/middleware/auth.py
index 2c213b655d5e923faf0d9a385b6ff1c758b7063b..2522dbd4139a1a76a75560b71b520af0f5fd86db 100644 (file)
--- a/cinder/api/middleware/auth.py
+++ b/cinder/api/middleware/auth.py
@@ -26,6 +26,7 @@ import webob.exc
 
 from cinder.api.openstack import wsgi
 from cinder import context
+from cinder.openstack.common.gettextutils import _
 from cinder.openstack.common import jsonutils
 from cinder.openstack.common import log as logging
 from cinder.openstack.common.middleware import request_id

diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py
index f02f2fa7f1979c585b0872ab9018c8212c7dd6ba..fc213bc9f24fbcbc02446b9b7fa240085e548cab 100644 (file)
--- a/cinder/hacking/checks.py
+++ b/cinder/hacking/checks.py
@@ -29,6 +29,12 @@ Guidelines for writing new hacking checks
 
 """
 
+UNDERSCORE_IMPORT_FILES = []
+
+log_translation = re.compile(
+    r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)_\(\s*('|\")")
+string_translation = re.compile(r"(.)*_\(\s*('|\")")
+
 
 def no_translate_debug_logs(logical_line, filename):
     """Check for 'LOG.debug(_('
@@ -53,6 +59,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 logical_line.endswith("import _"):
+        UNDERSCORE_IMPORT_FILES.append(filename)
+    elif(log_translation.match(logical_line) or
+         string_translation.match(logical_line)):
+        yield(0, "N323: Found use of _() without explicit import of _ !")
+
+
 def factory(register):
     register(no_translate_debug_logs)
     register(no_mutable_default_args)
+    register(check_explicit_underscore_import)

diff --git a/cinder/tests/test_backup_swift.py b/cinder/tests/test_backup_swift.py
index 91428d76a3522524ceba7d408c169d6079e6267b..dfc21b32ae066952b73af6880b45eb0886ccc331 100644 (file)
--- a/cinder/tests/test_backup_swift.py
+++ b/cinder/tests/test_backup_swift.py
@@ -29,6 +29,7 @@ from cinder.backup.drivers.swift import SwiftBackupDriver
 from cinder import context
 from cinder import db
 from cinder import exception
+from cinder.openstack.common.gettextutils import _
 from cinder.openstack.common import log as logging
 from cinder import test
 from cinder.tests.backup.fake_swift_client import FakeSwiftClient

diff --git a/cinder/tests/test_hacking.py b/cinder/tests/test_hacking.py
index e7656b79f424555ac8df1560658a01181635fd49..a58db7182a62325e9bb93cd205bd9e2c402d56e0 100644 (file)
--- a/cinder/tests/test_hacking.py
+++ b/cinder/tests/test_hacking.py
@@ -58,3 +58,20 @@ class HackingTestCase(test.TestCase):
 
         self.assertEqual(len(list(checks.no_translate_debug_logs(
             "LOG.info(_('foo'))", "cinder/scheduler/foo.py"))), 0)
+
+    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)

diff --git a/cinder/volume/drivers/nimble.py b/cinder/volume/drivers/nimble.py
index 396f909125d0ae5170a26199a2d297cf16a0d495..68405423d0afb36de4ea898e5161cfabc83bbc10 100644 (file)
--- a/cinder/volume/drivers/nimble.py
+++ b/cinder/volume/drivers/nimble.py
@@ -28,6 +28,7 @@ from oslo.config import cfg
 from suds import client
 
 from cinder import exception
+from cinder.openstack.common.gettextutils import _
 from cinder.openstack.common import log as logging
 from cinder.openstack.common import units
 from cinder.volume.drivers.san.san import SanISCSIDriver

diff --git a/cinder/zonemanager/utils.py b/cinder/zonemanager/utils.py
index fd976f7cd553fb675a1c65e4e80aa5851c15c23f..8b19467cfdcb22e54db4283fa447b41d80d21430 100644 (file)
--- a/cinder/zonemanager/utils.py
+++ b/cinder/zonemanager/utils.py
@@ -19,6 +19,7 @@ Utility functions related to the Zone Manager.
 """
 import logging
 
+from cinder.openstack.common.gettextutils import _
 from cinder.openstack.common import log
 from cinder.volume.configuration import Configuration
 from cinder.volume import manager