Fix issue in hacking with underscore imports

Hacking rule for underscore imports _() are not working correctly.
It also matches _.* imports like "import _LE"

See review: https://review.openstack.org/#/c/270754/

Closes-bug: 1541780
Change-Id: Ibdef35e1896882a8dfe3165dba989255639e61ba

diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py
index fe9e11626f1b022a9e29335a298f8f0e56e8c042..52ff7b7bf4506755591bf6b6e1e3f5eb47683fd0 100644 (file)
--- a/cinder/hacking/checks.py
+++ b/cinder/hacking/checks.py
@@ -39,7 +39,8 @@ translated_log = re.compile(
     "\(\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\s+_(.)*")
+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.
 custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*")
 no_audit_log = re.compile(r"(.)*LOG\.audit(.)*")
@@ -165,6 +166,7 @@ def check_explicit_underscore_import(logical_line, filename):
         if file in filename:
             return
     if (underscore_import_check.match(logical_line) or
+            underscore_import_check_multi.match(logical_line) or
             custom_underscore_check.match(logical_line)):
         UNDERSCORE_IMPORT_FILES.append(filename)
     elif(translated_log.match(logical_line) or

diff --git a/cinder/tests/unit/test_hacking.py b/cinder/tests/unit/test_hacking.py
index 3cd8de968d39bdae6a59231b2c29490ef7b07c21..b4393f1c2c73dde18eae5d5dc9a5e49ae6655043 100644 (file)
--- a/cinder/tests/unit/test_hacking.py
+++ b/cinder/tests/unit/test_hacking.py
@@ -105,7 +105,7 @@ class HackingTestCase(test.TestCase):
             "msg = _('My message')",
             "cinder.tests.unit/other_files.py"))))
         self.assertEqual(0, len(list(checks.check_explicit_underscore_import(
-            "from cinder.i18n import _, _LW",
+            "from cinder.i18n import _LE, _, _LW",
             "cinder.tests.unit/other_files2.py"))))
         self.assertEqual(0, len(list(checks.check_explicit_underscore_import(
             "msg = _('My message')",
@@ -120,6 +120,12 @@ class HackingTestCase(test.TestCase):
         self.assertEqual(0, len(list(checks.check_explicit_underscore_import(
             "LOG.info('My info message')",
             "cinder.tests.unit/other_files4.py"))))
+        self.assertEqual(0, len(list(checks.check_explicit_underscore_import(
+            "from cinder.i18n import _LW",
+            "cinder.tests.unit/other_files5.py"))))
+        self.assertEqual(1, len(list(checks.check_explicit_underscore_import(
+            "msg = _('My message')",
+            "cinder.tests.unit/other_files5.py"))))
 
     # We are patching pep8 so that only the check under test is actually
     # installed.