From eb32fd4fbd0c16b51efb91f2385ab3f2243fcd48 Mon Sep 17 00:00:00 2001 From: Takashi NATSUME Date: Tue, 26 Jul 2016 14:56:46 +0900 Subject: [PATCH] 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 --- HACKING.rst | 3 +- neutron_lib/hacking/checks.py | 1 + neutron_lib/hacking/translation_checks.py | 23 +++++++++++++ neutron_lib/tests/unit/hacking/test_checks.py | 34 +++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/HACKING.rst b/HACKING.rst index 49a077a..5fc3a95 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 f292493..0e7f7fa 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 b48902e..3251d0f 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 906ae0f..51bdbee 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)