From 27fe4ad04fd61979ae8b734d13a569a29a3b79bd Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Sun, 10 Feb 2019 02:48:32 -0800 Subject: [PATCH] Add knobs to split before an arithmetic operator. Closes #658 --- CHANGELOG | 6 +++++- README.rst | 13 ++++++++++--- yapf/yapflib/comment_splicer.py | 4 ++-- yapf/yapflib/format_token.py | 12 ++++++------ yapf/yapflib/split_penalty.py | 12 ++++++++---- yapf/yapflib/style.py | 10 ++++++++++ yapf/yapflib/unwrapped_line.py | 15 +++++---------- yapftests/reformatter_buganizer_test.py | 8 ++++---- yapftests/reformatter_pep8_test.py | 25 +++++++++++++++++++++++-- 9 files changed, 73 insertions(+), 32 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 107404d0f..ee72fdb4d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,7 +2,11 @@ # All notable changes to this project will be documented in this file. # This project adheres to [Semantic Versioning](http://semver.org/). -## [0.26.1] UNRELEASED +## [0.27.0] UNRELEASED +### Added +- `SPLIT_BEFORE_ARITHMETIC_OPERATOR` splits before an arithmetic operator when + set. `SPLIT_PENALTY_ARITHMETIC_OPERATOR` allows you to set the split penalty + around arithmetic operators. ### Fixed - Parse integer lists correctly, removing quotes if the list is within a string. diff --git a/README.rst b/README.rst index 388ddaaf0..d435334c0 100644 --- a/README.rst +++ b/README.rst @@ -564,9 +564,12 @@ Knobs If a comma separated list (dict, list, tuple, or function def) is on a line that is too long, split such that all elements are on a single line. -``SPLIT_BEFORE_BITWISE_OPERATOR`` - Set to ``True`` to prefer splitting before ``&``, ``|`` or ``^`` rather - than after. +``SPLIT_BEFORE__OPERATOR`` + Set to True to prefer splitting before '&', '|' or '^' rather than after. + +``SPLIT_BEFORE_ARITHMETIC_OPERATOR`` + Set to True to prefer splitting before '+', '-', '*', '/', '//', or '@' + rather than after. ``SPLIT_BEFORE_CLOSING_BRACKET`` Split before the closing bracket if a list or dict literal doesn't fit on @@ -639,6 +642,10 @@ Knobs ``SPLIT_PENALTY_AFTER_UNARY_OPERATOR`` The penalty for splitting the line after a unary operator. +``SPLIT_PENALTY_ARITHMETIC_OPERATOR`` + The penalty of splitting the line around the ``+``, ``-``, ``*``, ``/``, + ``//``, ``%``, and ``@`` operators. + ``SPLIT_PENALTY_BEFORE_IF_EXPR`` The penalty for splitting right before an ``if`` expression. diff --git a/yapf/yapflib/comment_splicer.py b/yapf/yapflib/comment_splicer.py index af999d260..8717394f0 100644 --- a/yapf/yapflib/comment_splicer.py +++ b/yapf/yapflib/comment_splicer.py @@ -177,8 +177,8 @@ def _VisitNodeRec(node): rindex = (0 if '\n' not in comment_prefix.rstrip() else comment_prefix.rstrip().rindex('\n') + 1) comment_column = ( - len(comment_prefix[rindex:]) - len( - comment_prefix[rindex:].lstrip())) + len(comment_prefix[rindex:]) - + len(comment_prefix[rindex:].lstrip())) comments = _CreateCommentsFromPrefix( comment_prefix, comment_lineno, diff --git a/yapf/yapflib/format_token.py b/yapf/yapflib/format_token.py index 8d41b900c..e7ed764a4 100644 --- a/yapf/yapflib/format_token.py +++ b/yapf/yapflib/format_token.py @@ -169,8 +169,8 @@ def AddWhitespacePrefix(self, newlines_before, spaces=0, indent_level=0): else: indent_before = '\t' * indent_level + ' ' * spaces else: - indent_before = ( - ' ' * indent_level * style.Get('INDENT_WIDTH') + ' ' * spaces) + indent_before = (' ' * indent_level * style.Get('INDENT_WIDTH') + + ' ' * spaces) if self.is_comment: comment_lines = [s.lstrip() for s in self.value.splitlines()] @@ -180,15 +180,15 @@ def AddWhitespacePrefix(self, newlines_before, spaces=0, indent_level=0): self.value = self.node.value if not self.whitespace_prefix: - self.whitespace_prefix = ( - '\n' * (self.newlines or newlines_before) + indent_before) + self.whitespace_prefix = ('\n' * (self.newlines or newlines_before) + + indent_before) else: self.whitespace_prefix += indent_before def AdjustNewlinesBefore(self, newlines_before): """Change the number of newlines before this token.""" - self.whitespace_prefix = ( - '\n' * newlines_before + self.whitespace_prefix.lstrip('\n')) + self.whitespace_prefix = ('\n' * newlines_before + + self.whitespace_prefix.lstrip('\n')) def RetainHorizontalSpacing(self, first_column, depth): """Retains a token's horizontal spacing.""" diff --git a/yapf/yapflib/split_penalty.py b/yapf/yapflib/split_penalty.py index 3ae6538b4..e9bf44877 100644 --- a/yapf/yapflib/split_penalty.py +++ b/yapf/yapflib/split_penalty.py @@ -550,10 +550,14 @@ def _SetBitwiseOperandPenalty(node, op): def _SetExpressionOperandPenalty(node, ops, penalty): for index in py3compat.range(1, len(node.children) - 1): - child = node.children[index] - if pytree_utils.NodeName(child) in ops: - next_node = pytree_utils.FirstLeafNode(node.children[index + 1]) - _SetSplitPenalty(next_node, penalty) + child = node.children[index] + if pytree_utils.NodeName(child) in ops: + if style.Get('SPLIT_BEFORE_ARITHMETIC_OPERATOR'): + _SetSplitPenalty(child, style.Get('SPLIT_PENALTY_ARITHMETIC_OPERATOR')) + else: + _SetSplitPenalty( + pytree_utils.FirstLeafNode(node.children[index + 1]), + style.Get('SPLIT_PENALTY_ARITHMETIC_OPERATOR')) def _IncreasePenalty(node, amt): diff --git a/yapf/yapflib/style.py b/yapf/yapflib/style.py index 7d2839497..c01e7a2db 100644 --- a/yapf/yapflib/style.py +++ b/yapf/yapflib/style.py @@ -241,6 +241,9 @@ def method(): comma."""), SPLIT_ALL_COMMA_SEPARATED_VALUES=textwrap.dedent("""\ Split before arguments"""), + SPLIT_BEFORE_ARITHMETIC_OPERATOR=textwrap.dedent("""\ + Set to True to prefer splitting before '+', '-', '*', '/', '//', or '@' + rather than after."""), SPLIT_BEFORE_BITWISE_OPERATOR=textwrap.dedent("""\ Set to True to prefer splitting before '&', '|' or '^' rather than after."""), @@ -297,6 +300,9 @@ def method(): The penalty for splitting right after the opening bracket."""), SPLIT_PENALTY_AFTER_UNARY_OPERATOR=textwrap.dedent("""\ The penalty for splitting the line after a unary operator."""), + SPLIT_PENALTY_ARITHMETIC_OPERATOR=textwrap.dedent("""\ + The penalty of splitting the line around the '+', '-', '*', '/', '//', + ``%``, and '@' operators."""), SPLIT_PENALTY_BEFORE_IF_EXPR=textwrap.dedent("""\ The penalty for splitting right before an if expression."""), SPLIT_PENALTY_BITWISE_OPERATOR=textwrap.dedent("""\ @@ -363,6 +369,7 @@ def CreatePEP8Style(): SPACES_BEFORE_COMMENT=2, SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=False, SPLIT_ALL_COMMA_SEPARATED_VALUES=False, + SPLIT_BEFORE_ARITHMETIC_OPERATOR=False, SPLIT_BEFORE_BITWISE_OPERATOR=True, SPLIT_BEFORE_CLOSING_BRACKET=True, SPLIT_BEFORE_DICT_SET_GENERATOR=True, @@ -374,6 +381,7 @@ def CreatePEP8Style(): SPLIT_COMPLEX_COMPREHENSION=False, SPLIT_PENALTY_AFTER_OPENING_BRACKET=30, SPLIT_PENALTY_AFTER_UNARY_OPERATOR=10000, + SPLIT_PENALTY_ARITHMETIC_OPERATOR=300, SPLIT_PENALTY_BEFORE_IF_EXPR=0, SPLIT_PENALTY_BITWISE_OPERATOR=300, SPLIT_PENALTY_COMPREHENSION=80, @@ -536,6 +544,7 @@ def _IntOrIntListConverter(s): SPACES_BEFORE_COMMENT=_IntOrIntListConverter, SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=_BoolConverter, SPLIT_ALL_COMMA_SEPARATED_VALUES=_BoolConverter, + SPLIT_BEFORE_ARITHMETIC_OPERATOR=_BoolConverter, SPLIT_BEFORE_BITWISE_OPERATOR=_BoolConverter, SPLIT_BEFORE_CLOSING_BRACKET=_BoolConverter, SPLIT_BEFORE_DICT_SET_GENERATOR=_BoolConverter, @@ -547,6 +556,7 @@ def _IntOrIntListConverter(s): SPLIT_COMPLEX_COMPREHENSION=_BoolConverter, SPLIT_PENALTY_AFTER_OPENING_BRACKET=int, SPLIT_PENALTY_AFTER_UNARY_OPERATOR=int, + SPLIT_PENALTY_ARITHMETIC_OPERATOR=int, SPLIT_PENALTY_BEFORE_IF_EXPR=int, SPLIT_PENALTY_BITWISE_OPERATOR=int, SPLIT_PENALTY_COMPREHENSION=int, diff --git a/yapf/yapflib/unwrapped_line.py b/yapf/yapflib/unwrapped_line.py index b45d12a9f..fb1275583 100644 --- a/yapf/yapflib/unwrapped_line.py +++ b/yapf/yapflib/unwrapped_line.py @@ -228,7 +228,7 @@ def _IsUnaryOperator(tok): def _HasPrecedence(tok): - """Whether a binary operation has presedence within its context.""" + """Whether a binary operation has precedence within its context.""" node = tok.node # We let ancestor be the statement surrounding the operation that tok is the @@ -241,12 +241,12 @@ def _HasPrecedence(tok): predecessor_type = pytree_utils.NodeName(ancestor) if predecessor_type in ['arith_expr', 'term']: # An ancestor "arith_expr" or "term" means we have found an operator - # with lower presedence than our tok. + # with lower precedence than our tok. return True if predecessor_type != 'atom': # We understand the context to look for precedence within as an # arbitrary nesting of "arith_expr", "term", and "atom" nodes. If we - # leave this context we have not found a lower presedence operator. + # leave this context we have not found a lower precedence operator. return False # Under normal usage we expect a complete parse tree to be available and # we will return before we get an AttributeError from the root. @@ -254,7 +254,7 @@ def _HasPrecedence(tok): def _PriorityIndicatingNoSpace(tok): - """Whether to remove spaces around an operator due to presedence.""" + """Whether to remove spaces around an operator due to precedence.""" if not tok.is_arithmetic_op or not tok.is_simple_expr: # Limit space removal to highest priority arithmetic operators return False @@ -519,7 +519,7 @@ def IsSurroundedByBrackets(tok): _LOGICAL_OPERATORS = frozenset({'and', 'or'}) _BITWISE_OPERATORS = frozenset({'&', '|', '^'}) -_TERM_OPERATORS = frozenset({'*', '/', '%', '//', '@'}) +_ARITHMETIC_OPERATORS = frozenset({'+', '-', '*', '/', '%', '//', '@'}) def _SplitPenalty(prev_token, cur_token): @@ -568,9 +568,6 @@ def _SplitPenalty(prev_token, cur_token): if pval == ',': # Breaking after a comma is fine, if need be. return 0 - if prev_token.is_binary_op: - # We would rather not split after an equality operator. - return split_penalty.CONNECTED if pval == '**' or cval == '**': return split_penalty.STRONGLY_CONNECTED if (format_token.Subtype.VARARGS_STAR in prev_token.subtypes or @@ -596,6 +593,4 @@ def _SplitPenalty(prev_token, cur_token): if cur_token.ClosesScope(): # Give a slight penalty for splitting before the closing scope. return 100 - if pval in _TERM_OPERATORS or cval in _TERM_OPERATORS: - return 50 return 0 diff --git a/yapftests/reformatter_buganizer_test.py b/yapftests/reformatter_buganizer_test.py index 38e4437f1..22309ac34 100644 --- a/yapftests/reformatter_buganizer_test.py +++ b/yapftests/reformatter_buganizer_test.py @@ -840,8 +840,8 @@ def _(): expected_formatted_code = textwrap.dedent("""\ class _(): def _(): - hints.append(('hg tag -f -l -r %s %s # %s' % (short( - ctx.node()), candidatetag, firstline))[:78]) + hints.append(('hg tag -f -l -r %s %s # %s' % + (short(ctx.node()), candidatetag, firstline))[:78]) """) uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code) self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines)) @@ -1457,8 +1457,8 @@ def foo(): expected_formatted_code = textwrap.dedent("""\ def foo(): if True: - return ( - struct.pack('aaaa', bbbbbbbbbb, ccccccccccccccc, dddddddd) + eeeeeee) + return (struct.pack('aaaa', bbbbbbbbbb, ccccccccccccccc, dddddddd) + + eeeeeee) """) uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code) self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines)) diff --git a/yapftests/reformatter_pep8_test.py b/yapftests/reformatter_pep8_test.py index 635760ade..729b33cf1 100644 --- a/yapftests/reformatter_pep8_test.py +++ b/yapftests/reformatter_pep8_test.py @@ -165,8 +165,8 @@ def testIndentSizeChanging(self): """) expected_formatted_code = textwrap.dedent("""\ if True: - runtime_mins = ( - program_end_time - program_start_time).total_seconds() / 60.0 + runtime_mins = (program_end_time - + program_start_time).total_seconds() / 60.0 """) uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code) self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines)) @@ -454,6 +454,27 @@ def _(): uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code) self.assertEqual(expected_code, reformatter.Reformat(uwlines)) + def testSplitBeforeArithmeticOperators(self): + try: + style.SetGlobalStyle( + style.CreateStyleFromConfig( + '{based_on_style: pep8, split_before_arithmetic_operator: true}')) + + unformatted_code = """\ +def _(): + raise ValueError('This is a long message that ends with an argument: ' + str(42)) +""" + expected_formatted_code = """\ +def _(): + raise ValueError('This is a long message that ends with an argument: ' + + str(42)) +""" + uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code) + self.assertCodeEqual(expected_formatted_code, + reformatter.Reformat(uwlines)) + finally: + style.SetGlobalStyle(style.CreatePEP8Style()) + if __name__ == '__main__': unittest.main()