clang-format: Refactor indentation behavior for multiple nested blocks.

This fixes a few oddities when formatting multiple nested JavaScript
blocks, e.g.:

Before:
  promise.then(
      function success() {
        doFoo();
        doBar();
      },
      [], function error() {
        doFoo();
        doBaz();
      });
  promise.then([],
               function success() {
                 doFoo();
                 doBar();
               },
               function error() {
    doFoo();
    doBaz();
  });

After:
  promise.then(
      function success() {
        doFoo();
        doBar();
      },
      [],
      function error() {
        doFoo();
        doBaz();
      });
  promise.then([],
               function success() {
                 doFoo();
                 doBar();
               },
               function error() {
                 doFoo();
                 doBaz();
               });

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@210097 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 81 insertions, 33 deletions

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 01467f995d..5e63d0cb9b 100644
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -112,6 +112,15 @@ bool ContinuationIndenter::canBreak(const LineState &State) {
     return false;
   if (Current.isMemberAccess() && State.Stack.back().ContainsUnwrappedBuilder)
     return false;
+
+  // Don't create a 'hanging' indent if there are multiple blocks in a single
+  // statement.
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+      Previous.is(tok::l_brace) && State.Stack.size() > 1 &&
+      State.Stack[State.Stack.size() - 2].JSFunctionInlined &&
+      State.Stack[State.Stack.size() - 2].HasMultipleNestedBlocks)
+    return false;
+
   return !State.Stack.back().NoLineBreak;
 }
 
@@ -294,7 +303,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
     // Treat the condition inside an if as if it was a second function
     // parameter, i.e. let nested calls have a continuation indent.
     State.Stack.back().LastSpace = State.Column;
