Repository: google/yapf
Pull request #589 (2018-07-23 09:10:16)

py3compat: remove any BOM
This is a redo of #384 as it got lost...

----

Currently yapf fails on files containing the infamous BOM. Those two commits are an attempt to test and fix that issue.

This was the easiest way I found to test it but it's kind of agressive. I'm keen on knowing a better way.

Cheers,

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

Coverage decreased (-0.02%) to 95.568% when pulling **fd535ec6f88d99b5e6764d899d09147ae131d33e on greut:bom** into **2c13f6f93e8ccfa0c966b67f88eeffe20ccb32a0 on google:master**.

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

Coverage decreased (-0.02%) to 95.568% when pulling **fd535ec6f88d99b5e6764d899d09147ae131d33e on greut:bom** into **2c13f6f93e8ccfa0c966b67f88eeffe20ccb32a0 on google:master**.


Patch:
diff --git a/CHANGELOG b/CHANGELOG
index c41315c..40afca9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,12 +2,6 @@
 # All notable changes to this project will be documented in this file.
 # This project adheres to [Semantic Versioning](http://semver.org/).
 
-## [0.24.0] UNRELEASED
-### Added
-- Added 'SPLIT_BEFORE_DOT' knob to support "builder style" calls. The "builder
-  style" option didn't work as advertised. Lines would split after the dots,
-  not before them regardless of the penalties.
-
 ## [0.23.0] 2018-08-27
 ### Added
 - `DISABLE_ENDING_COMMA_HEURISTIC` is a new knob to disable the heuristic which
diff --git a/README.rst b/README.rst
index b5d22d9..ddd3ffe 100644
--- a/README.rst
+++ b/README.rst
@@ -503,20 +503,6 @@ Knobs
             for variable in bar if variable != 42
         }
 
-``SPLIT_BEFORE_DOT``
-    Split before the '.' if we need to split a longer expression:
-
-    .. code-block:: python
-
-      foo = ('This is a really long string: {}, {}, {}, {}'.format(a, b, c, d))
-
-    would reformat to something like:
-
-    .. code-block:: python
-
-      foo = ('This is a really long string: {}, {}, {}, {}'
-             .format(a, b, c, d))
-
 ``SPLIT_BEFORE_EXPRESSION_AFTER_OPENING_PAREN``
     Split after the opening paren which surrounds an expression if it doesn't
     fit on a single line.
diff --git a/yapf/__init__.py b/yapf/__init__.py
index 5d8a27c..6a3f3ed 100644
--- a/yapf/__init__.py
+++ b/yapf/__init__.py
@@ -176,6 +176,7 @@ def main(argv):
       style_config = file_resources.GetDefaultStyleForDir(os.getcwd())
 
     source = [line.rstrip() for line in original_source]
+    source[0] = py3compat.removeBOM(source[0])
     reformatted_source, _ = yapf_api.FormatCode(
         py3compat.unicode('\n'.join(source) + '\n'),
         filename='<stdin>',
diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py
index d61ea82..bff8ea3 100644
--- a/yapf/yapflib/format_decision_state.py
+++ b/yapf/yapflib/format_decision_state.py
@@ -336,8 +336,8 @@ class FormatDecisionState(object):
     ###########################################################################
     # Argument List Splitting
     if (style.Get('SPLIT_BEFORE_NAMED_ASSIGNS') and not current.is_comment and
-        format_token.Subtype.DEFAULT_OR_NAMED_ASSIGN_ARG_LIST in current
-        .subtypes):
+        format_token.Subtype.DEFAULT_OR_NAMED_ASSIGN_ARG_LIST in
+        current.subtypes):
       if (previous.value not in {'=', ':', '*', '**'} and
           current.value not in ':=,)' and not _IsFunctionDefinition(previous)):
         # If we're going to split the lines because of named arguments, then we
diff --git a/yapf/yapflib/py3compat.py b/yapf/yapflib/py3compat.py
index c66d6c6..5373b01 100644
--- a/yapf/yapflib/py3compat.py
+++ b/yapf/yapflib/py3compat.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 """Utilities for Python2 / Python3 compatibility."""
 
+import codecs
 import io
 import os
 import sys
@@ -116,3 +117,13 @@ class ConfigParser(configparser.ConfigParser):
 
     def read_file(self, fp, source=None):
       self.readfp(fp, filename=source)
