Add W504 for line breaks before binary operators

This flips the W503 rule to enforce line breaks before binary operators.

Related #498

3 files changed, 100 insertions, 49 deletions

diff --git a/pycodestyle.py b/pycodestyle.py
index eb9ff63..ad17382 100755
--- a/pycodestyle.py
+++ b/pycodestyle.py
@@ -80,7 +80,7 @@ except ImportError:
 __version__ = '2.3.1'
 
 DEFAULT_EXCLUDE = '.svn,CVS,.bzr,.hg,.git,__pycache__,.tox'
-DEFAULT_IGNORE = 'E121,E123,E126,E226,E24,E704,W503'
+DEFAULT_IGNORE = 'E121,E123,E126,E226,E24,E704,W503,W504'
 try:
     if sys.platform == 'win32':
         USER_CONFIG = os.path.expanduser(r'~\.pycodestyle')
@@ -1135,8 +1135,47 @@ def explicit_line_join(logical_line, tokens):
                 parens -= 1
 
 
+def _is_binary_operator(token_type, text):
+    is_op_token = token_type == tokenize.OP
+    is_conjunction = text in ['and', 'or']
+    # NOTE(sigmavirus24): Previously the not_a_symbol check was executed
+    # conditionally. Since it is now *always* executed, text may be None.
+    # In that case we get a TypeError for `text not in str`.
+    not_a_symbol = text and text not in "()[]{},:.;@=%~"
+    # The % character is strictly speaking a binary operator, but the
+    # common usage seems to be to put it next to the format parameters,
+    # after a line break.
+    return ((is_op_token or is_conjunction) and not_a_symbol)
+
+
+def _break_around_binary_operators(tokens):
+    """Private function to reduce duplication.
+
+    This factors out the shared details between
+    :func:`break_before_binary_operator` and
+    :func:`break_after_binary_operator`.
+    """
+    line_break = False
+    unary_context = True
+    # Previous non-newline token types and text
+    previous_token_type = None
+    previous_text = None
+    for token_type, text, start, end, line in tokens:
+        if token_type == tokenize.COMMENT:
+            continue
+        if ('\n' in text or '\r' in text) and token_type != tokenize.STRING:
+            line_break = True
+        else:
+            yield (token_type, text, previous_token_type, previous_text,
+                   line_break, unary_context, start)
+            unary_context = text in '([{,;'
+            line_break = False
+            previous_token_type = token_type
+            previous_text = text
+
+
 @register_check
-def break_around_binary_operator(logical_line, tokens):
+def break_before_binary_operator(logical_line, tokens):
     r"""
     Avoid breaks before binary operators.
 
@@ -1156,33 +1195,47 @@ def break_around_binary_operator(logical_line, tokens):
     Okay: var = (1 /\n       -2)
     Okay: var = (1 +\n       -1 +\n       -2)
     """
-    def is_binary_operator(token_type, text):
-        # The % character is strictly speaking a binary operator, but the
-        # common usage seems to be to put it next to the format parameters,
-        # after a line break.
-        return ((token_type == tokenize.OP or text in ['and', 'or']) and
-                text not in "()[]{},:.;@=%~")
+    for context in _break_around_binary_operators(tokens):
+        (token_type, text, previous_token_type, previous_text,
+         line_break, unary_context, start) = context
+        if (_is_binary_operator(token_type, text) and line_break and
+                not unary_context and
+                not _is_binary_operator(previous_token_type,
+                                        previous_text)):
+            yield start, "W503 line break before binary operator"
 
-    line_break = False
-    unary_context = True
-    # Previous non-newline token types and text
-    previous_token_type = None
-    previous_text = None
-    for token_type, text, start, end, line in tokens:
-        if token_type == tokenize.COMMENT:
-            continue
-        if ('\n' in text or '\r' in text) and token_type != tokenize.STRING:
-            line_break = True
-        else:
-            if (is_binary_operator(token_type, text) and line_break and
-                    not unary_context and
-                    not is_binary_operator(previous_token_type,
-                                           previous_text)):
-                yield start, "W503 line break before binary operator"
-            unary_context = text in '([{,;'
-            line_break = False
-            previous_token_type = token_type
-            previous_text = text
+
+@register_check
+def break_after_binary_operator(logical_line, tokens):
+    r"""
+    Avoid breaks after binary operators.
+
+    The preferred place to break around a binary operator is after the
+    operator, not before it.
+
+    W504: (width == 0 +\n height == 0)
+    W504: (width == 0 and\n height == 0)
+
+    Okay: (width == 0\n + height == 0)
+    Okay: foo(\n    -x)
+    Okay: foo(x\n    [])
+    Okay: x = '''\n''' + ''
+    Okay: x = '' + '''\n'''
+    Okay: foo(x,\n    -y)
+    Okay: foo(x,  # comment\n    -y)
+    Okay: var = (1\n       & ~2)
+    Okay: var = (1\n       / -2)
+    Okay: var = (1\n       + -1\n       + -2)
+    """
+    for context in _break_around_binary_operators(tokens):
+        (token_type, text, previous_token_type, previous_text,
+         line_break, unary_context, start) = context
+        if (_is_binary_operator(previous_token_type, previous_text)
+                and line_break
+                and not unary_context
+                and not _is_binary_operator(token_type, text)):
+            error_pos = (start[0] - 1, start[1])
+            yield error_pos, "W504 line break after binary operator"
 
 
 @register_check
diff --git a/testsuite/E12.py b/testsuite/E12.py
index a995c95..acdd81f 100644
--- a/testsuite/E12.py
+++ b/testsuite/E12.py
@@ -20,9 +20,9 @@ print "E124", ("visual",
 #: E124
 a = (123,
 )
-#: E129
-if (row < 0 or self.moduleCount <= row or
-    col < 0 or self.moduleCount <= col):
+#: E129 W503
+if (row < 0 or self.moduleCount <= row
+    or col < 0 or self.moduleCount <= col):
     raise Exception("%s,%s - %s" % (row, col, self.moduleCount))
 #: E126
 print "E126", (
@@ -195,9 +195,9 @@ def qualify_by_address(
     self, cr, uid, ids, context=None,
     params_to_check=frozenset(QUALIF_BY_ADDRESS_PARAM)):
     """ This gets called by the web server """
-#: E129
-if (a == 2 or
-    b == "abc def ghi"
+#: E129 W503
+if (a == 2
+    or b == "abc def ghi"
     "jkl mno"):
     return True
 #:
@@ -225,22 +225,21 @@ rv.update(dict.fromkeys((
 eat_a_dict_a_day({
         "foo": "bar",
 })
-#: E126
+#: E126 W503
 if (
     x == (
             3
-    ) or
-        y == 4):
+    )
+        or y == 4):
     pass
-#: E126
+#: E126 W503 W503
 if (
     x == (
         3
-    ) or
-    x == (
-            3
-    ) or
-        y == 4):
+    )
+    or x == (
+            3)
+        or y == 4):
     pass
 #: E131
 troublesome_hash = {
diff --git a/testsuite/E12not.py b/testsuite/E12not.py
index 6528107..34d6efe 100644
--- a/testsuite/E12not.py
+++ b/testsuite/E12not.py
@@ -1,8 +1,7 @@
 if (
         x == (
             3
-        ) or
-        y == 4):
+        ) or y == 4):
     pass
 
 y = x == 2 \
@@ -19,13 +18,13 @@ if x == 2 \
     pass
 
 
-if (foo == bar and
-        baz == frop):
+if (foo == bar
+        and baz == frop):
     pass
 
 if (
-    foo == bar and
-    baz == frop
+    foo == bar
+    and baz == frop
 ):
     pass