Repository: google/yapf
Pull request #808 (2020-02-02 03:47:18)

Adds FORCE_MULTILINE_DICT knob
I often find myself wanting to format my dictionaries with keys on their own lines, ala:

```
my_dict = {
    'spam': 'eggs',
    'foo': 'bar'
}
```

EACH_DICT_ENTRY_ON_SEPARATE_LINE did previous exist, but would only fire if the length of the dictionary was greater than the max column length. This patch allows it to always be turned on with the FORCE_MULTILINE_DICT config option, even for short dictionaries.

- Defaults to False (off)
- Adds docs
- Adds tests
- While in there, fixed two flaky tests that were dependent on previous tests setting up the environment

NOTE: There may be a better way to do that LBRACE check in reformatter.py. A quick glance through the codebase didn't show anything obvious, but let me know if I'm redoing work here.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/28477383/badge)](https://coveralls.io/builds/28477383)

Coverage decreased (-0.06%) to 96.218% when pulling **c6857ad5c17ef51e89c446f215bef76b61f2467a on brianmego:force_multiline_dict** into **e3b159be5fbac6bc2194cd1f90ec3f6f8ba60f3c on google:master**.

brianmego: Any thoughts on this?
brianmego: Thanks for the review! Spacing/README/CHANGELOG/paren patches have all been applied. Regarding the test note, I cannot remove that and also have the tests pass. Perhaps the addition of the new tests exposed a bit of flakiness in environment?
googlebot: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fyapf%2Fpull%2F808) for more info**.

<!-- need_author_cla -->
googlebot: CLAs look good, thanks!

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fyapf%2Fpull%2F808) for more info**.

<!-- ok -->
brianmego: Am I taking crazy pills with this test coverage? As far as I can tell, every line I added is now tested. I was so frustrated trying to find the line that decreased coverage that I just found some untested code and added tests around it, that made the coverage go down MORE! What am I missing here?

Patch:
diff --git a/CHANGELOG b/CHANGELOG
index e1feffb..236ca89 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -11,8 +11,12 @@
 - Don't over-indent a parameter list when not needed. But make sure it is
   properly indented so that it doesn't collide with the lines afterwards.
 - Don't split between two-word comparison operators: "is not", "not in", etc.
+- Adds `FORCE_MULTILINE_DICT` knob to ensure dictionaries always split,
+  even when shorter than the max line length.
 - New knob `SPACE_INSIDE_BRACKETS` to add spaces inside brackets, braces, and
   parentheses.
+- New knob `SPACES_AROUND_SUBSCRIPT_COLON` to add spaces around the subscript /
+  slice operator.
 
 ## [0.29.0] 2019-11-28
 ### Added
diff --git a/README.rst b/README.rst
index 1fd04ea..274e1f8 100644
--- a/README.rst
+++ b/README.rst
@@ -473,6 +473,10 @@ Knobs
 ``EACH_DICT_ENTRY_ON_SEPARATE_LINE``
     Place each dictionary entry onto its own line.
 
+``FORCE_MULTILINE_DICT``
+    Respect EACH_DICT_ENTRY_ON_SEPARATE_LINE even if the line is shorter than
+    COLUMN_LIMIT.
+
 ``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
@@ -545,6 +549,13 @@ Knobs
     Set to ``True`` to prefer spaces around the assignment operator for default
     or keyword arguments.
 
