diff --git a/HACKING.rst b/HACKING.rst index 78f2640..109f348 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -8,3 +8,4 @@ masakari-monitors Specific Commandments - [M301] Ensure that the _() function is explicitly imported to ensure proper translations. - [M302] Validate that log messages are not translated. +- [M303] Yield must always be followed by a space when yielding a value. diff --git a/masakarimonitors/hacking/checks.py b/masakarimonitors/hacking/checks.py index 43cfd6b..9594cb3 100644 --- a/masakarimonitors/hacking/checks.py +++ b/masakarimonitors/hacking/checks.py @@ -47,6 +47,7 @@ log_translation = re.compile( r"\(" r"(_|_LC|_LE|_LI|_LW)" r"\(") +yield_not_followed_by_space = re.compile(r"^\s*yield(?:\(|{|\[|\"|').*$") def check_explicit_underscore_import(logical_line, filename): @@ -69,7 +70,7 @@ def check_explicit_underscore_import(logical_line, filename): UNDERSCORE_IMPORT_FILES.append(filename) elif(translated_log.match(logical_line) or string_translation.match(logical_line)): - yield(0, "M301: Found use of _() without explicit import of _ !") + yield (0, "M301: Found use of _() without explicit import of _ !") def no_translate_logs(logical_line): @@ -77,9 +78,27 @@ def no_translate_logs(logical_line): M302 """ if log_translation.match(logical_line): - yield(0, "M302 Don't translate log messages!") + yield (0, "M302 Don't translate log messages!") + + +def yield_followed_by_space(logical_line): + """Yield should be followed by a space. + + Yield should be followed by a space to clarify that yield is + not a function. Adding a space may force the developer to rethink + if there are unnecessary parentheses in the written code. + + Not correct: yield(x), yield(a, b) + Correct: yield x, yield (a, b), yield a, b + + M303 + """ + if yield_not_followed_by_space.match(logical_line): + yield (0, + "M303: Yield keyword should be followed by a space.") def factory(register): register(check_explicit_underscore_import) register(no_translate_logs) + register(yield_followed_by_space) diff --git a/masakarimonitors/tests/unit/test_hacking.py b/masakarimonitors/tests/unit/test_hacking.py index bb69106..ddff306 100644 --- a/masakarimonitors/tests/unit/test_hacking.py +++ b/masakarimonitors/tests/unit/test_hacking.py @@ -12,6 +12,10 @@ # License for the specific language governing permissions and limitations # under the License. +import textwrap + +import mock +import pep8 import testtools from masakarimonitors.hacking import checks @@ -49,6 +53,30 @@ class HackingTestCase(testtools.TestCase): just assertTrue if the check is expected to fail and assertFalse if it should pass. """ + + # We are patching pep8 so that only the check under test is actually + # installed. + @mock.patch('pep8._checks', + {'physical_line': {}, 'logical_line': {}, 'tree': {}}) + def _run_check(self, code, checker, filename=None): + pep8.register_check(checker) + + lines = textwrap.dedent(code).strip().splitlines(True) + + checker = pep8.Checker(filename=filename, lines=lines) + checker.check_all() + checker.report._deferred_print.sort() + return checker.report._deferred_print + + def _assert_has_errors(self, code, checker, expected_errors=None, + filename=None): + actual_errors = [e[:3] for e in + self._run_check(code, checker, filename)] + self.assertEqual(expected_errors or [], actual_errors) + + def _assert_has_no_errors(self, code, checker, filename=None): + self._assert_has_errors(code, checker, filename=filename) + def test_check_explicit_underscore_import(self): self.assertEqual(len(list(checks.check_explicit_underscore_import( "LOG.info(_('My info message'))", @@ -93,3 +121,25 @@ class HackingTestCase(testtools.TestCase): self.assertEqual(1, len(list(checks.no_translate_logs( "LOG.critical(_LC('foo'))")))) + + def test_yield_followed_by_space(self): + code = """ + yield(x, y) + yield{"type": "test"} + yield[a, b, c] + yield"test" + yield'test' + """ + errors = [(x + 1, 0, 'M303') for x in range(5)] + self._assert_has_errors(code, checks.yield_followed_by_space, + expected_errors=errors) + code = """ + yield x + yield (x, y) + yield {"type": "test"} + yield [a, b, c] + yield "test" + yield 'test' + yieldx_func(a, b) + """ + self._assert_has_no_errors(code, checks.yield_followed_by_space)