Merge "Fix H904 hacking_delayed_string_interpolation"

hacking/checks/other.py

@@ -15,9 +15,8 @@ import re
 from hacking import core
 
 
-log_string_interpolation = re.compile(r".*LOG\.(?:error|warn|warning|info"
-                                      r"|critical|exception|debug)"
-                                      r"\([^,]*%[^,]*[,)]")
+log_string = re.compile(r".*LOG\.(?:error|warn|warning|info"
+                        r"|critical|exception|debug)")
 
 
 @core.flake8ext
@@ -50,5 +49,9 @@ def hacking_delayed_string_interpolation(logical_line, noqa):
     if noqa:
         return
 
-    if log_string_interpolation.match(logical_line):
-        yield 0, msg
+    if log_string.match(logical_line):
+        # Line is a log statement, strip out strings and see if % is used,
+        # just to make sure we don't match on a format specifier in a string.
+        line = re.sub(r"[\"'].+?[\"']", '', logical_line)
+        if '%' in line:
+            yield 0, msg

hacking/tests/checks/test_other.py

@@ -0,0 +1,39 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import ddt
+
+from hacking.checks import other
+from hacking import tests
+
+
+@ddt.ddt
+class OthersTestCase(tests.TestCase):
+    """This tests hacking checks from the 'other' group."""
+
+    @ddt.unpack
+    @ddt.data(
+        (1, 'LOG.debug("Test %s" % foo)', None),
+        (0, 'LOG.info("Test %s", foo)', None),
+        (0, 'LOG.error("Test %s" % foo)', '# noqa'),
+        (1, 'LOG.debug("Test %s" % "foo")', None),
+        (0, 'LOG.debug("Test %s", "foo")', None),
+        (0, "LOG.warning('Testing some stuff')", None))
+    def test_H904_hacking_delayed_string_interpolation(
+            self, err_count, line, noqa):
+        if err_count > 0:
+            self.assertCheckFails(other.hacking_delayed_string_interpolation,
+                                  line, noqa)
+        else:
+            self.assertCheckPasses(other.hacking_delayed_string_interpolation,
+                                   line, noqa)

test-requirements.txt

@@ -8,6 +8,7 @@ python-subunit>=1.0.0 # Apache-2.0/BSD
 stestr>=2.0.0 # Apache-2.0
 testscenarios>=0.4 # Apache-2.0/BSD
 testtools>=2.2.0 # MIT
+ddt>=1.2.1 # MIT
 
 # hacking doesn't use this anywhere, but nova imports this in nova/__init__.py
 # since eventlet is such a common universal import, add it to the hacking test