+``SPACES_AROUND_SUBSCRIPT_COLON``
+    Use spaces around the subscript / slice operator.  For example:
+
+    .. code-block:: python
+
+        my_list[1 : 10 : 2]
+
 ``SPACES_BEFORE_COMMENT``
     The number of spaces required before a trailing comment.
     This can be a single value (representing the number of spaces
diff --git a/yapf/yapflib/format_token.py b/yapf/yapflib/format_token.py
index 79b2b5c..e6af942 100644
--- a/yapf/yapflib/format_token.py
+++ b/yapf/yapflib/format_token.py
@@ -313,6 +313,12 @@ class FormatToken(object):
     """Token is an operator in a simple expression."""
     return Subtype.SIMPLE_EXPRESSION in self.subtypes
 
+  @property
+  @py3compat.lru_cache()
+  def is_subscript_colon(self):
+    """Token is a subscript colon."""
+    return Subtype.SUBSCRIPT_COLON in self.subtypes
+
   @property
   @py3compat.lru_cache()
   def name(self):
diff --git a/yapf/yapflib/reformatter.py b/yapf/yapflib/reformatter.py
index 0faa91f..7ec9888 100644
--- a/yapf/yapflib/reformatter.py
+++ b/yapf/yapflib/reformatter.py
@@ -257,6 +257,9 @@ def _CanPlaceOnSingleLine(uwline):
   Returns:
     True if the line can or should be added to a single line. False otherwise.
   """
+  token_names = [x.name for x in uwline.tokens]
+  if (style.Get('FORCE_MULTILINE_DICT') and 'LBRACE' in token_names):
+    return False
   indent_amt = style.Get('INDENT_WIDTH') * uwline.depth
   last = uwline.last
   last_index = -1
diff --git a/yapf/yapflib/style.py b/yapf/yapflib/style.py
index 5f2483f..3747941 100644
--- a/yapf/yapflib/style.py
+++ b/yapf/yapflib/style.py
@@ -172,6 +172,13 @@ _STYLE_HELP = dict(
       if the list is comma-terminated."""),
     EACH_DICT_ENTRY_ON_SEPARATE_LINE=textwrap.dedent("""\
       Place each dictionary entry onto its own line."""),
+    FORCE_MULTILINE_DICT=textwrap.dedent("""\
+      Require multiline dictionary even if it would normally fit on one line.
+      For example:
+
+        config = {
+            'key1': 'value1'
+        }"""),
     I18N_COMMENT=textwrap.dedent("""\
       The regex for an i18n comment. The presence of this comment stops
       reformatting of that line, because the comments are required to be
@@ -219,6 +226,11 @@ _STYLE_HELP = dict(
       Use spaces around the power operator."""),
     SPACES_AROUND_DEFAULT_OR_NAMED_ASSIGN=textwrap.dedent("""\
       Use spaces around default or named assigns."""),