+
+
+def removeBOM(source):
+  """Remove any Byte-order-Mark bytes from the beginning of a file."""
+  bom = codecs.BOM_UTF8
+  if PY3:
+    bom = bom.decode('utf-8')
+  if source.startswith(bom):
+    return source[len(bom):]
+  return source
diff --git a/yapf/yapflib/split_penalty.py b/yapf/yapflib/split_penalty.py
index c6ebb0b..416eda3 100644
--- a/yapf/yapflib/split_penalty.py
+++ b/yapf/yapflib/split_penalty.py
@@ -193,11 +193,8 @@ class _SplitPenaltyAssigner(pytree_visitor.PyTreeVisitor):
   def Visit_trailer(self, node):  # pylint: disable=invalid-name
     # trailer ::= '(' [arglist] ')' | '[' subscriptlist ']' | '.' NAME
     if node.children[0].value == '.':
-      before = style.Get('SPLIT_BEFORE_DOT')
-      _SetSplitPenalty(node.children[0], STRONGLY_CONNECTED
-                       if before else DOTTED_NAME)
-      _SetSplitPenalty(node.children[1], DOTTED_NAME
-                       if before else STRONGLY_CONNECTED)
+      self._SetUnbreakableOnChildren(node)
+      _SetSplitPenalty(node.children[1], DOTTED_NAME)
     elif len(node.children) == 2:
       # Don't split an empty argument list if at all possible.
       _SetSplitPenalty(node.children[1], VERY_STRONGLY_CONNECTED)
@@ -256,9 +253,7 @@ class _SplitPenaltyAssigner(pytree_visitor.PyTreeVisitor):
         pytree_utils.NodeName(node.children[1]) == 'trailer'):
       # children[1] itself is a whole trailer: we don't want to
       # mark all of it as unbreakable, only its first token: (, [ or .
-      first = pytree_utils.FirstLeafNode(node.children[1])
-      if first.value != '.':
-        _SetUnbreakable(node.children[1].children[0])
+      _SetUnbreakable(node.children[1].children[0])
 
       # A special case when there are more trailers in the sequence. Given:
       #   atom tr1 tr2
@@ -315,6 +310,10 @@ class _SplitPenaltyAssigner(pytree_visitor.PyTreeVisitor):
           # split the two.
           _SetStronglyConnected(trailer.children[-1])
 
+    # If the original source has a "builder" style calls, then we should allow
+    # the reformatter to retain that.
+    _AllowBuilderStyleCalls(node)
+
   def Visit_subscript(self, node):  # pylint: disable=invalid-name
     # subscript ::= test | [test] ':' [test] [sliceop]
     _SetStronglyConnected(*node.children)
@@ -591,3 +590,23 @@ def _DecrementSplitPenalty(node, amt):
 def _SetSplitPenalty(node, penalty):
   pytree_utils.SetNodeAnnotation(node, pytree_utils.Annotation.SPLIT_PENALTY,
                                  penalty)
+
+
+def _AllowBuilderStyleCalls(node):
+  """Allow splitting before '.' if it's a builder style function call."""
+
+  def RecGetLeaves(node):
+    if isinstance(node, pytree.Leaf):
+      return [node]
+    children = []
+    for child in node.children:
+      children += RecGetLeaves(child)
+    return children
+
+  list_of_children = RecGetLeaves(node)
+  prev_child = None
+  for child in list_of_children:
+    if child.value == '.':
+      if prev_child.lineno != child.lineno:
+        _SetSplitPenalty(child, 0)
+    prev_child = child
diff --git a/yapf/yapflib/style.py b/yapf/yapflib/style.py
index e4725df..b9e8b69 100644
--- a/yapf/yapflib/style.py
+++ b/yapf/yapflib/style.py
@@ -194,16 +194,6 @@ _STYLE_HELP = dict(
             variable: 'Hello world, have a nice day!'
             for variable in bar if variable != 42
         }"""),
-    SPLIT_BEFORE_DOT=textwrap.dedent("""\
-      Split before the '.' if we need to split a longer expression:
-
-        foo = ('This is a really long string: {}, {}, {}, {}'.format(a, b, c, d))
-
-      would reformat to something like:
-
-        foo = ('This is a really long string: {}, {}, {}, {}'
-               .format(a, b, c, d))
-      """),
     SPLIT_BEFORE_EXPRESSION_AFTER_OPENING_PAREN=textwrap.dedent("""\
       Split after the opening paren which surrounds an expression if it doesn't
       fit on a single line.
