Stablize reformatting with Facebook style
The line number cannot be used to determine if the line should be split when
"dedenting" is specified. The reason for this is because a newline may be added
without splitting around the brackets. Then if we rerun yapf on the output, the
code will be on different lines, and therefore be formatted again.
This change doesn't rely upon the token's original line number.
Closes #210
diff --git a/CHANGELOG b/CHANGELOG
index 2bdf8ab..78c3f6b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,16 +2,16 @@
# All notable changes to this project will be documented in this file.
# This project adheres to [Semantic Versioning](http://semver.org/).
-## [0.9.0] UNRELEASED
-### Added
-- Add a knob, 'NO_SPLIT_WHEN_BIN_PACKING', that prevents splitting before the
- closing bracket of a function call if the arguments are bin packed. This is
- used in conjunction with 'DEDENT_CLOSING_BRACKETS'.
-
+## [0.8.1] UNRELEASED
### Fixed
- 'SPLIT_BEFORE_LOGICAL_OPERATOR' wasn't working correctly. The penalty was
being set incorrectly when it was part of a larger construct.
- Don't separate a keyword, like "await", from a left paren.
+- Don't rely upon the original tokens' line number to determine if we should
+ perform splitting in Facebook mode. The line number isn't the line number of
+ the reformatted token, but the line number where it was in the original code.
+ Instead, we need to carefully determine if the line is liabel to be split and
+ act accordingly.
## [0.8.0] 2016-05-10
### Added
diff --git a/README.rst b/README.rst
index 6bbc053..9adf63b 100644
--- a/README.rst
+++ b/README.rst
@@ -322,11 +322,6 @@
end_ts=now(),
) # <--- this bracket is dedented and on a separate line
-``NO_SPLIT_WHEN_BIN_PACKING``
- Used in conjuction with ``DEDENT_CLOSING_BRACKETS``, it doesn't split
- before the closing bracket of a function call if the arguments are bin
- packed.
-
``I18N_COMMENT``
The regex for an internationalization comment. The presence of this comment
stops reformatting of that line, because the comments are required to be
diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py
index dad4775..0eaece4 100644
--- a/yapf/yapflib/format_decision_state.py
+++ b/yapf/yapflib/format_decision_state.py
@@ -128,6 +128,29 @@
if current.must_break_before:
return True
+ if style.Get('DEDENT_CLOSING_BRACKETS'):
+ bracket = current if current.ClosesScope() else previous
+ if ((bracket.OpensScope() or bracket.ClosesScope()) and
+ format_token.Subtype.SUBSCRIPT_BRACKET not in bracket.subtypes):
+ if previous and previous.OpensScope():
+ length = (previous.matching_bracket.total_length -
+ previous.total_length)
+ if _IsLastScopeInLine(previous.matching_bracket):
+ last_token = _LastTokenInLine(previous.matching_bracket)
+ length = last_token.total_length - previous.total_length
+
+ if length + previous.column >= column_limit:
+ return True
+ elif current.ClosesScope():
+ opening = current.matching_bracket
+ length = current.total_length - opening.total_length
+ if _IsLastScopeInLine(current):
+ last_token = _LastTokenInLine(current)
+ length = last_token.total_length - opening.total_length
+
+ if length + opening.column >= column_limit:
+ return True
+
if (self.stack[-1].split_before_closing_bracket and
# FIXME(morbo): Use the 'matching_bracket' instead of this.
# FIXME(morbo): Don't forget about tuples!
@@ -463,6 +486,20 @@
return previous
+def _LastTokenInLine(current):
+ while current.next_token:
+ current = current.next_token
+ return current
+
+
+def _IsLastScopeInLine(current):
+ while current:
+ current = current.next_token
+ if current and (current.OpensScope() or current.ClosesScope()):
+ return False
+ return True
+
+
class _ParenState(object):
"""Maintains the state of the bracket enclosures.
diff --git a/yapf/yapflib/format_token.py b/yapf/yapflib/format_token.py
index 0227374..996eaef 100644
--- a/yapf/yapflib/format_token.py
+++ b/yapf/yapflib/format_token.py
@@ -38,16 +38,17 @@
UNARY_OPERATOR = 1
BINARY_OPERATOR = 2
SUBSCRIPT_COLON = 3
- DEFAULT_OR_NAMED_ASSIGN = 4
- VARARGS_STAR = 5
- KWARGS_STAR_STAR = 6
- ASSIGN_OPERATOR = 7
- DICTIONARY_KEY = 8
- DICTIONARY_VALUE = 9
- DICT_SET_GENERATOR = 10
- COMP_FOR = 11
- COMP_IF = 12
- DEFAULT_OR_NAMED_ASSIGN_ARG_LIST = 13
+ SUBSCRIPT_BRACKET = 4
+ DEFAULT_OR_NAMED_ASSIGN = 5
+ VARARGS_STAR = 6
+ KWARGS_STAR_STAR = 7
+ ASSIGN_OPERATOR = 8
+ DICTIONARY_KEY = 9
+ DICTIONARY_VALUE = 10
+ DICT_SET_GENERATOR = 11
+ COMP_FOR = 12
+ COMP_IF = 13
+ DEFAULT_OR_NAMED_ASSIGN_ARG_LIST = 14
class FormatToken(object):
diff --git a/yapf/yapflib/split_penalty.py b/yapf/yapflib/split_penalty.py
index a2a0a03..1518d65 100644
--- a/yapf/yapflib/split_penalty.py
+++ b/yapf/yapflib/split_penalty.py
@@ -197,7 +197,8 @@
break
if trailer.children[0].value in '([':
if len(trailer.children) > 2:
- self._SetUnbreakable(trailer.children[-1])
+ if not style.Get('DEDENT_CLOSING_BRACKETS'):
+ self._SetUnbreakable(trailer.children[-1])
if _FirstChildNode(trailer).lineno == _LastChildNode(trailer).lineno:
# If the trailer was originally on one line, then try to keep it
# like that.
diff --git a/yapf/yapflib/style.py b/yapf/yapflib/style.py
index e60aba7..5f7aa3c 100644
--- a/yapf/yapflib/style.py
+++ b/yapf/yapflib/style.py
@@ -100,10 +100,6 @@
end_ts=now(),
) # <--- this bracket is dedented and on a separate line
"""),
- NO_SPLIT_WHEN_BIN_PACKING=textwrap.dedent("""\
- Used in conjuction with DEDENT_CLOSING_BRACKETS, it doesn't split before
- the closing bracket of a function call if the arguments are bin packed.
- """),
JOIN_MULTIPLE_LINES=
"Join short lines into one line. E.g., single line 'if' statements.",
SPACE_BETWEEN_ENDING_COMMA_AND_CLOSING_BRACKET=textwrap.dedent("""\
@@ -155,7 +151,6 @@
ALLOW_MULTILINE_LAMBDAS=False,
COLUMN_LIMIT=79,
DEDENT_CLOSING_BRACKETS=False,
- NO_SPLIT_WHEN_BIN_PACKING=False,
I18N_COMMENT='',
I18N_FUNCTION_CALL='',
INDENT_DICTIONARY_VALUE=False,
@@ -242,7 +237,6 @@
ALLOW_MULTILINE_LAMBDAS=_BoolConverter,
COLUMN_LIMIT=int,
DEDENT_CLOSING_BRACKETS=_BoolConverter,
- NO_SPLIT_WHEN_BIN_PACKING=_BoolConverter,
I18N_COMMENT=str,
I18N_FUNCTION_CALL=_StringListConverter,
INDENT_DICTIONARY_VALUE=_BoolConverter,
diff --git a/yapf/yapflib/subtype_assigner.py b/yapf/yapflib/subtype_assigner.py
index e2dc6c3..63e1e28 100644
--- a/yapf/yapflib/subtype_assigner.py
+++ b/yapf/yapflib/subtype_assigner.py
@@ -198,6 +198,12 @@
if isinstance(child, pytree.Leaf) and child.value == '**':
self._AppendTokenSubtype(child, format_token.Subtype.BINARY_OPERATOR)
+ def Visit_trailer(self, node): # pylint: disable=invalid-name
+ for child in node.children:
+ self.Visit(child)
+ if isinstance(child, pytree.Leaf) and child.value in '[]':
+ self._AppendTokenSubtype(child, format_token.Subtype.SUBSCRIPT_BRACKET)
+
def Visit_subscript(self, node): # pylint: disable=invalid-name
# subscript ::= test | [test] ':' [test] [sliceop]
for child in node.children:
diff --git a/yapf/yapflib/unwrapped_line.py b/yapf/yapflib/unwrapped_line.py
index 3ce6011..bf740c3 100644
--- a/yapf/yapflib/unwrapped_line.py
+++ b/yapf/yapflib/unwrapped_line.py
@@ -326,21 +326,6 @@
# reasonable assumption, because otherwise they should have written them
# all on the same line, or with a '+'.
return True
- if style.Get('DEDENT_CLOSING_BRACKETS') and cur_token.ClosesScope():
- opening = cur_token.matching_bracket
- trailer_length = cur_token.total_length - opening.total_length
- if (trailer_length > style.Get('COLUMN_LIMIT') or
- cur_token.lineno != opening.lineno):
- # Since we're already dedenting the closing bracket, let's put a newline
- # after the opening one so that we have more horizontal space for the
- # trailer.
- opening.next_token.must_break_before = True
- if (style.Get('NO_SPLIT_WHEN_BIN_PACKING') and
- opening.previous_token and opening.previous_token.is_name and
- (not opening.previous_token.previous_token or
- opening.previous_token.previous_token.value != 'def')):
- return prev_token.value == ','
- return True
return pytree_utils.GetNodeAnnotation(cur_token.node,
pytree_utils.Annotation.MUST_SPLIT,
default=False)
diff --git a/yapftests/reformatter_test.py b/yapftests/reformatter_test.py
index 26180c5..3df727a 100644
--- a/yapftests/reformatter_test.py
+++ b/yapftests/reformatter_test.py
@@ -1599,8 +1599,7 @@
style.SetGlobalStyle(style.CreateStyleFromConfig(
'{based_on_style: pep8, indent_width: 2, '
'continuation_indent_width: 4, indent_dictionary_value: True, '
- 'dedent_closing_brackets: True, no_split_when_bin_packing: True, '
- 'split_before_named_assigns: False}'))
+ 'dedent_closing_brackets: True, split_before_named_assigns: False}'))
code = textwrap.dedent("""\
a_very_long_function_name(
long_argument_name_1=1,
@@ -1611,7 +1610,8 @@
a_very_long_function_name(
long_argument_name_1=1, long_argument_name_2=2, long_argument_name_3=3,
- long_argument_name_4=4)
+ long_argument_name_4=4
+ )
""")
uwlines = _ParseAndUnwrap(code)
reformatted_code = reformatter.Reformat(uwlines)
@@ -3118,15 +3118,15 @@
pass0_code = textwrap.dedent("""\
try:
pass
- except (IOError, OSError, LookupError, RuntimeError, OverflowError\
-) as exception:
+ except (IOError, OSError, LookupError, RuntimeError, OverflowError) as exception:
pass
""")
pass1_code = textwrap.dedent("""\
try:
pass
- except (IOError, OSError, LookupError, RuntimeError,
- OverflowError) as exception:
+ except (
+ IOError, OSError, LookupError, RuntimeError, OverflowError
+ ) as exception:
pass
""")
uwlines = _ParseAndUnwrap(pass0_code)
@@ -3166,6 +3166,43 @@
uwlines = _ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+ def testSimpleDedenting(self):
+ unformatted_code = textwrap.dedent("""\
+ if True:
+ self.assertEqual(result.reason_not_added, "current preflight is still running")
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ if True:
+ self.assertEqual(
+ result.reason_not_added, "current preflight is still running"
+ )
+ """)
+ uwlines = _ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
+ def testDedentingWithSubscripts(self):
+ unformatted_code = textwrap.dedent("""\
+ class Foo:
+ class Bar:
+ @classmethod
+ def baz(cls, clues_list, effect, constraints, constraint_manager):
+ if clues_lists:
+ return cls.single_constraint_not(clues_lists, effect, constraints[0], constraint_manager)
+
+ """)
+ expected_formatted_code = textwrap.dedent("""\
+ class Foo:
+ class Bar:
+ @classmethod
+ def baz(cls, clues_list, effect, constraints, constraint_manager):
+ if clues_lists:
+ return cls.single_constraint_not(
+ clues_lists, effect, constraints[0], constraint_manager
+ )
+ """)
+ uwlines = _ParseAndUnwrap(unformatted_code)
+ self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
def _ParseAndUnwrap(code, dumptree=False):
"""Produces unwrapped lines from the given code.
diff --git a/yapftests/subtype_assigner_test.py b/yapftests/subtype_assigner_test.py
index 58af588..d84d5a7 100644
--- a/yapftests/subtype_assigner_test.py
+++ b/yapftests/subtype_assigner_test.py
@@ -96,10 +96,10 @@
[('return', [format_token.Subtype.NONE]),
('-', {format_token.Subtype.UNARY_OPERATOR}),
('x', [format_token.Subtype.NONE]),
- ('[', [format_token.Subtype.NONE]),
+ ('[', {format_token.Subtype.SUBSCRIPT_BRACKET}),
(':', {format_token.Subtype.SUBSCRIPT_COLON}),
('42', [format_token.Subtype.NONE]),
- (']', [format_token.Subtype.NONE])]
+ (']', {format_token.Subtype.SUBSCRIPT_BRACKET})]
]) # yapf: disable
def testFuncCallWithDefaultAssign(self):
@@ -192,13 +192,13 @@
uwlines = self._ParseAndUnwrap(code)
self._CheckFormatTokenSubtypes(uwlines, [
[('x', [format_token.Subtype.NONE]),
- ('[', [format_token.Subtype.NONE]),
+ ('[', {format_token.Subtype.SUBSCRIPT_BRACKET}),
('0', [format_token.Subtype.NONE]),
(':', {format_token.Subtype.SUBSCRIPT_COLON}),
('42', [format_token.Subtype.NONE]),
(':', {format_token.Subtype.SUBSCRIPT_COLON}),
('1', [format_token.Subtype.NONE]),
- (']', [format_token.Subtype.NONE])]
+ (']', {format_token.Subtype.SUBSCRIPT_BRACKET})]
]) # yapf: disable
def testFunctionCallWithStarExpression(self):
