Ensure that parameter lists don't collide with following code

A parameter list shouldn't align with the code following it. Make sure
that it's properly indented when this is the case.

Closes google#785
Closes google#784
Closes google#781

CHANGELOG

@@ -8,6 +8,8 @@
 - Don't require splitting before comments in a list when
   `SPLIT_ALL_TOP_LEVEL_COMMA_SEPARATED_VALUES` is set. The knob is meant for
   values, not comments, which may be associated with the current line.
+- 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.
 
 ## [0.29.0] 2019-11-28
 ### Added

yapf/yapflib/format_decision_state.py

@@ -661,8 +661,8 @@ def _AddTokenOnNewline(self, dry_run, must_split):
          previous.previous_token.OpensScope())):
       dedent = (style.Get('CONTINUATION_INDENT_WIDTH'),
                 0)[style.Get('INDENT_CLOSING_BRACKETS')]
-      self.stack[-1].closing_scope_indent = max(0,
-                                                self.stack[-1].indent - dedent)
+      self.stack[-1].closing_scope_indent = (
+          max(0, self.stack[-1].indent - dedent))
       self.stack[-1].split_before_closing_bracket = True
 
     # Calculate the split penalty.
@@ -952,27 +952,25 @@ def _GetNewlineColumn(self):
         if format_token.Subtype.DICTIONARY_VALUE in current.subtypes:
           return top_of_stack.indent
 