@@ -302,7 +292,6 @@ def CreatePEP8Style():
       SPLIT_BEFORE_BITWISE_OPERATOR=True,
       SPLIT_BEFORE_CLOSING_BRACKET=True,
       SPLIT_BEFORE_DICT_SET_GENERATOR=True,
-      SPLIT_BEFORE_DOT=False,
       SPLIT_BEFORE_EXPRESSION_AFTER_OPENING_PAREN=False,
       SPLIT_BEFORE_FIRST_ARGUMENT=False,
       SPLIT_BEFORE_LOGICAL_OPERATOR=True,
@@ -345,7 +334,6 @@ def CreateChromiumStyle():
   style['INDENT_WIDTH'] = 2
   style['JOIN_MULTIPLE_LINES'] = False
   style['SPLIT_BEFORE_BITWISE_OPERATOR'] = True
-  style['SPLIT_BEFORE_DOT'] = True
   style['SPLIT_BEFORE_EXPRESSION_AFTER_OPENING_PAREN'] = True
   return style
 
@@ -459,7 +447,6 @@ _STYLE_OPTION_VALUE_CONVERTER = dict(
     SPLIT_BEFORE_BITWISE_OPERATOR=_BoolConverter,
     SPLIT_BEFORE_CLOSING_BRACKET=_BoolConverter,
     SPLIT_BEFORE_DICT_SET_GENERATOR=_BoolConverter,
-    SPLIT_BEFORE_DOT=_BoolConverter,
     SPLIT_BEFORE_EXPRESSION_AFTER_OPENING_PAREN=_BoolConverter,
     SPLIT_BEFORE_FIRST_ARGUMENT=_BoolConverter,
     SPLIT_BEFORE_LOGICAL_OPERATOR=_BoolConverter,
@@ -540,10 +527,10 @@ def _CreateConfigParserFromConfigString(config_string):
   config = py3compat.ConfigParser()
   config.add_section('style')
   for key, value, _ in re.findall(
-      r'([a-zA-Z0-9_]+)\s*[:=]\s*'
-      r'(?:'
-      r'((?P<quote>[\'"]).*?(?P=quote)|'
-      r'[a-zA-Z0-9_]+)'
+      r'([a-zA-Z0-9_]+)\s*[:=]\s*' +
+      r'(?:' +
+      r'((?P<quote>[\'"]).*?(?P=quote)|' +
+      r'[a-zA-Z0-9_]+)' +
       r')', config_string):  # yapf: disable
     config.set('style', key, value)
   return config
diff --git a/yapf/yapflib/unwrapped_line.py b/yapf/yapflib/unwrapped_line.py
index 0bf4fba..01b6620 100644
--- a/yapf/yapflib/unwrapped_line.py
+++ b/yapf/yapflib/unwrapped_line.py
@@ -418,6 +418,9 @@ def _CanBreakBefore(prev_token, cur_token):
   if prev_token.is_name and cval == '[':
     # Don't break in the middle of an array dereference.
     return False
+  if prev_token.is_name and cval == '.':
+    # Don't break before the '.' in a dotted name.
+    return False
   if cur_token.is_comment and prev_token.lineno == cur_token.lineno:
     # Don't break a comment at the end of the line.
     return False


Test patch:
diff --git a/yapftests/reformatter_buganizer_test.py b/yapftests/reformatter_buganizer_test.py
index 7712351..6a1c781 100644
--- a/yapftests/reformatter_buganizer_test.py
+++ b/yapftests/reformatter_buganizer_test.py
@@ -28,22 +28,6 @@ class BuganizerFixes(yapf_test_helper.YAPFTest):
   def setUpClass(cls):
     style.SetGlobalStyle(style.CreateChromiumStyle())
 
-  def test113210278(self):
-    unformatted_code = """\
-def _():
-  aaaaaaaaaaa = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.cccccccccccccccccccccccccccc(\
-eeeeeeeeeeeeeeeeeeeeeeeeee.fffffffffffffffffffffffffffffffffffffff.\
-ggggggggggggggggggggggggggggggggg.hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh())
-"""
-    expected_formatted_code = """\
-def _():
-  aaaaaaaaaaa = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.cccccccccccccccccccccccccccc(
-      eeeeeeeeeeeeeeeeeeeeeeeeee.fffffffffffffffffffffffffffffffffffffff
-      .ggggggggggggggggggggggggggggggggg.hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh())
-"""
-    uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
-    self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
-
   def testB77923341(self):
     code = """\
 def f():
@@ -861,8 +845,8 @@ class _():
         """)
     expected_formatted_code = textwrap.dedent("""\
         def lulz():
-          return (some_long_module_name.SomeLongClassName.some_long_attribute_name
-                  .some_long_method_name())
+          return (some_long_module_name.SomeLongClassName.some_long_attribute_name.
+                  some_long_method_name())
         """)
     uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
     self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
