Add hacking check for print() statements

We are frequently having to -1 patches because people
forget print() statements that were used for debug in
their development.  Can save everyone time and trouble by
adding this simple hacking check.

The check excluded the cinder/cmd directory as the files in there
legitimately need to use the print() command.  Also wsgi.py and
the test_hds_nas_backend.py files make use of print, so I have
excluded those from checking as well.

Change-Id: I2066eeb2bdc6c9af294d0b9befb7e0b32abd1378

diff --git a/HACKING.rst b/HACKING.rst
index 507aea50b216d95a608e9b92cef2fa8483c6861e..066b4eb187b86fdd178079501dd626b8ebc213c3 100644 (file)
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -20,6 +20,7 @@ Cinder Specific Commandments
 - [N339] Prevent use of deprecated contextlib.nested.
 - [C301] timeutils.utcnow() from oslo_utils should be used instead of datetime.now().
 - [C302] six.text_type should be used instead of unicode
+- [C303] Ensure that there are no 'print()' statements in code that is being committed.
 
 
 General

diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py
index 0676d447b08bb50ffe560de86723b3060997c422..e33ebf97085bc786a464ba5b7939faea517e5e93 100644 (file)
--- a/cinder/hacking/checks.py
+++ b/cinder/hacking/checks.py
@@ -41,6 +41,7 @@ underscore_import_check = re.compile(r"(.)*i18n\s+import\s+_(.)*")
 # We need this for cases where they have created their own _ function.
 custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*")
 no_audit_log = re.compile(r"(.)*LOG\.audit(.)*")
+no_print_statements = re.compile(r"\s*print\s*\(.+\).*")
 
 # NOTE(jsbryant): When other oslo libraries switch over non-namespaced
 # imports, we will need to add them to the regex below.
@@ -207,6 +208,18 @@ def check_unicode_usage(logical_line, noqa):
         yield(0, msg)
 
 
+def check_no_print_statements(logical_line, filename, noqa):
+    # The files in cinder/cmd do need to use 'print()' so
+    # we don't need to check those files.  Other exemptions
+    # should use '# noqa' to avoid failing here.
+    if "cinder/cmd" not in filename and not noqa:
+        if re.match(no_print_statements, logical_line):
+            msg = ("C303: print() should not be used. "
+                   "Please use LOG.[info|error|warning|exception|debug]. "
+                   "If print() must be used, use '# noqa' to skip this check.")
+            yield(0, msg)
+
+
 def factory(register):
     register(no_vi_headers)
     register(no_translate_debug_logs)
@@ -219,3 +232,4 @@ def factory(register):
     register(check_datetime_now)
     register(validate_log_translations)
     register(check_unicode_usage)
+    register(check_no_print_statements)

diff --git a/cinder/tests/test_hacking.py b/cinder/tests/test_hacking.py
index e2a673b0c29544a6e7d6da31527052809a9926b6..c41455a230846c2324545ea757f3f3d83066657a 100644 (file)
--- a/cinder/tests/test_hacking.py
+++ b/cinder/tests/test_hacking.py
@@ -209,3 +209,20 @@ class HackingTestCase(test.TestCase):
             "unicode(msg)", False))))
         self.assertEqual(0, len(list(checks.check_unicode_usage(
             "unicode(msg)  # noqa", True))))
+
+    def test_no_print_statements(self):
+        self.assertEqual(0, len(list(checks.check_no_print_statements(
+            "a line with no print statement",
+            "cinder/file.py", False))))
+        self.assertEqual(1, len(list(checks.check_no_print_statements(
+            "print('My print statement')",
+            "cinder/file.py", False))))
+        self.assertEqual(0, len(list(checks.check_no_print_statements(
+            "print('My print statement in cinder/cmd, which is ok.')",
+            "cinder/cmd/file.py", False))))
+        self.assertEqual(0, len(list(checks.check_no_print_statements(
+            "print('My print statement that I just must have.')",
+            "cinder/tests/file.py", True))))
+        self.assertEqual(1, len(list(checks.check_no_print_statements(
+            "print ('My print with space')",
+            "cinder/volume/anotherFile.py", False))))

diff --git a/cinder/tests/test_hds_hnas_backend.py b/cinder/tests/test_hds_hnas_backend.py
index 9146b056a4c86e3adc52c0f53fe92e301267d921..8f2070a7564ccd5e40faf61ff90d934d0affe452 100644 (file)
--- a/cinder/tests/test_hds_hnas_backend.py
+++ b/cinder/tests/test_hds_hnas_backend.py
@@ -263,8 +263,8 @@ UTILS_EXEC_OUT = ["output: test_cmd", ""]
 
 
 def m_run_cmd(*args, **kargs):
-    print(args)
-    print(HNAS_CMDS.get(args))
+    print(args)  # noqa
+    print(HNAS_CMDS.get(args))  # noqa
     return HNAS_CMDS.get(args)
 
 

diff --git a/cinder/wsgi.py b/cinder/wsgi.py
index d26bca53eb009873b89083c0880bbcb4fcd42ec0..1d1c4d0e39201657c104318b600a1ff37715f16e 100644 (file)
--- a/cinder/wsgi.py
+++ b/cinder/wsgi.py
@@ -438,16 +438,16 @@ class Debug(Middleware):
 
     @webob.dec.wsgify(RequestClass=Request)
     def __call__(self, req):
-        print(('*' * 40) + ' REQUEST ENVIRON')
+        print(('*' * 40) + ' REQUEST ENVIRON')  # noqa
         for key, value in req.environ.items():
-            print(key, '=', value)
-        print()
+            print(key, '=', value)  # noqa
+        print()  # noqa
         resp = req.get_response(self.application)
 
-        print(('*' * 40) + ' RESPONSE HEADERS')
+        print(('*' * 40) + ' RESPONSE HEADERS')  # noqa
         for (key, value) in resp.headers.iteritems():
-            print(key, '=', value)
-        print()
+            print(key, '=', value)  # noqa
+        print()  # noqa
 
         resp.app_iter = self.print_generator(resp.app_iter)
 
@@ -456,12 +456,12 @@ class Debug(Middleware):
     @staticmethod
     def print_generator(app_iter):
         """Iterator that prints the contents of a wrapper string."""
-        print(('*' * 40) + ' BODY')
+        print(('*' * 40) + ' BODY')  # noqa
         for part in app_iter:
             sys.stdout.write(part)
             sys.stdout.flush()
             yield part
-        print()
+        print()  # noqa
 
 
 class Router(object):