-  else if (Current.isNot(tok::comment) &&
+  else if (!Current.isOneOf(tok::comment, tok::caret) &&
            (Previous.is(tok::comma) ||
             (Previous.is(tok::colon) && Previous.Type == TT_ObjCMethodExpr)))
     State.Stack.back().LastSpace = State.Column;
@@ -589,8 +598,6 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
         Current.LastOperator ? 0 : State.Column + Current.ColumnWidth;
   if (Current.Type == TT_ObjCSelectorName)
     State.Stack.back().ObjCSelectorNameFound = true;
-  if (Current.Type == TT_LambdaLSquare)
-    ++State.Stack.back().LambdasFound;
   if (Current.Type == TT_CtorInitializerColon) {
     // Indent 2 from the column, so:
     // SomeClass::SomeClass()
@@ -767,24 +774,24 @@ static void consumeRParens(LineState& State, const FormatToken &Tok) {
 //   SomeFunction(a, [] {
 //                     f();  // break
 //                   });
-static bool fakeRParenSpecialCase(const FormatToken& Tok) {
+static bool fakeRParenSpecialCase(const LineState &State) {
+  const FormatToken &Tok = *State.NextToken;
   if (!Tok.MatchingParen)
     return false;
   const FormatToken *Left = &Tok;
   if (Tok.isOneOf(tok::r_brace, tok::r_square))
     Left = Tok.MatchingParen;
-  return Left->isOneOf(tok::l_brace, tok::l_square) &&
+  return !State.Stack.back().HasMultipleNestedBlocks &&
+         Left->isOneOf(tok::l_brace, tok::l_square) &&
          (Left->BlockKind == BK_Block ||
           Left->Type == TT_ArrayInitializerLSquare ||
           Left->Type == TT_DictLiteral);
 }
 
 void ContinuationIndenter::moveStatePastFakeRParens(LineState &State) {
-  const FormatToken &Current = *State.NextToken;
-
   // Don't remove FakeRParens attached to r_braces that surround nested blocks
   // as they will have been removed early (see above).
-  if (fakeRParenSpecialCase(Current))
+  if (fakeRParenSpecialCase(State))
     return;
 
   consumeRParens(State, *State.NextToken);
@@ -806,7 +813,7 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
   bool AvoidBinPacking;
   bool BreakBeforeParameter = false;
   if (Current.is(tok::l_brace) || Current.Type == TT_ArrayInitializerLSquare) {
-    if (fakeRParenSpecialCase(Current))
+    if (fakeRParenSpecialCase(State))
       consumeRParens(State, *Current.MatchingParen);
 
     NewIndent = State.Stack.back().LastSpace;
@@ -848,6 +855,7 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
                                    State.Stack.back().LastSpace,
                                    AvoidBinPacking, NoLineBreak));
   State.Stack.back().BreakBeforeParameter = BreakBeforeParameter;
+  State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 1;
 }
 
 void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
@@ -874,10 +882,7 @@ void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
 void ContinuationIndenter::moveStateToNewBlock(LineState &State) {
   // If we have already found more than one lambda introducers on this level, we
   // opt out of this because similarity between the lambdas is more important.
-  // FIXME: This should use fakeRParenSpecialCase() and fakeRParenSpecialCase()
-  // Needs to include the LambdasFound check. Otherwise the corresponding
-  // fake r_parens will never be consumed.
-  if (State.Stack.back().LambdasFound <= 1)
+  if (fakeRParenSpecialCase(State))
     consumeRParens(State, *State.NextToken->MatchingParen);
 
   // For some reason, ObjC blocks are indented like continuations.
diff --git a/lib/Format/ContinuationIndenter.h b/lib/Format/ContinuationIndenter.h
index 4733bda0a0..0969a8cec9 100644
--- a/lib/Format/ContinuationIndenter.h
+++ b/lib/Format/ContinuationIndenter.h
@@ -151,8 +151,8 @@ struct ParenState {
         StartOfFunctionCall(0), StartOfArraySubscripts(0),
         NestedNameSpecifierContinuation(0), CallContinuation(0), VariablePos(0),
         ContainsLineBreak(false), ContainsUnwrappedBuilder(0),
-        AlignColons(true), ObjCSelectorNameFound(false), LambdasFound(0),
-        JSFunctionInlined(false) {}
+        AlignColons(true), ObjCSelectorNameFound(false),
+        HasMultipleNestedBlocks(false), JSFunctionInlined(false) {}
 
   /// \brief The position to which a specific parenthesis level needs to be
   /// indented.
@@ -247,11 +247,11 @@ struct ParenState {
   /// the same token.
   bool ObjCSelectorNameFound;
 
-  /// \brief Counts the number of lambda introducers found on this level.
+  /// \brief \c true if there are multiple nested blocks inside these parens.
   ///
   /// Not considered for memoization as it will always have the same value at
   /// the same token.
-  unsigned LambdasFound;
+  bool HasMultipleNestedBlocks;
 
   // \brief The previous JavaScript 'function' keyword is not wrapped to a new
   // line.
diff --git a/lib/Format/FormatToken.h b/lib/Format/FormatToken.h
index f2bba324e6..90208b6065 100644
--- a/lib/Format/FormatToken.h
+++ b/lib/Format/FormatToken.h
@@ -103,9 +103,10 @@ struct FormatToken {
         IsFirst(false), MustBreakBefore(false), IsUnterminatedLiteral(false),
         BlockKind(BK_Unknown), Type(TT_Unknown), SpacesRequiredBefore(0),
         CanBreakBefore(false), ClosesTemplateDeclaration(false),
-        ParameterCount(0), PackingKind(PPK_Inconclusive), TotalLength(0),
-        UnbreakableTailLength(0), BindingStrength(0), NestingLevel(0),
-        SplitPenalty(0), LongestObjCSelectorName(0), FakeRParens(0),
+        ParameterCount(0), BlockParameterCount(0),
+        PackingKind(PPK_Inconclusive), TotalLength(0), UnbreakableTailLength(0),
+        BindingStrength(0), NestingLevel(0), SplitPenalty(0),
+        LongestObjCSelectorName(0), FakeRParens(0),
         StartsBinaryExpression(false), EndsBinaryExpression(false),
         OperatorIndex(0), LastOperator(false),
         PartOfMultiVariableDeclStmt(false), IsForEachMacro(false),
@@ -191,6 +192,10 @@ struct FormatToken {
   /// the number of commas.
   unsigned ParameterCount;
 
+  /// \brief Number of parameters that are nested blocks,
+  /// if this is "(", "[" or "<".
+  unsigned BlockParameterCount;
+
   /// \brief A token can have a special role that can carry extra information
   /// about the token's formatting.
   std::unique_ptr<TokenRole> Role;
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 849dfb02f8..cbb9d7f964 100644
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -198,7 +198,6 @@ private:
         return false;
       else if (CurrentToken->is(tok::l_brace))
         Left->Type = TT_Unknown; // Not TT_ObjCBlockLParen
-      updateParameterCount(Left, CurrentToken);
       if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
           !CurrentToken->Next->HasUnescapedNewline &&
           !CurrentToken->Next->isTrailingComment())
@@ -206,8 +205,10 @@ private:
       if (CurrentToken->isOneOf(tok::kw_const, tok::kw_auto) ||
           CurrentToken->isSimpleTypeSpecifier())
         Contexts.back().IsExpression = false;
+      FormatToken *Tok = CurrentToken;
       if (!consumeToken())
         return false;
+      updateParameterCount(Left, Tok);
       if (CurrentToken && CurrentToken->HasUnescapedNewline)
         HasMultipleLines = true;
     }
@@ -266,7 +267,7 @@ private:
         if (Contexts.back().FirstObjCSelectorName) {
           Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
               Contexts.back().LongestObjCSelectorName;
-          if (Contexts.back().NumBlockParameters > 1)
+          if (Left->BlockParameterCount > 1)
             Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
         }
         next();
@@ -281,9 +282,10 @@ private:
           (Left->Type == TT_ArraySubscriptLSquare ||
            (Left->Type == TT_ObjCMethodExpr && !ColonFound)))
         Left->Type = TT_ArrayInitializerLSquare;
-      updateParameterCount(Left, CurrentToken);
+      FormatToken* Tok = CurrentToken;
       if (!consumeToken())
         return false;
+      updateParameterCount(Left, Tok);
     }
     return false;
   }
@@ -324,6 +326,12 @@ private:
   }
 
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+    if (Current->Type == TT_LambdaLSquare ||
+        (Current->is(tok::caret) && Current->Type == TT_UnaryOperator) ||
+        (Style.Language == FormatStyle::LK_JavaScript &&
+         Current->TokenText == "function")) {
+      ++Left->BlockParameterCount;
+    }
     if (Current->is(tok::comma)) {
       ++Left->ParameterCount;
       if (!Left->Role)
@@ -639,17 +647,16 @@ private:
     Context(tok::TokenKind ContextKind, unsigned BindingStrength,
             bool IsExpression)
         : ContextKind(ContextKind), BindingStrength(BindingStrength),
-          LongestObjCSelectorName(0), NumBlockParameters(0),
-          ColonIsForRangeExpr(false), ColonIsDictLiteral(false),
-          ColonIsObjCMethodExpr(false), FirstObjCSelectorName(nullptr),
-          FirstStartOfName(nullptr), IsExpression(IsExpression),
-          CanBeExpression(true), InTemplateArgument(false),
-          InCtorInitializer(false), CaretFound(false), IsForEachMacro(false) {}
+          LongestObjCSelectorName(0), ColonIsForRangeExpr(false),
+          ColonIsDictLiteral(false), ColonIsObjCMethodExpr(false),
+          FirstObjCSelectorName(nullptr), FirstStartOfName(nullptr),
+          IsExpression(IsExpression), CanBeExpression(true),
+          InTemplateArgument(false), InCtorInitializer(false),
+          CaretFound(false), IsForEachMacro(false) {}
 
     tok::TokenKind ContextKind;
     unsigned BindingStrength;
     unsigned LongestObjCSelectorName;
-    unsigned NumBlockParameters;
     bool ColonIsForRangeExpr;
     bool ColonIsDictLiteral;
     bool ColonIsObjCMethodExpr;
@@ -742,10 +749,8 @@ private:
                                   Contexts.back().InTemplateArgument);
       } else if (Current.isOneOf(tok::minus, tok::plus, tok::caret)) {
         Current.Type = determinePlusMinusCaretUsage(Current);
-        if (Current.Type == TT_UnaryOperator && Current.is(tok::caret)) {
-          ++Contexts.back().NumBlockParameters;
+        if (Current.Type == TT_UnaryOperator && Current.is(tok::caret))
           Contexts.back().CaretFound = true;
-        }
       } else if (Current.isOneOf(tok::minusminus, tok::plusplus)) {
         Current.Type = determineIncrementUsage(Current);
       } else if (Current.is(tok::exclaim)) {
diff --git a/unittests/Format/FormatTestJS.cpp b/unittests/Format/FormatTestJS.cpp
index ecf4e69999..ddd594bb6e 100644
--- a/unittests/Format/FormatTestJS.cpp
+++ b/unittests/Format/FormatTestJS.cpp
@@ -146,6 +146,39 @@ TEST_F(FormatTestJS, Closures) {
                getGoogleJSStyleWithColumns(37));
 }
 
+TEST_F(FormatTestJS, MultipleFunctionLiterals) {
+  verifyFormat("promise.then(\n"
+               "    function success() {\n"
+               "      doFoo();\n"
+               "      doBar();\n"
+               "    },\n"
+               "    function error() {\n"
+               "      doFoo();\n"
+               "      doBaz();\n"
+               "    },\n"
+               "    []);\n");
+  verifyFormat("promise.then(\n"
+               "    function success() {\n"
+               "      doFoo();\n"
+               "      doBar();\n"
+               "    },\n"
+               "    [],\n"
+               "    function error() {\n"
+               "      doFoo();\n"
+               "      doBaz();\n"
+               "    });\n");
+  // FIXME: Here, we should probably break right after the "(" for consistency.
+  verifyFormat("promise.then([],\n"
+               "             function success() {\n"
+               "               doFoo();\n"
+               "               doBar();\n"
+               "             },\n"
+               "             function error() {\n"
+               "               doFoo();\n"
+               "               doBaz();\n"
+               "             });\n");
+}
+
 TEST_F(FormatTestJS, ReturnStatements) {
   verifyFormat("function() { return [hello, world]; }");
 }