@@ -960,8 +944,8 @@ class _():
         def _():
           _xxxxxxxxxxxxxxx(
               aaaaaaaa,
-              bbbbbbbbbbbbbb.cccccccccc[dddddddddddddddddddddddddddd
-                                        .eeeeeeeeeeeeeeeeeeeeee.fffffffffffffffffffff])
+              bbbbbbbbbbbbbb.cccccccccc[dddddddddddddddddddddddddddd.
+                                        eeeeeeeeeeeeeeeeeeeeee.fffffffffffffffffffff])
         """)
     uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
     self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
@@ -1033,8 +1017,8 @@ class _():
     expected_formatted_code = textwrap.dedent("""\
         def _():
           aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = (
-              self.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
-              .cccccccccccccccccccccccccccccccccccc)
+              self.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.
+              cccccccccccccccccccccccccccccccccccc)
         """)
     uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
     self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
diff --git a/yapftests/split_penalty_test.py b/yapftests/split_penalty_test.py
index 0d300d8..7d500da 100644
--- a/yapftests/split_penalty_test.py
+++ b/yapftests/split_penalty_test.py
@@ -198,8 +198,8 @@ class SplitPenaltyTest(unittest.TestCase):
         ('foo', STRONGLY_CONNECTED),
         ('if', 0),
         ('a', STRONGLY_CONNECTED),
-        ('.', DOTTED_NAME),
-        ('x', STRONGLY_CONNECTED),
+        ('.', UNBREAKABLE),
+        ('x', DOTTED_NAME),
         ('==', STRONGLY_CONNECTED),
         ('37', STRONGLY_CONNECTED),
         (']', None),
@@ -224,10 +224,10 @@ class SplitPenaltyTest(unittest.TestCase):
     tree = self._ParseAndComputePenalties(code)
     self._CheckPenalties(tree, [
         ('foo', None),
-        ('.', DOTTED_NAME),
-        ('bar', STRONGLY_CONNECTED),
-        ('.', DOTTED_NAME),
-        ('baz', STRONGLY_CONNECTED),
+        ('.', UNBREAKABLE),
+        ('bar', DOTTED_NAME),
+        ('.', STRONGLY_CONNECTED),
+        ('baz', DOTTED_NAME),
         ('(', STRONGLY_CONNECTED),
         ('1', None),
         (',', UNBREAKABLE),
diff --git a/yapftests/yapf_test.py b/yapftests/yapf_test.py
index 68bc77a..6e64083 100644
--- a/yapftests/yapf_test.py
+++ b/yapftests/yapf_test.py
@@ -411,7 +411,8 @@ class CommandLineTest(unittest.TestCase):
         stdin=subprocess.PIPE,
         stderr=subprocess.PIPE,
         env=env)
-    reformatted_code, stderrdata = p.communicate(unformatted.encode('utf-8'))
+    reformatted_code, stderrdata = p.communicate(
+        unformatted.encode('utf-8-sig'))
     self.assertEqual(stderrdata, b'')
     self.assertMultiLineEqual(reformatted_code.decode('utf-8'), expected)
 
@@ -1363,7 +1364,7 @@ a = 4-b / c@d**37
         expected_formatted_code,
         extra_options=[
             '--style',
-            '{based_on_style: pep8, '
+            '{based_on_style: pep8, ' +
             'no_spaces_around_selected_binary_operators: "@,**,-"}',
         ])