[clang-format] Fix ObjC message arguments formatting.

[jolesiak]

Summary

Fixes formatting of ObjC message arguments when inline block is a first
argument.

Having inline block as a first when method has multiple parameters is
discouraged by Apple:
"It’s best practice to use only one block argument to a method. If the
method also needs other non-block arguments, the block should come last"
(https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html#//apple_ref/doc/uid/TP40011210-CH8-SW7),
it should be correctly formatted nevertheless.

Current formatting:

[object blockArgument:^{
  a = 42;
}
    anotherArg:42];

Fixed (colon alignment):

[object
  blockArgument:^{
    a = 42;
  }
     anotherArg:42];

Test Plan: make -j12 FormatTests && tools/clang/unittests/Format/FormatTests

[benhamilton]

lib/Format/TokenAnnotator.cpp

419 ↗

(On Diff #131294)

What does this line do? Seems like it's initialized to 0 already, right?

[jolesiak]

lib/Format/TokenAnnotator.cpp

419 ↗

(On Diff #131294)

It is indeed initialized to 0.
However, before 'Left' bracket is recognized as TT_ObjCMethodExpr it has a different type assigned. Hence it gets updated here:
https://github.com/llvm-mirror/clang/blob/release_60/lib/Format/TokenAnnotator.cpp#L495
This assignment is due to fact that for other languages number of parameters is calculated as (1 + number_of_commas).

[benhamilton]

lib/Format/TokenAnnotator.cpp

419 ↗

(On Diff #131294)

I see. Could you add a comment explaining that?

[jolesiak]

• Add comment explaining ParameterCount reset.

[benhamilton]

[krasimir]

unittests/Format/FormatTestObjC.cpp

678 ↗

(On Diff #131825)

Can you put a test with a non-block between two blocks please

[jolesiak]

• Add test

cfe/trunk/lib/Format/ContinuationIndenter.cpp

bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Previous = *Current.Previous;
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
     return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
       Current.closesBlockOrBlockTypeList(Style))
     return true;
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
     return true;
+  if (Style.Language == FormatStyle::LK_ObjC &&
+      Current.ObjCSelectorNameParts > 1 &&
+      Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
+    return true;
+  }
   if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
        (Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
         Style.isCpp() &&
         // FIXME: This is a temporary workaround for the case where clang-format
         // sets BreakBeforeParameter to avoid bin packing and this creates a
         // completely unnecessary line break after a template type that isn't
         // line-wrapped.
         (Previous.NestingLevel == 1 || Style.BinPackParameters)) ||

cfe/trunk/lib/Format/FormatToken.h

struct FormatToken {
 
   /// \brief If this is the first ObjC selector name in an ObjC method
   /// definition or call, this contains the length of the longest name.
   ///
   /// This being set to 0 means that the selectors should not be colon-aligned,
   /// e.g. because several of them are block-type.
   unsigned LongestObjCSelectorName = 0;
 
+  /// \brief How many parts ObjC selector have (i.e. how many parameters method
+  /// has).
+  unsigned ObjCSelectorNameParts = 0;
+
   /// \brief Stores the number of required fake parentheses and the
   /// corresponding operator precedence.
   ///
   /// If multiple fake parentheses start at a token, this vector stores them in
   /// reverse order, i.e. inner fake parenthesis first.
   SmallVector<prec::Level, 4> FakeLParens;
   /// \brief Insert this many fake ) after this token for correct indentation.
   unsigned FakeRParens = 0;

cfe/trunk/lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
           if (Parent && Parent->is(TT_PointerOrReference))
             Parent->Type = TT_BinaryOperator;
         }
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
         if (Contexts.back().FirstObjCSelectorName) {
           Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
               Contexts.back().LongestObjCSelectorName;
+          Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
+              Left->ParameterCount;
           if (Left->BlockParameterCount > 1)
             Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
         }
         next();
         return true;
       }
       if (CurrentToken->isOneOf(tok::r_paren, tok::r_brace))
         return false;
       if (CurrentToken->is(tok::colon)) {
         if (Left->isOneOf(TT_ArraySubscriptLSquare,
                           TT_DesignatedInitializerLSquare)) {
           Left->Type = TT_ObjCMethodExpr;
           StartsObjCMethodExpr = true;
+          // ParameterCount might have been set to 1 before expression was
+          // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
+          // used for other expression types). Parameter counter has to be,
+          // therefore, reset to 0.
+          Left->ParameterCount = 0;
           Contexts.back().ColonIsObjCMethodExpr = true;
           if (Parent && Parent->is(tok::r_paren))
             Parent->Type = TT_CastRParen;
         }
         ColonFound = true;
       }
       if (CurrentToken->is(tok::comma) && Left->is(TT_ObjCMethodExpr) &&
           !ColonFound)
if (CurrentToken) {
       }
     }
     return true;
   }
 
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
     if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
       ++Left->BlockParameterCount;
-    if (Current->is(tok::comma)) {
+    if (Left->Type == TT_ObjCMethodExpr) {
+      if (Current->is(tok::colon))
+        ++Left->ParameterCount;
+    } else if (Current->is(tok::comma)) {
       ++Left->ParameterCount;
       if (!Left->Role)
         Left->Role.reset(new CommaSeparatedList(Style));
       Left->Role->CommaFound(Current);
     } else if (Left->ParameterCount == 0 && Current->isNot(tok::comment)) {
       Left->ParameterCount = 1;
     }
   }

cfe/trunk/unittests/Format/FormatTestObjC.cpp

verifyFormat("bool a = ([aaaaaaaa aaaaa] == aaaaaaaaaaaaaaaaa ||\n"
                "          [aaaaaaaa aaaaa] == aaaaaaaaaaaaaaaaaaaa);");
 
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
                "        ofSize:aa:bbb\n"
                "      atOrigin:cc:dd];");
 
+  // Inline block as a first argument.
+  verifyFormat("[object justBlock:^{\n"
+               "  a = 42;\n"
+               "}];");
+  verifyFormat("[object\n"
+               "    justBlock:^{\n"
+               "      a = 42;\n"
+               "    }\n"
+               "     notBlock:42\n"
+               "            a:42];");
+  verifyFormat("[object\n"
+               "    firstBlock:^{\n"
+               "      a = 42;\n"
+               "    }\n"
+               "    blockWithLongerName:^{\n"
+               "      a = 42;\n"
+               "    }];");
+  verifyFormat("[object\n"
+               "    blockWithLongerName:^{\n"
+               "      a = 42;\n"
+               "    }\n"
+               "    secondBlock:^{\n"
+               "      a = 42;\n"
+               "    }];");
+  verifyFormat("[object\n"
+               "    firstBlock:^{\n"
+               "      a = 42;\n"
+               "    }\n"
+               "    notBlock:42\n"
+               "    secondBlock:^{\n"
+               "      a = 42;\n"
+               "    }];");
+
   Style.ColumnLimit = 70;
   verifyFormat(
       "void f() {\n"
       "  popup_wdow_.reset([[RenderWidgetPopupWindow alloc]\n"
       "      iniithContentRect:NSMakRet(origin_global.x, origin_global.y,\n"
       "                                 pos.width(), pos.height())\n"
       "                syeMask:NSBorderlessWindowMask\n"
       "                  bking:NSBackingStoreBuffered\n"