clang-format: keep ObjC colon alignment with short object name

[Typz]

Summary

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 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];

[Typz]

fix type in commit message

[djasper]

lib/Format/ContinuationIndenter.cpp

905

I'd prefer to use std::max<unsigned>( .. )

(and we generally don't use c-style casts)

[krasimir]

unittests/Format/FormatTestObjC.cpp

794

Could you add an instance where the first one is the longest and is wrapped?

[Typz]

Address review comments

lib/Format/ContinuationIndenter.cpp

905

this is C++-style C-style cast :-)

anyway changed to use ColumnWidth, as this is also what is used for initializing LongestObjCSelectorName.

[djasper]

Looks good.

[Typz]

rebase on latest master.

lib/Format/ContinuationIndenter.cpp

if (!State.Stack.back().ObjCSelectorNameFound) {
       if (NextNonComment->LongestObjCSelectorName == 0) {
         State.Stack.back().AlignColons = false;
       } else {
         State.Stack.back().ColonPos =
             (Style.IndentWrappedFunctionNames
                  ? std::max(State.Stack.back().Indent,
                             State.FirstIndent + Style.ContinuationIndentWidth)
                  : State.Stack.back().Indent) +
-            NextNonComment->LongestObjCSelectorName;
+            std::max(NextNonComment->LongestObjCSelectorName,
+                     NextNonComment->ColumnWidth);
       }
     } else if (State.Stack.back().AlignColons &&
                State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
       State.Stack.back().ColonPos = State.Column + NextNonComment->ColumnWidth;
     }
   } else if (PreviousNonComment && PreviousNonComment->is(tok::colon) &&
              PreviousNonComment->isOneOf(TT_ObjCMethodExpr, TT_DictLiteral)) {
     // FIXME: This is hacky, find a better way. The problem is that in an ObjC
unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
   if (NextNonComment->is(TT_SelectorName)) {
     if (!State.Stack.back().ObjCSelectorNameFound) {
       if (NextNonComment->LongestObjCSelectorName == 0)
         return State.Stack.back().Indent;
       return (Style.IndentWrappedFunctionNames
                   ? std::max(State.Stack.back().Indent,
                              State.FirstIndent + Style.ContinuationIndentWidth)
                   : State.Stack.back().Indent) +
-             NextNonComment->LongestObjCSelectorName -
+             std::max(NextNonComment->LongestObjCSelectorName,
+                      NextNonComment->ColumnWidth) -
              NextNonComment->ColumnWidth;
     }
     if (!State.Stack.back().AlignColons)
       return State.Stack.back().Indent;
     if (State.Stack.back().ColonPos > NextNonComment->ColumnWidth)
       return State.Stack.back().ColonPos - NextNonComment->ColumnWidth;
     return State.Stack.back().Indent;
   }

lib/Format/TokenAnnotator.cpp

case tok::colon:
         const FormatToken *BeforePrevious = Tok->Previous->Previous;
         if (!BeforePrevious ||
             !(BeforePrevious->is(TT_CastRParen) ||
               (BeforePrevious->is(TT_ObjCMethodExpr) &&
                BeforePrevious->is(tok::colon))) ||
             BeforePrevious->is(tok::r_square) ||
             Contexts.back().LongestObjCSelectorName == 0) {
           Tok->Previous->Type = TT_SelectorName;
-          if (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;
-          if (!Contexts.back().FirstObjCSelectorName)
-            Contexts.back().FirstObjCSelectorName = Tok->Previous;
         }
       } else if (Contexts.back().ColonIsForRangeExpr) {
         Tok->Type = TT_RangeBasedForLoopColon;
       } else if (CurrentToken && CurrentToken->is(tok::numeric_constant)) {
         Tok->Type = TT_BitFieldColon;
       } else if (Contexts.size() == 1 &&
                  !Line.First->isOneOf(tok::kw_enum, tok::kw_case)) {
         if (Tok->getPreviousNonComment()->isOneOf(tok::r_paren,

unittests/Format/FormatTestObjC.cpp

verifyFormat("[self // break\n"
                "      a:a\n"
                "    aaa:aaa];");
   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];");
+               "       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"
verifyFormat(
       "void f() {\n"
       "  popup_window_.reset([[RenderWidgetPopupWindow alloc]\n"
       "      initWithContentRect:NSMakeRect(origin_global.x, origin_global.y,\n"
       "                                     pos.width(), pos.height())\n"
       "                styleMask:NSBorderlessWindowMask\n"
       "                  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];");
+  verifyFormat("[aaaaaaaaaaaaaaaaaaaaaaaaa\n"
+               "        performSelectorOnMainThread:@selector(loadAccessories)\n"
+               "                         withObject:nil\n"
+               "                      waitUntilDone:false];");
+  verifyFormat("[self // force wrapping\n"
+               "        performSelectorOnMainThread:@selector(loadAccessories)\n"
+               "                         withObject:nil\n"
+               "                      waitUntilDone:false];");
 }
 
 TEST_F(FormatTestObjC, ObjCAt) {
   verifyFormat("@autoreleasepool");
   verifyFormat("@catch");
   verifyFormat("@class");
   verifyFormat("@compatibility_alias");
   verifyFormat("@defs");
   verifyFormat("@dynamic");
   verifyFormat("@encode");
   verifyFormat("@end");
   verifyFormat("@finally");
   verifyFormat("@implementation");
   verifyFormat("@import");
   verifyFormat("@interface");
   verifyFormat("@optional");
   verifyFormat("@package");
   verifyFormat("@private");
   verifyFormat("@property");
   verifyFormat("@protected");