Merge "Update log translation hacking rule"

HACKING.rst

@@ -1,5 +1,5 @@
 Cloudkitty Style Commandments
-============================
+=============================
 
 - Step 1: Read the OpenStack Style Commandments
   https://docs.openstack.org/hacking/latest
@@ -7,12 +7,12 @@ Cloudkitty Style Commandments
 
 
 Cloudkitty Specific Commandments
--------------------------------
+--------------------------------
 
 - [C310] Check for improper use of logging format arguments.
 - [C311] Use assertIsNone(...) instead of assertEqual(None, ...).
 - [C312] Use assertTrue(...) rather than assertEqual(True, ...).
-- [C313] Validate that debug level logs are not translated.
+- [C313] Validate that logs are not translated.
 - [C314] str() and unicode() cannot be used on an exception. Remove or use six.text_type().
 - [C315] Translated messages cannot be concatenated.  String should be
   included in translated message.

cloudkitty/hacking/checks.py

@@ -36,11 +36,12 @@ Guidelines for writing new hacking checks
 
 UNDERSCORE_IMPORT_FILES = []
 
-log_translation = re.compile(
-    r"(.)*LOG\.(audit|error|info|critical|exception)\(\s*('|\")")
-translated_log = re.compile(
-    r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)"
-    "\(\s*_\(\s*('|\")")
+_all_log_levels = {'debug', 'error', 'info', 'warning',
+                   'critical', 'exception'}
+# Since _Lx have been removed, we just need to check _()
+translated_logs = re.compile(
+    r"(.)*LOG\.(%(level)s)\(\s*_\(" % {'level': '|'.join(_all_log_levels)})
+
 string_translation = re.compile(r"[^_]*_\(\s*('|\")")
 underscore_import_check = re.compile(r"(.)*import _$")
 underscore_import_check_multi = re.compile(r"(.)*import (.)*_, (.)*")
@@ -99,12 +100,11 @@ class BaseASTChecker(ast.NodeVisitor):
         return False
 
 
-def no_translate_debug_logs(logical_line, filename):
-    """Check for 'LOG.debug(_('
+def no_translate_logs(logical_line, filename):
+    """Check for 'LOG.*(_('
 
-    As per our translation policy,
-    https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
-    we shouldn't translate debug level logs.
+    Starting with the Pike series, OpenStack no longer supports log
+    translation.
 
     * This check assumes that 'LOG' is a logger.
     * Use filename so we can start enforcing this in specific folders instead
@@ -112,8 +112,8 @@ def no_translate_debug_logs(logical_line, filename):
 
     C313
     """
-    if logical_line.startswith("LOG.debug(_("):
-        yield(0, "C313 Don't translate debug level logs")
+    if translated_logs.match(logical_line):
+        yield(0, "C313 Don't translate logs")
 
 
 class CheckLoggingFormatArgs(BaseASTChecker):
@@ -198,8 +198,7 @@ def check_explicit_underscore_import(logical_line, filename):
           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
-          string_translation.match(logical_line)):
+    elif string_translation.match(logical_line):
         yield(0, "C321: Found use of _() without explicit import of _ !")
 
 
@@ -341,7 +340,7 @@ def no_log_warn_check(logical_line):
 
 def factory(register):
     register(check_explicit_underscore_import)
-    register(no_translate_debug_logs)
+    register(no_translate_logs)
     register(CheckForStrUnicodeExc)
     register(CheckLoggingFormatArgs)
     register(CheckForTransAdd)

cloudkitty/tests/test_hacking.py

@@ -57,15 +57,13 @@ class HackingTestCase(tests.TestCase):
     should pass.
     """
 
-    def test_no_translate_debug_logs(self):
-        self.assertEqual(1, len(list(checks.no_translate_debug_logs(
-            "LOG.debug(_('foo'))", "cloudkitty/scheduler/foo.py"))))
-
-        self.assertEqual(0, len(list(checks.no_translate_debug_logs(
-            "LOG.debug('foo')", "cloudkitty/scheduler/foo.py"))))
-
-        self.assertEqual(0, len(list(checks.no_translate_debug_logs(
-            "LOG.info(_('foo'))", "cloudkitty/scheduler/foo.py"))))
+    def test_no_log_translations(self):
+        for log in checks._all_log_levels:
+            bad = 'LOG.%s(_("Bad"))' % log
+            self.assertEqual(1, len(list(checks.no_translate_logs(bad, 'f'))))
+            # Catch abuses when used with a variable and not a literal
+            bad = 'LOG.%s(_(msg))' % log
+            self.assertEqual(1, len(list(checks.no_translate_logs(bad, 'f'))))
 
     def test_check_explicit_underscore_import(self):
         self.assertEqual(1, len(list(checks.check_explicit_underscore_import(