Merge "Removed unnecessary parantheses in yield statements"

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.

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)

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)