diff --git a/HACKING.rst b/HACKING.rst index 49a077a78..5fc3a9575 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -20,4 +20,5 @@ Neutron Library Specific Commandments - [N532] Validate that LOG.warning is used instead of LOG.warn. The latter is deprecated. - [N533] Validate that debug level logs are not translated - [N534] Exception messages should be translated -- [N535] Usage of Python eventlet module not allowed \ No newline at end of file +- [N535] Usage of Python eventlet module not allowed +- [N536] String interpolation should be delayed at logging calls. diff --git a/neutron_lib/hacking/checks.py b/neutron_lib/hacking/checks.py index f2924935e..0e7f7fa30 100644 --- a/neutron_lib/hacking/checks.py +++ b/neutron_lib/hacking/checks.py @@ -171,3 +171,4 @@ def factory(register): register(translation_checks.no_translate_debug_logs) register(translation_checks.check_log_warn_deprecated) register(translation_checks.check_raised_localized_exceptions) + register(translation_checks.check_delayed_string_interpolation) diff --git a/neutron_lib/hacking/translation_checks.py b/neutron_lib/hacking/translation_checks.py index b48902ef0..3251d0fd0 100644 --- a/neutron_lib/hacking/translation_checks.py +++ b/neutron_lib/hacking/translation_checks.py @@ -41,6 +41,8 @@ def _regex_for_level(level, hint): _log_translation_hint = re.compile( '|'.join('(?:%s)' % _regex_for_level(level, hint) for level, hint in _all_log_levels.items())) +_log_string_interpolation = re.compile( + r".*LOG\.(error|warning|info|critical|exception|debug)\([^,]*%[^,]*[,)]") def _translation_is_not_expected(filename): @@ -93,3 +95,24 @@ def check_raised_localized_exceptions(logical_line, filename): if exception_msg.startswith("\"") or exception_msg.startswith("\'"): msg = "N534: Untranslated exception message." yield (logical_line.index(exception_msg), msg) + + +def check_delayed_string_interpolation(logical_line, filename, noqa): + """N536 String interpolation should be delayed at logging calls. + + N536: LOG.debug('Example: %s' % 'bad') + Okay: LOG.debug('Example: %s', 'good') + """ + msg = ("N536 String interpolation should be delayed to be " + "handled by the logging code, rather than being done " + "at the point of the logging call. " + "Use ',' instead of '%'.") + + if noqa: + return + + if '/tests/' in filename: + return + + if _log_string_interpolation.match(logical_line): + yield(logical_line.index('%'), msg) diff --git a/neutron_lib/tests/unit/hacking/test_checks.py b/neutron_lib/tests/unit/hacking/test_checks.py index 906ae0f3c..51bdbee2a 100644 --- a/neutron_lib/tests/unit/hacking/test_checks.py +++ b/neutron_lib/tests/unit/hacking/test_checks.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import re + import testtools from neutron_lib.hacking import checks @@ -179,3 +181,35 @@ class HackingTestCase(base.BaseTestCase): f = tc.check_raised_localized_exceptions self.assertLinePasses(f, "raise KeyError('Error text')", 'neutron_lib/tests/unit/mytest.py') + + def test_check_delayed_string_interpolation(self): + dummy_noqa = re.search('a', 'a') + f = tc.check_delayed_string_interpolation + + # In 'logical_line', Contents of strings replaced with + # "xxx" of same length. + self.assertLineFails(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)', + 'neutron_lib/db/utils.py', None) + self.assertLineFails(f, "LOG.warning(msg % 'xxxxx')", + 'neutron_lib/db/utils.py', None) + self.assertLineFails(f, "LOG.info(_LI('xxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxx') % {'xx': v1, 'xx': v2})", + 'neutron_lib/db/utils.py', None) + + self.assertLinePasses( + f, 'LOG.error(_LE("xxxxxxxxxxxxxxxxxx"), value)', + 'neutron_lib/db/utils.py', None) + self.assertLinePasses(f, "LOG.warning(msg, 'xxxxx')", + 'neutron_lib/db/utils.py', None) + self.assertLinePasses(f, "LOG.info(_LI('xxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxx'), {'xx': v1, 'xx': v2})", + 'neutron_lib/db/utils.py', None) + + # check a file in neutron_lib/tests + self.assertLinePasses(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)', + 'neutron_lib/tests/unit/test_neutron_lib.py', + None) + # check code including 'noqa' + self.assertLinePasses(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)', + 'neutron_lib/db/utils.py', + dummy_noqa)