-    if (_IsCompoundStatement(self.line.first) and
+    if (not self.param_list_stack and _IsCompoundStatement(self.line.first) and
         (not (style.Get('DEDENT_CLOSING_BRACKETS') or
               style.Get('INDENT_CLOSING_BRACKETS')) or
          style.Get('SPLIT_BEFORE_FIRST_ARGUMENT'))):
       token_indent = (
           len(self.line.first.whitespace_prefix.split('\n')[-1]) +
           style.Get('INDENT_WIDTH'))
       if token_indent == top_of_stack.indent:
-        if self.param_list_stack and _IsFunctionDef(self.line.first):
-          last_param = self.param_list_stack[-1]
-          if (last_param.LastParamFitsOnLine(token_indent) and
-              not last_param.LastParamFitsOnLine(
-                  token_indent + style.Get('CONTINUATION_INDENT_WIDTH'))):
-            self.param_list_stack[-1].split_before_closing_bracket = True
-            return token_indent
-
-          if not last_param.LastParamFitsOnLine(token_indent):
-            self.param_list_stack[-1].split_before_closing_bracket = True
-            return token_indent
         return token_indent + style.Get('CONTINUATION_INDENT_WIDTH')
 
+    if (self.param_list_stack and
+        not self.param_list_stack[-1].SplitBeforeClosingBracket(
+            top_of_stack.indent) and top_of_stack.indent == (
+                (self.line.depth + 1) * style.Get('INDENT_WIDTH'))):
+      if (format_token.Subtype.PARAMETER_START in current.subtypes or
+          (previous.is_comment and
+           format_token.Subtype.PARAMETER_START in previous.subtypes)):
+        return top_of_stack.indent + style.Get('CONTINUATION_INDENT_WIDTH')
+
     return top_of_stack.indent
 
   def _FitsOnLine(self, start, end):

yapf/yapflib/object_state.py

@@ -120,21 +120,46 @@ def has_typed_return(self):
   def has_default_values(self):
     return any(param.has_default_value for param in self.parameters)
 
+  @property
+  @py3compat.lru_cache()
+  def ends_in_comma(self):
+    if not self.parameters:
+      return False
+    return self.parameters[-1].last_token.next_token.value == ','
+
+  @property
+  @py3compat.lru_cache()
+  def last_token(self):
+    token = self.opening_bracket.matching_bracket
+    while not token.is_comment and token.next_token:
+      token = token.next_token
+    return token
+
   @py3compat.lru_cache()
   def LastParamFitsOnLine(self, indent):
     """Return true if the last parameter fits on a single line."""
     if not self.has_typed_return:
       return False
     if not self.parameters:
       return True
+    total_length = self.last_token.total_length
     last_param = self.parameters[-1].first_token
-    last_token = self.opening_bracket.matching_bracket
-    while not last_token.is_comment and last_token.next_token:
-      last_token = last_token.next_token
-    total_length = last_token.total_length
     total_length -= last_param.total_length - len(last_param.value)
     return total_length + indent <= style.Get('COLUMN_LIMIT')
 
+  @py3compat.lru_cache()
+  def SplitBeforeClosingBracket(self, indent):
+    if style.Get('DEDENT_CLOSING_BRACKETS'):
+      return True
+    if self.ends_in_comma:
+      return True
+    if not self.parameters:
+      return False
+    total_length = self.last_token.total_length
+    last_param = self.parameters[-1].first_token
+    total_length -= last_param.total_length - len(last_param.value)
+    return total_length + indent > style.Get('COLUMN_LIMIT')
+
   def Clone(self):
     clone = ParameterListState(self.opening_bracket,
                                self.has_split_before_first_param,

yapftests/reformatter_pep8_test.py

@@ -589,6 +589,60 @@ class nested_class():
     finally:
       style.SetGlobalStyle(style.CreatePEP8Style())
 
+  def testParamListIndentationCollision1(self):
+    unformatted_code = textwrap.dedent("""\
+class _():
+
+    def __init__(self, title: Optional[str], diffs: Collection[BinaryDiff] = (), charset: Union[Type[AsciiCharset], Type[LineCharset]] = AsciiCharset, preprocess: Callable[[str], str] = identity,
+            # TODO(somebody): Make this a Literal type.
+            justify: str = 'rjust'):
+        self._cs = charset
+        self._preprocess = preprocess
+        """)
+    expected_formatted_code = textwrap.dedent("""\
+class _():
+    def __init__(
+            self,
+            title: Optional[str],
+            diffs: Collection[BinaryDiff] = (),
+            charset: Union[Type[AsciiCharset],
+                           Type[LineCharset]] = AsciiCharset,
+            preprocess: Callable[[str], str] = identity,
+            # TODO(somebody): Make this a Literal type.
+            justify: str = 'rjust'):
+        self._cs = charset
+        self._preprocess = preprocess
+        """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+    self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
+  def testParamListIndentationCollision2(self):
+    code = textwrap.dedent("""\
+        def simple_pass_function_with_an_extremely_long_name_and_some_arguments(
+                argument0, argument1):
+            pass
+        """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(code)
+    self.assertCodeEqual(code, reformatter.Reformat(uwlines))
+
+  def testParamListIndentationCollision3(self):
+    code = textwrap.dedent("""\
+        def func1(
+            arg1,
+            arg2,
+        ) -> None:
+            pass
+
+
+        def func2(
+            arg1,
+            arg2,
+        ):
+            pass
+        """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(code)
+    self.assertCodeEqual(code, reformatter.Reformat(uwlines))
+
 
 if __name__ == '__main__':
   unittest.main()

yapftests/reformatter_python3_test.py

@@ -440,6 +440,25 @@ async def fn():
     uwlines = yapf_test_helper.ParseAndUnwrap(code)
     self.assertCodeEqual(code, reformatter.Reformat(uwlines))
 
+  def testParameterListIndentationConflicts(self):
+    unformatted_code = textwrap.dedent("""\
+        def raw_message(  # pylint: disable=too-many-arguments
+                    self, text, user_id=1000, chat_type='private', forward_date=None, forward_from=None):
+                pass
+        """)
+    expected_formatted_code = textwrap.dedent("""\
+        def raw_message(  # pylint: disable=too-many-arguments
+                self,
+                text,
+                user_id=1000,
+                chat_type='private',
+                forward_date=None,
+                forward_from=None):
+            pass
+        """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
+    self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(uwlines))
+
 
 if __name__ == '__main__':
   unittest.main()