Browse Source

Add a hacking rule for string interpolation at logging

String interpolation should be delayed to be handled
by the logging code, rather than being done at the point of
the logging call. So add the following hacking rule for it.

- [N536] String interpolation should be delayed at logging calls.

We need this to ensure that all projects using the neutron-lib
hacking rules still have enforcement of the log policies
of the project.

See the oslo i18n guideline.

* http://docs.openstack.org/developer/oslo.i18n/guidelines.html

Change-Id: I901dcbfbd53d5d19db651473d2891bc8e8a59710
Related-Bug: #1596829
tags/0.4.0
Takashi NATSUME 3 years ago
parent
commit
eb32fd4fbd

+ 2
- 1
HACKING.rst View File

@@ -20,4 +20,5 @@ Neutron Library Specific Commandments
20 20
 - [N532] Validate that LOG.warning is used instead of LOG.warn. The latter is deprecated.
21 21
 - [N533] Validate that debug level logs are not translated
22 22
 - [N534] Exception messages should be translated
23
-- [N535] Usage of Python eventlet module not allowed
23
+- [N535] Usage of Python eventlet module not allowed
24
+- [N536] String interpolation should be delayed at logging calls.

+ 1
- 0
neutron_lib/hacking/checks.py View File

@@ -171,3 +171,4 @@ def factory(register):
171 171
     register(translation_checks.no_translate_debug_logs)
172 172
     register(translation_checks.check_log_warn_deprecated)
173 173
     register(translation_checks.check_raised_localized_exceptions)
174
+    register(translation_checks.check_delayed_string_interpolation)

+ 23
- 0
neutron_lib/hacking/translation_checks.py View File

@@ -41,6 +41,8 @@ def _regex_for_level(level, hint):
41 41
 _log_translation_hint = re.compile(
42 42
     '|'.join('(?:%s)' % _regex_for_level(level, hint)
43 43
              for level, hint in _all_log_levels.items()))
44
+_log_string_interpolation = re.compile(
45
+    r".*LOG\.(error|warning|info|critical|exception|debug)\([^,]*%[^,]*[,)]")
44 46
 
45 47
 
46 48
 def _translation_is_not_expected(filename):
@@ -93,3 +95,24 @@ def check_raised_localized_exceptions(logical_line, filename):
93 95
         if exception_msg.startswith("\"") or exception_msg.startswith("\'"):
94 96
             msg = "N534: Untranslated exception message."
95 97
             yield (logical_line.index(exception_msg), msg)
98
+
99
+
100
+def check_delayed_string_interpolation(logical_line, filename, noqa):
101
+    """N536 String interpolation should be delayed at logging calls.
102
+
103
+    N536: LOG.debug('Example: %s' % 'bad')
104
+    Okay: LOG.debug('Example: %s', 'good')
105
+    """
106
+    msg = ("N536 String interpolation should be delayed to be "
107
+           "handled by the logging code, rather than being done "
108
+           "at the point of the logging call. "
109
+           "Use ',' instead of '%'.")
110
+
111
+    if noqa:
112
+        return
113
+
114
+    if '/tests/' in filename:
115
+        return
116
+
117
+    if _log_string_interpolation.match(logical_line):
118
+        yield(logical_line.index('%'), msg)

+ 34
- 0
neutron_lib/tests/unit/hacking/test_checks.py View File

@@ -10,6 +10,8 @@
10 10
 #    License for the specific language governing permissions and limitations
11 11
 #    under the License.
12 12
 
13
+import re
14
+
13 15
 import testtools
14 16
 
15 17
 from neutron_lib.hacking import checks
@@ -179,3 +181,35 @@ class HackingTestCase(base.BaseTestCase):
179 181
         f = tc.check_raised_localized_exceptions
180 182
         self.assertLinePasses(f, "raise KeyError('Error text')",
181 183
                               'neutron_lib/tests/unit/mytest.py')
184
+
185
+    def test_check_delayed_string_interpolation(self):
186
+        dummy_noqa = re.search('a', 'a')
187
+        f = tc.check_delayed_string_interpolation
188
+
189
+        # In 'logical_line', Contents of strings replaced with
190
+        # "xxx" of same length.
191
+        self.assertLineFails(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)',
192
+                             'neutron_lib/db/utils.py', None)
193
+        self.assertLineFails(f, "LOG.warning(msg % 'xxxxx')",
194
+                             'neutron_lib/db/utils.py', None)
195
+        self.assertLineFails(f, "LOG.info(_LI('xxxxxxxxxxxxxxxxxxxxxxxxx"
196
+                                "xxxxxxxxx') % {'xx': v1, 'xx': v2})",
197
+                             'neutron_lib/db/utils.py', None)
198
+
199
+        self.assertLinePasses(
200
+            f, 'LOG.error(_LE("xxxxxxxxxxxxxxxxxx"), value)',
201
+            'neutron_lib/db/utils.py', None)
202
+        self.assertLinePasses(f, "LOG.warning(msg, 'xxxxx')",
203
+                              'neutron_lib/db/utils.py', None)
204
+        self.assertLinePasses(f, "LOG.info(_LI('xxxxxxxxxxxxxxxxxxxxxxxx"
205
+                                 "xxxxxxxxx'), {'xx': v1, 'xx': v2})",
206
+                              'neutron_lib/db/utils.py', None)
207
+
208
+        # check a file in neutron_lib/tests
209
+        self.assertLinePasses(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)',
210
+                              'neutron_lib/tests/unit/test_neutron_lib.py',
211
+                              None)
212
+        # check code including 'noqa'
213
+        self.assertLinePasses(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)',
214
+                              'neutron_lib/db/utils.py',
215
+                              dummy_noqa)

Loading…
Cancel
Save