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 a hacking rule for it.

See the oslo i18n guideline.

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

Change-Id: Ib7d97e6edbb8069c12b22505c0d6653b4a17ec78
Closes-Bug: #1596829
tags/2.0.0
Takashi NATSUME 3 years ago
parent
commit
cd3be63409
3 changed files with 78 additions and 0 deletions
  1. 1
    0
      HACKING.rst
  2. 25
    0
      masakari/hacking/checks.py
  3. 52
    0
      masakari/tests/unit/test_hacking.py

+ 1
- 0
HACKING.rst View File

@@ -48,3 +48,4 @@ Masakari Specific Commandments
48 48
 - [M327] Python 3: do not use dict.iterkeys.
49 49
 - [M328] Python 3: do not use dict.itervalues.
50 50
 - [M329] Deprecated library function os.popen()
51
+- [M330] String interpolation should be delayed at logging calls.

+ 25
- 0
masakari/hacking/checks.py View File

@@ -95,6 +95,9 @@ spawn_re = re.compile(
95 95
 contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(")
96 96
 doubled_words_re = re.compile(
97 97
     r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b")
98
+log_string_interpolation = re.compile(r".*LOG\.(error|warning|info"
99
+                                      r"|critical|exception|debug)"
100
+                                      r"\([^,]*%[^,]*[,)]")
98 101
 
99 102
 
100 103
 def no_db_session_in_public_api(logical_line, filename):
@@ -418,6 +421,27 @@ def no_os_popen(logical_line):
418 421
                  'Replace it using subprocess module. ')
419 422
 
420 423
 
424
+def check_delayed_string_interpolation(logical_line, filename, noqa):
425
+    """M330 String interpolation should be delayed at logging calls.
426
+
427
+    M330: LOG.debug('Example: %s' % 'bad')
428
+    Okay: LOG.debug('Example: %s', 'good')
429
+    """
430
+    msg = ("M330 String interpolation should be delayed to be "
431
+           "handled by the logging code, rather than being done "
432
+           "at the point of the logging call. "
433
+           "Use ',' instead of '%'.")
434
+
435
+    if noqa:
436
+        return
437
+
438
+    if '/tests/' in filename:
439
+        return
440
+
441
+    if log_string_interpolation.match(logical_line):
442
+        yield(logical_line.index('%'), msg)
443
+
444
+
421 445
 def factory(register):
422 446
     register(no_db_session_in_public_api)
423 447
     register(use_timeutils_utcnow)
@@ -445,3 +469,4 @@ def factory(register):
445 469
     register(check_python3_no_iterkeys)
446 470
     register(check_python3_no_itervalues)
447 471
     register(no_os_popen)
472
+    register(check_delayed_string_interpolation)

+ 52
- 0
masakari/tests/unit/test_hacking.py View File

@@ -481,3 +481,55 @@ class HackingTestCase(test.NoDBTestCase):
481 481
         errors = [(4, 0, 'M329'), (8, 8, 'M329')]
482 482
         self._assert_has_errors(code, checks.no_os_popen,
483 483
                                 expected_errors=errors)
484
+
485
+    def test_check_delayed_string_interpolation(self):
486
+        checker = checks.check_delayed_string_interpolation
487
+        code = """
488
+               msg_w = _LW('Test string (%s)')
489
+               msg_i = _LI('Test string (%s)')
490
+               value = 'test'
491
+
492
+               LOG.error(_LE("Test string (%s)") % value)
493
+               LOG.warning(msg_w % 'test%string')
494
+               LOG.info(msg_i %
495
+                        "test%string%info")
496
+               LOG.critical(
497
+                   _LC('Test string (%s)') % value,
498
+                   instance=instance)
499
+               LOG.exception(_LE(" 'Test quotation %s' \"Test\"") % 'test')
500
+               LOG.debug(' "Test quotation %s" \'Test\'' % "test")
501
+               LOG.debug('Tesing %(test)s' %
502
+                         {'test': ','.join(
503
+                             ['%s=%s' % (name, value)
504
+                              for name, value in test.items()])})
505
+               """
506
+
507
+        expected_errors = [(5, 34, 'M330'), (6, 18, 'M330'), (7, 15, 'M330'),
508
+                           (10, 28, 'M330'), (12, 49, 'M330'),
509
+                           (13, 40, 'M330'), (14, 28, 'M330')]
510
+        self._assert_has_errors(code, checker, expected_errors=expected_errors)
511
+        self._assert_has_no_errors(
512
+            code, checker, filename='masakari/tests/unit/test_hacking.py')
513
+
514
+        code = """
515
+               msg_w = _LW('Test string (%s)')
516
+               msg_i = _LI('Test string (%s)')
517
+               value = 'test'
518
+
519
+               LOG.error(_LE("Test string (%s)"), value)
520
+               LOG.error(_LE("Test string (%s)") % value) # noqa
521
+               LOG.warn(_LW('Test string (%s)'),
522
+                        value)
523
+               LOG.info(msg_i,
524
+                        "test%string%info")
525
+               LOG.critical(
526
+                   _LC('Test string (%s)'), value,
527
+                   instance=instance)
528
+               LOG.exception(_LE(" 'Test quotation %s' \"Test\""), 'test')
529
+               LOG.debug(' "Test quotation %s" \'Test\'', "test")
530
+               LOG.debug('Tesing %(test)s',
531
+                         {'test': ','.join(
532
+                             ['%s=%s' % (name, value)
533
+                              for name, value in test.items()])})
534
+               """
535
+        self._assert_has_no_errors(code, checker)

Loading…
Cancel
Save