diff --git a/HACKING.rst b/HACKING.rst index e1bec9e51..49ee011d6 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -14,4 +14,3 @@ Mistral Specific Commandments - [M328] Python 3: do not use dict.iteritems. - [M329] Python 3: do not use dict.iterkeys. - [M330] Python 3: do not use dict.itervalues. -- [M331] String interpolation should be delayed at logging calls. diff --git a/mistral/hacking/checks.py b/mistral/hacking/checks.py index 0e0cbfc0c..78dd691ea 100644 --- a/mistral/hacking/checks.py +++ b/mistral/hacking/checks.py @@ -30,9 +30,6 @@ import six oslo_namespace_imports_dot = re.compile(r"import[\s]+oslo[.][^\s]+") oslo_namespace_imports_from_dot = re.compile(r"from[\s]+oslo[.]") oslo_namespace_imports_from_root = re.compile(r"from[\s]+oslo[\s]+import[\s]+") -log_string_interpolation = re.compile(r".*LOG\.(?:error|warn|warning|info" - r"|critical|exception|debug)" - r"\([^,]*%[^,]*[,)]") def no_assert_equal_true_false(logical_line): @@ -108,27 +105,6 @@ def check_python3_no_itervalues(logical_line): yield(0, msg) -def check_delayed_string_interpolation(logical_line, filename, noqa): - """M331: String interpolation should be delayed at logging calls. - - M331: LOG.debug('Example: %s' % 'bad') - Okay: LOG.debug('Example: %s', 'good') - """ - msg = ("M331 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 'mistral/tests/' in filename: - return - - if log_string_interpolation.match(logical_line): - yield(logical_line.index('%'), msg) - - class BaseASTChecker(ast.NodeVisitor): """Provides a simple framework for writing AST-based checks. @@ -297,4 +273,3 @@ def factory(register): register(check_python3_no_iterkeys) register(check_python3_no_itervalues) register(check_python3_xrange) - register(check_delayed_string_interpolation) diff --git a/mistral/tests/unit/base.py b/mistral/tests/unit/base.py index e90f67455..cbfdbe38f 100644 --- a/mistral/tests/unit/base.py +++ b/mistral/tests/unit/base.py @@ -142,7 +142,7 @@ class BaseTest(base.BaseTestCase): found = len(filtered_items) if found != count: - LOG.info("[failed test ctx] items=%s, expected_props=%s" % (str( + LOG.info("[failed test ctx] items=%s, expected_props=%s", (str( items), props)) self.fail("Wrong number of items found [props=%s, " "expected=%s, found=%s]" % (props, count, found)) diff --git a/mistral/tests/unit/hacking/test_checks.py b/mistral/tests/unit/hacking/test_checks.py index dbf15ea44..76f2efed8 100644 --- a/mistral/tests/unit/hacking/test_checks.py +++ b/mistral/tests/unit/hacking/test_checks.py @@ -121,59 +121,6 @@ class BaseLoggingCheckTest(base.BaseTest): self.assertEqual(0, len(list(checks.check_python3_no_itervalues( "six.itervalues(ob))")))) - def test_check_delayed_string_interpolation(self): - checker = checks.check_delayed_string_interpolation - code = """ - msg_w = _LW('Test string (%s)') - msg_i = _LI('Test string (%s)') - value = 'test' - - LOG.error(_LE("Test string (%s)") % value) - LOG.warning(msg_w % 'test%string') - LOG.info(msg_i % - "test%string%info") - LOG.critical( - _LC('Test string (%s)') % value, - instance=instance) - LOG.exception(_LE(" 'Test quotation %s' \"Test\"") % 'test') - LOG.debug(' "Test quotation %s" \'Test\'' % "test") - LOG.debug('Tesing %(test)s' % - {'test': ','.join( - ['%s=%s' % (name, value) - for name, value in test.items()])}) - """ - - expected_errors = [(5, 34, 'M331'), (6, 18, 'M331'), (7, 15, 'M331'), - (10, 28, 'M331'), (12, 49, 'M331'), - (13, 40, 'M331'), (14, 28, 'M331')] - self._assert_has_errors(code, checker, expected_errors=expected_errors) - self._assert_has_no_errors( - code, checker, filename='mistral/tests/unit/hacking/test_checks.py' - ) - - code = """ - msg_w = _LW('Test string (%s)') - msg_i = _LI('Test string (%s)') - value = 'test' - - LOG.error(_LE("Test string (%s)"), value) - LOG.error(_LE("Test string (%s)") % value) # noqa - LOG.warn(_LW('Test string (%s)'), - value) - LOG.info(msg_i, - "test%string%info") - LOG.critical( - _LC('Test string (%s)'), value, - instance=instance) - LOG.exception(_LE(" 'Test quotation %s' \"Test\""), 'test') - LOG.debug(' "Test quotation %s" \'Test\'', "test") - LOG.debug('Tesing %(test)s', - {'test': ','.join( - ['%s=%s' % (name, value) - for name, value in test.items()])}) - """ - self._assert_has_no_errors(code, checker) - class TestLoggingWithWarn(BaseLoggingCheckTest): diff --git a/tox.ini b/tox.ini index 37a6673ee..532a3b009 100644 --- a/tox.ini +++ b/tox.ini @@ -79,7 +79,8 @@ show-source = true builtins = _ # [H106] Don't put vim configuration in source files. # [H203] Use assertIs(Not)None to check for None. -enable-extensions = H106,H203 +# [H904] Delay string interpolations at logging calls. +enable-extensions = H106,H203,H904 exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,tools,scripts [doc8]