Merge "Add hacking check for print() statements"

5 files changed, 42 insertions, 10 deletions

diff --git a/HACKING.rst b/HACKING.rst
index 507aea50b..066b4eb18 100644
--- 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 0676d447b..e33ebf970 100644
--- 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 e2a673b0c..c41455a23 100644
--- 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 9146b056a..8f2070a75 100644
--- 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 d26bca53e..1d1c4d0e3 100644
--- 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):