From c2658d9c09d9960d6028e9685fe0baa6c50d0f54 Mon Sep 17 00:00:00 2001 From: "Jay S. Bryant" Date: Mon, 6 Apr 2015 11:22:09 -0500 Subject: [PATCH] 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 --- HACKING.rst | 1 + cinder/hacking/checks.py | 14 ++++++++++++++ cinder/tests/test_hacking.py | 17 +++++++++++++++++ cinder/tests/test_hds_hnas_backend.py | 4 ++-- cinder/wsgi.py | 16 ++++++++-------- 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): -- 2.45.2