From: Jacek Olesiak <jolesiak@google.com>
Date: Wed, 7 Feb 2018 10:35:08 +0000 (+0000)
Subject: [clang-format] Fix ObjC message arguments formatting.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=848874aed95a913fb45f363120500cebfe54e2ef;p=clang

[clang-format] Fix ObjC message arguments formatting.

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

Having inline block as a first argument 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

Reviewers: krasimir, benhamilton

Reviewed By: krasimir, benhamilton

Subscribers: benhamilton, klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D42493

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

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 717ebb8a81..96281db954 100644
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -266,6 +266,11 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
     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() &&
diff --git a/lib/Format/FormatToken.h b/lib/Format/FormatToken.h
index 071230a526..79e8df95e7 100644
--- a/lib/Format/FormatToken.h
+++ b/lib/Format/FormatToken.h
@@ -240,6 +240,10 @@ struct FormatToken {
   /// 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.
   ///
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index bb7c5ada51..993c937a77 100644
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -411,6 +411,8 @@ private:
         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;
         }
@@ -424,6 +426,11 @@ private:
                           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;
@@ -498,7 +505,10 @@ private:
   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));
diff --git a/unittests/Format/FormatTestObjC.cpp b/unittests/Format/FormatTestObjC.cpp
index 16a4390203..a99f486adf 100644
--- a/unittests/Format/FormatTestObjC.cpp
+++ b/unittests/Format/FormatTestObjC.cpp
@@ -693,6 +693,39 @@ TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
                "        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"