From 528ee090bf5749fb0b5674d8f8bd3cff964c8cfd Mon Sep 17 00:00:00 2001 From: Tom Barron Date: Mon, 23 May 2016 17:34:49 -0400 Subject: [PATCH] Hacking check for str in exception breaks in py34 So change [1] skips this check in python34 tests. Replace CheckForStrExc with CheckForStrUnicodeEx and quit skipping unit test for this hacking check. Partially-Implements: bp py3-compatibility [1] Id3d196d6006e7b2e5e56343bd7ba207c5196b61d Change-Id: I9fdd035a61715039b737bf65df5c5636e8a55f54 --- HACKING.rst | 2 +- manila/hacking/checks.py | 39 +++++++++++++++++++----------- manila/tests/test_hacking.py | 47 ++++++++++++++++++++++++++++++------ 3 files changed, 66 insertions(+), 22 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 93dbb65dcd..066eb81773 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -13,7 +13,7 @@ Manila Specific Commandments - [M313] Use assertTrue(...) rather than assertEqual(True, ...). - [M319] Validate that debug level logs are not translated. - [M323] Ensure that the _() function is explicitly imported to ensure proper translations. -- [M325] str() cannot be used on an exception. Remove use or use six.text_type() +- [M325] str() and unicode() cannot be used on an exception. Remove or use six.text_type(). - [M326] Translated messages cannot be concatenated. String should be included in translated message. - [M328] LOG.critical messages require translations _LC()! diff --git a/manila/hacking/checks.py b/manila/hacking/checks.py index 3aa3582006..3bc84b770e 100644 --- a/manila/hacking/checks.py +++ b/manila/hacking/checks.py @@ -170,40 +170,51 @@ def check_explicit_underscore_import(logical_line, filename): yield(0, "M323: Found use of _() without explicit import of _ !") -class CheckForStrExc(BaseASTChecker): - """Checks for the use of str() on an exception. +class CheckForStrUnicodeExc(BaseASTChecker): + """Checks for the use of str() or unicode() on an exception. - This currently only handles the case where str() is used in - the scope of an exception handler. If the exception is passed - into a function, returned from an assertRaises, or used on an - exception created in the same scope, this does not catch it. + This currently only handles the case where str() or unicode() + is used in the scope of an exception handler. If the exception + is passed into a function, returned from an assertRaises, or + used on an exception created in the same scope, this does not + catch it. """ - CHECK_DESC = ('M325 str() cannot be used on an exception. ' - 'Remove or use six.text_type()') + CHECK_DESC = ('M325 str() and unicode() cannot be used on an ' + 'exception. Remove or use six.text_type()') def __init__(self, tree, filename): - super(CheckForStrExc, self).__init__(tree, filename) + super(CheckForStrUnicodeExc, self).__init__(tree, filename) self.name = [] self.already_checked = [] + # Python 2 def visit_TryExcept(self, node): for handler in node.handlers: if handler.name: self.name.append(handler.name.id) - super(CheckForStrExc, self).generic_visit(node) + super(CheckForStrUnicodeExc, self).generic_visit(node) self.name = self.name[:-1] else: - super(CheckForStrExc, self).generic_visit(node) + super(CheckForStrUnicodeExc, self).generic_visit(node) + + # Python 3 + def visit_ExceptHandler(self, node): + if node.name: + self.name.append(node.name) + super(CheckForStrUnicodeExc, self).generic_visit(node) + self.name = self.name[:-1] + else: + super(CheckForStrUnicodeExc, self).generic_visit(node) def visit_Call(self, node): - if self._check_call_names(node, ['str']): + if self._check_call_names(node, ['str', 'unicode']): if node not in self.already_checked: self.already_checked.append(node) if isinstance(node.args[0], ast.Name): if node.args[0].id in self.name: self.add_error(node.args[0]) - super(CheckForStrExc, self).generic_visit(node) + super(CheckForStrUnicodeExc, self).generic_visit(node) class CheckForTransAdd(BaseASTChecker): @@ -268,8 +279,8 @@ def validate_assertIsNone(logical_line): def factory(register): register(validate_log_translations) register(check_explicit_underscore_import) + register(CheckForStrUnicodeExc) register(no_translate_debug_logs) - register(CheckForStrExc) register(CheckForTransAdd) register(check_oslo_namespace_imports) register(dict_constructor_with_list_copy) diff --git a/manila/tests/test_hacking.py b/manila/tests/test_hacking.py index b05a4aebfc..4cb54067d1 100644 --- a/manila/tests/test_hacking.py +++ b/manila/tests/test_hacking.py @@ -17,8 +17,6 @@ import textwrap import mock import pep8 -import six -import testtools from manila.hacking import checks from manila import test @@ -126,10 +124,12 @@ class HackingTestCase(test.TestCase): self._run_check(code, checker, filename)] self.assertEqual(expected_errors or [], actual_errors) - @testtools.skipIf(six.PY3, "It is PY2-specific. Skip it for PY3.") - def test_str_exception(self): + def _assert_has_no_errors(self, code, checker, filename=None): + self._assert_has_errors(code, checker, filename=filename) - checker = checks.CheckForStrExc + def test_str_on_exception(self): + + checker = checks.CheckForStrUnicodeExc code = """ def f(a, b): try: @@ -141,6 +141,20 @@ class HackingTestCase(test.TestCase): errors = [(5, 16, 'M325')] self._assert_has_errors(code, checker, expected_errors=errors) + def test_no_str_unicode_on_exception(self): + checker = checks.CheckForStrUnicodeExc + code = """ + def f(a, b): + try: + p = unicode(a) + str(b) + except ValueError as e: + p = e + return p + """ + self._assert_has_no_errors(code, checker) + + def test_unicode_on_exception(self): + checker = checks.CheckForStrUnicodeExc code = """ def f(a, b): try: @@ -149,9 +163,11 @@ class HackingTestCase(test.TestCase): p = unicode(e) return p """ - errors = [] + errors = [(5, 20, 'M325')] self._assert_has_errors(code, checker, expected_errors=errors) + def test_str_on_multiple_exceptions(self): + checker = checks.CheckForStrUnicodeExc code = """ def f(a, b): try: @@ -161,12 +177,29 @@ class HackingTestCase(test.TestCase): p = unicode(a) + unicode(b) except ValueError as ve: p = str(e) + str(ve) - p = unicode(e) + p = e return p """ errors = [(8, 20, 'M325'), (8, 29, 'M325')] self._assert_has_errors(code, checker, expected_errors=errors) + def test_str_unicode_on_multiple_exceptions(self): + checker = checks.CheckForStrUnicodeExc + code = """ + def f(a, b): + try: + p = str(a) + str(b) + except ValueError as e: + try: + p = unicode(a) + unicode(b) + except ValueError as ve: + p = str(e) + unicode(ve) + p = str(e) + return p + """ + errors = [(8, 20, 'M325'), (8, 33, 'M325'), (9, 16, 'M325')] + self._assert_has_errors(code, checker, expected_errors=errors) + def test_trans_add(self): checker = checks.CheckForTransAdd