[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

When the target object expression is short and the first selector name is
long, clang-format used to break the colon alignment:

  [I performSelectorOnMainThread:@selector(loadAccessories)
                       withObject:nil
                    waitUntilDone:false];

This happens because the colon is placed at `ContinuationIndent +
LongestObjCSelectorName`, so that any selector can be wrapped. This is
however not needed in case the longest selector is the first one, and not
wrapped.

To overcome this, this patch does not include the first selector in
`LongestObjCSelectorName` computation (in TokenAnnotator), and lets
ContinuationIndenter account decide how to account for the first selector
when wrapping. (Note this was already partly the case, see line 521 of
ContinuationIndenter.cpp)

This way, the code gets properly aligned whenever possible without
breaking the continuation indent.

  [I performSelectorOnMainThread:@selector(loadAccessories)
                      withObject:nil
                   waitUntilDone:false];
  [I // force break
      performSelectorOnMainThread:@selector(loadAccessories)
                       withObject:nil
                    waitUntilDone:false];
  [I perform:@selector(loadAccessories)
      withSelectorOnMainThread:true
                 waitUntilDone:false];


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===================================================================
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // 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];");
+               "       ofSize:aa:bbb\n"
+               "     atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,7 +686,19 @@
       "                  backing:NSBackingStoreBuffered\n"
       "                    defer:NO]);\n"
       "}");
-}
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+               "                       withObject:nil\n"
+               "                    waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+               "        withObjectOnMainThread:nil\n"
+               "                 waitUntilDone:false];");
+  }
 
 TEST_F(FormatTestObjC, ObjCAt) {
   verifyFormat("@autoreleasepool");
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -571,12 +571,12 @@
             BeforePrevious->is(tok::r_square) ||
             Contexts.back().LongestObjCSelectorName == 0) {
           Tok->Previous->Type = TT_SelectorName;
-          if (Tok->Previous->ColumnWidth >
-              Contexts.back().LongestObjCSelectorName)
-            Contexts.back().LongestObjCSelectorName =
-                Tok->Previous->ColumnWidth;
           if (!Contexts.back().FirstObjCSelectorName)
             Contexts.back().FirstObjCSelectorName = Tok->Previous;
+          else if (Tok->Previous->ColumnWidth >
+                   Contexts.back().LongestObjCSelectorName)
+            Contexts.back().LongestObjCSelectorName =
+                Tok->Previous->ColumnWidth;
         }
       } else if (Contexts.back().ColonIsForRangeExpr) {
         Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -696,7 +696,8 @@
                  ? std::max(State.Stack.back().Indent,
                             State.FirstIndent + Style.ContinuationIndentWidth)
                  : State.Stack.back().Indent) +
-            NextNonComment->LongestObjCSelectorName;
+            std::max(NextNonComment->LongestObjCSelectorName,
+                     unsigned(NextNonComment->TokenText.size()));
       }
     } else if (State.Stack.back().AlignColons &&
                State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -895,7 +896,8 @@
                   ? std::max(State.Stack.back().Indent,
                              State.FirstIndent + Style.ContinuationIndentWidth)
                   : State.Stack.back().Indent) +
-             NextNonComment->LongestObjCSelectorName -
+             std::max(NextNonComment->LongestObjCSelectorName,
+                      unsigned(NextNonComment->TokenText.size())) -
              NextNonComment->ColumnWidth;
     }
     if (!State.Stack.back().AlignColons)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43121.133588.patch
Type: text/x-patch
Size: 3628 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180209/18136ffa/attachment.bin>