+    SPACES_AROUND_SUBSCRIPT_COLON=textwrap.dedent("""\
+      Use spaces around the subscript / slice operator.  For example:
+
+        my_list[1 : 10 : 2]
+      """),
     SPACES_BEFORE_COMMENT=textwrap.dedent("""\
       The number of spaces required before a trailing comment.
       This can be a single value (representing the number of spaces
@@ -383,6 +395,7 @@ def CreatePEP8Style():
       INDENT_CLOSING_BRACKETS=False,
       DISABLE_ENDING_COMMA_HEURISTIC=False,
       EACH_DICT_ENTRY_ON_SEPARATE_LINE=True,
+      FORCE_MULTILINE_DICT=False,
       I18N_COMMENT='',
       I18N_FUNCTION_CALL='',
       INDENT_DICTIONARY_VALUE=False,
@@ -394,6 +407,7 @@ def CreatePEP8Style():
       SPACE_INSIDE_BRACKETS=False,
       SPACES_AROUND_POWER_OPERATOR=False,
       SPACES_AROUND_DEFAULT_OR_NAMED_ASSIGN=False,
+      SPACES_AROUND_SUBSCRIPT_COLON=False,
       SPACES_BEFORE_COMMENT=2,
       SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=False,
       SPLIT_ALL_COMMA_SEPARATED_VALUES=False,
@@ -566,6 +580,7 @@ _STYLE_OPTION_VALUE_CONVERTER = dict(
     INDENT_CLOSING_BRACKETS=_BoolConverter,
     DISABLE_ENDING_COMMA_HEURISTIC=_BoolConverter,
     EACH_DICT_ENTRY_ON_SEPARATE_LINE=_BoolConverter,
+    FORCE_MULTILINE_DICT=_BoolConverter,
     I18N_COMMENT=str,
     I18N_FUNCTION_CALL=_StringListConverter,
     INDENT_DICTIONARY_VALUE=_BoolConverter,
@@ -577,6 +592,7 @@ _STYLE_OPTION_VALUE_CONVERTER = dict(
     SPACE_INSIDE_BRACKETS=_BoolConverter,
     SPACES_AROUND_POWER_OPERATOR=_BoolConverter,
     SPACES_AROUND_DEFAULT_OR_NAMED_ASSIGN=_BoolConverter,
+    SPACES_AROUND_SUBSCRIPT_COLON=_BoolConverter,
     SPACES_BEFORE_COMMENT=_IntOrIntListConverter,
     SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=_BoolConverter,
     SPLIT_ALL_COMMA_SEPARATED_VALUES=_BoolConverter,
diff --git a/yapf/yapflib/unwrapped_line.py b/yapf/yapflib/unwrapped_line.py
index e4cb428..ec28f9b 100644
--- a/yapf/yapflib/unwrapped_line.py
+++ b/yapf/yapflib/unwrapped_line.py
@@ -261,6 +261,10 @@ def _PriorityIndicatingNoSpace(tok):
   return _HasPrecedence(tok)
 
 
+def _IsSubscriptColonAndValuePair(token1, token2):
+  return (token1.is_number or token1.is_name) and token2.is_subscript_colon
+
+
 def _SpaceRequiredBetween(left, right):
   """Return True if a space is required between the left and right token."""
   lval = left.value
@@ -293,6 +297,11 @@ def _SpaceRequiredBetween(left, right):
       return True
     if rval in pytree_utils.CLOSING_BRACKETS and lval == ':':
       return True
+  if (style.Get('SPACES_AROUND_SUBSCRIPT_COLON') and
+      (_IsSubscriptColonAndValuePair(left, right) or
+       _IsSubscriptColonAndValuePair(right, left))):
+    # Supersede the "never want a space before a colon or comma" check.
+    return True
   if rval in ':,':
     # Otherwise, we never want a space before a colon or comma.
     return False


Test patch:
diff --git a/yapftests/file_resources_test.py b/yapftests/file_resources_test.py
index 07e3134..c91d93c 100644
--- a/yapftests/file_resources_test.py
+++ b/yapftests/file_resources_test.py
@@ -439,5 +439,33 @@ class WriteReformattedCodeTest(unittest.TestCase):
     self.assertEqual(stream.getvalue(), s)
 
 
+class LineEndingTest(unittest.TestCase):
+
+  def test_line_ending_linefeed(self):
+    lines = ['spam\n', 'spam\n']
+    actual = file_resources.LineEnding(lines)
+    self.assertEqual(actual, '\n')
+
+  def test_line_ending_carriage_return(self):
+    lines = ['spam\r', 'spam\r']
+    actual = file_resources.LineEnding(lines)
+    self.assertEqual(actual, '\r')
+
+  def test_line_ending_combo(self):
+    lines = ['spam\r\n', 'spam\r\n']
+    actual = file_resources.LineEnding(lines)
+    self.assertEqual(actual, '\r\n')
+
+  def test_line_ending_weighted(self):
+    lines = [
+        'spam\n',
+        'spam\n',
+        'spam\r',
+        'spam\r\n',
+    ]
+    actual = file_resources.LineEnding(lines)
+    self.assertEqual(actual, '\n')
+
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/yapftests/reformatter_basic_test.py b/yapftests/reformatter_basic_test.py
index 8e93042..7868d0b 100644
--- a/yapftests/reformatter_basic_test.py
+++ b/yapftests/reformatter_basic_test.py
@@ -1937,8 +1937,8 @@ class A(object):
 
   def testDontAddBlankLineAfterMultilineString(self):
     code = textwrap.dedent("""\
-      query = '''SELECT id 
-      FROM table 
+      query = '''SELECT id
+      FROM table
       WHERE day in {}'''
       days = ",".join(days)
       """)
@@ -2954,6 +2954,39 @@ my_dict = {
     uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
     self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
 
+  def testForceMultilineDict_True(self):
+    try:
+      style.SetGlobalStyle(
+          style.CreateStyleFromConfig('{force_multiline_dict: true}'))
+      unformatted_code = textwrap.dedent(
+          "responseDict = {'childDict': {'spam': 'eggs'}}\n")
+      uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+      actual = reformatter.Reformat(uwlines)
+      expected = textwrap.dedent("""\
+        responseDict = {
+            'childDict': {
+                'spam': 'eggs'
+            }
+        }
+      """)
+      self.assertCodeEqual(expected, actual)
+    finally:
+      style.SetGlobalStyle(style.CreateChromiumStyle())
+
+  def testForceMultilineDict_False(self):
+    try:
+      style.SetGlobalStyle(
+          style.CreateStyleFromConfig('{force_multiline_dict: false}'))
+      unformatted_code = textwrap.dedent("""\
+        responseDict = {'childDict': {'spam': 'eggs'}}
+      """)
+      expected_formatted_code = unformatted_code
+      uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+      self.assertCodeEqual(expected_formatted_code,
+                           reformatter.Reformat(uwlines))
+    finally:
+      style.SetGlobalStyle(style.CreateChromiumStyle())
+
 
 if __name__ == '__main__':
   unittest.main()
diff --git a/yapftests/reformatter_pep8_test.py b/yapftests/reformatter_pep8_test.py
index 8f336e1..bdd074a 100644
--- a/yapftests/reformatter_pep8_test.py
+++ b/yapftests/reformatter_pep8_test.py
@@ -843,5 +843,77 @@ class TestsForSpacesInsideBrackets(yapf_test_helper.YAPFTest):
     self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
 
 
+class TestsForSpacesAroundSubscriptColon(yapf_test_helper.YAPFTest):
+  """Test the SPACES_AROUND_SUBSCRIPT_COLON style option."""
+  unformatted_code = textwrap.dedent("""\
+    a = list1[ : ]
+    b = list2[ slice_start: ]
+    c = list3[ slice_start:slice_end ]
+    d = list4[ slice_start:slice_end: ]
+    e = list5[ slice_start:slice_end:slice_step ]
+    a1 = list1[ : ]
+    b1 = list2[ 1: ]
+    c1 = list3[ 1:20 ]
+    d1 = list4[ 1:20: ]
+    e1 = list5[ 1:20:3 ]
+  """)
+
+  def testEnabled(self):
+    style.SetGlobalStyle(
+        style.CreateStyleFromConfig('{spaces_around_subscript_colon: True}'))
+    expected_formatted_code = textwrap.dedent("""\
+      a = list1[:]
+      b = list2[slice_start :]
+      c = list3[slice_start : slice_end]
+      d = list4[slice_start : slice_end :]
+      e = list5[slice_start : slice_end : slice_step]
+      a1 = list1[:]
+      b1 = list2[1 :]
+      c1 = list3[1 : 20]
+      d1 = list4[1 : 20 :]
+      e1 = list5[1 : 20 : 3]
+    """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(self.unformatted_code)
+    self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
+  def testWithSpaceInsideBrackets(self):
+    style.SetGlobalStyle(
+        style.CreateStyleFromConfig('{'
+                                    'spaces_around_subscript_colon: true, '
+                                    'space_inside_brackets: true,'
+                                    '}'))
+    expected_formatted_code = textwrap.dedent("""\
+      a = list1[ : ]
+      b = list2[ slice_start : ]
+      c = list3[ slice_start : slice_end ]
+      d = list4[ slice_start : slice_end : ]
+      e = list5[ slice_start : slice_end : slice_step ]
+      a1 = list1[ : ]
+      b1 = list2[ 1 : ]
+      c1 = list3[ 1 : 20 ]
+      d1 = list4[ 1 : 20 : ]
+      e1 = list5[ 1 : 20 : 3 ]
+    """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(self.unformatted_code)
+    self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
+  def testDefault(self):
+    style.SetGlobalStyle(style.CreatePEP8Style())
+    expected_formatted_code = textwrap.dedent("""\
+      a = list1[:]
+      b = list2[slice_start:]
+      c = list3[slice_start:slice_end]
+      d = list4[slice_start:slice_end:]
+      e = list5[slice_start:slice_end:slice_step]
+      a1 = list1[:]
+      b1 = list2[1:]
+      c1 = list3[1:20]
+      d1 = list4[1:20:]
+      e1 = list5[1:20:3]
+    """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(self.unformatted_code)
+    self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
+
 if __name__ == '__main__':
   unittest.main()