Initial support for formatting ObjC method declarations/calls.

We can now format stuff like:
- (void)doSomethingWith:(GTMFoo *)theFoo
                   rect:(NSRect)theRect
               interval:(float)theInterval {
  [myObject doFooWith:arg1 //
                 name:arg2
                error:arg3];

}

This seems to fix everything mentioned in llvm.org/PR14939.

llvm-svn: 174364

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 06ebc78..bbebec3 100644 (file)
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -268,7 +268,7 @@ private:
         : Indent(Indent), LastSpace(LastSpace), AssignmentColumn(0),
           FirstLessLess(0), BreakBeforeClosingBrace(false), QuestionColumn(0),
           AvoidBinPacking(AvoidBinPacking), BreakAfterComma(false),
-          HasMultiParameterLine(HasMultiParameterLine) {
+          HasMultiParameterLine(HasMultiParameterLine), ColonPos(0) {
     }
 
     /// \brief The position to which a specific parenthesis level needs to be
@@ -312,6 +312,9 @@ private:
     /// \brief This context already has a line with more than one parameter.
     bool HasMultiParameterLine;
 
+    /// \brief The position of the colon in an ObjC method declaration/call.
+    unsigned ColonPos;
+
     bool operator<(const ParenState &Other) const {
       if (Indent != Other.Indent)
         return Indent < Other.Indent;
@@ -331,6 +334,8 @@ private:
         return BreakAfterComma;
       if (HasMultiParameterLine != Other.HasMultiParameterLine)
         return HasMultiParameterLine;
+      if (ColonPos != Other.ColonPos)
+        return ColonPos < Other.ColonPos;
       return false;
     }
   };
@@ -427,6 +432,17 @@ private:
       } else if (Previous.Type == TT_BinaryOperator &&
                  State.Stack.back().AssignmentColumn != 0) {
         State.Column = State.Stack.back().AssignmentColumn;
+      } else if (Current.Type == TT_ObjCSelectorName) {
+        if (State.Stack.back().ColonPos > Current.FormatTok.TokenLength) {
+          State.Column =
+              State.Stack.back().ColonPos - Current.FormatTok.TokenLength;
+        } else {
+          State.Column = State.Stack.back().Indent;
+          State.Stack.back().ColonPos =
+              State.Column + Current.FormatTok.TokenLength;
+        }
+      } else if (Previous.Type == TT_ObjCMethodExpr) {
+        State.Column = State.Stack.back().Indent + 4;
       } else {
         State.Column = State.Stack[ParenLevel].Indent;
       }
@@ -461,6 +477,17 @@ private:
       if (!DryRun)
         Whitespaces.replaceWhitespace(Current, 0, Spaces, State.Column, Style);
 
+      if (Current.Type == TT_ObjCSelectorName &&
+          State.Stack.back().ColonPos == 0) {
+        if (State.Stack.back().Indent + Current.LongestObjCSelectorName >
+            State.Column + Spaces + Current.FormatTok.TokenLength)
+          State.Stack.back().ColonPos =
+              State.Stack.back().Indent + Current.LongestObjCSelectorName;
+        else
+          State.Stack.back().ColonPos =
+              State.Column + Spaces + Current.LongestObjCSelectorName;
+      }
+
       // FIXME: Do we need to do this for assignments nested in other
       // expressions?
       if (RootToken.isNot(tok::kw_for) && ParenLevel == 0 &&
@@ -699,6 +726,9 @@ private:
         State.Stack.back().BreakAfterComma &&
         !isTrailingComment(*State.NextToken))
       return true;
+    if (State.NextToken->Type == TT_ObjCSelectorName &&
+        State.Stack.back().ColonPos != 0)
+      return true;
     if ((State.NextToken->Type == TT_CtorInitializerColon ||
          (State.NextToken->Parent->ClosesTemplateDeclaration &&
           State.Stack.size() == 1)))

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index f34bc89..03717e9 100644 (file)
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -20,15 +20,6 @@
 namespace clang {
 namespace format {
 
-/// \brief Returns if a token is an Objective-C selector name.
-///
-/// For example, "bar" is a selector name in [foo bar:(4 + 5)].
-static bool isObjCSelectorName(const AnnotatedToken &Tok) {
-  return Tok.is(tok::identifier) && !Tok.Children.empty() &&
-         Tok.Children[0].is(tok::colon) &&
-         Tok.Children[0].Type == TT_ObjCMethodExpr;
-}
-
 static bool isBinaryOperator(const AnnotatedToken &Tok) {
   // Comma is a binary operator, but does not behave as such wrt. formatting.
   return getPrecedence(Tok) > prec::Comma;
@@ -65,6 +56,7 @@ public:
   AnnotatingParser(SourceManager &SourceMgr, Lexer &Lex, AnnotatedLine &Line)
       : SourceMgr(SourceMgr), Lex(Lex), Line(Line), CurrentToken(&Line.First),
         KeywordVirtualFound(false), ColonIsObjCMethodExpr(false),
+        LongestObjCSelectorName(0), FirstObjCSelectorName(NULL),
         ColonIsForRangeExpr(false), IsExpression(false),
         LookForFunctionName(Line.MustBeDeclaration), BindingStrength(1) {
   }
@@ -82,6 +74,8 @@ public:
 
     void markStart(AnnotatedToken &Left) {
       P.ColonIsObjCMethodExpr = true;
+      P.LongestObjCSelectorName = 0;
+      P.FirstObjCSelectorName = NULL;
       Left.Type = TT_ObjCMethodExpr;
     }
 
@@ -168,8 +162,13 @@ public:
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
 
-        if (StartsObjCMethodExpr)
+        if (StartsObjCMethodExpr) {
           objCSelector.markEnd(*CurrentToken);
+          if (FirstObjCSelectorName != NULL) {
+            FirstObjCSelectorName->LongestObjCSelectorName =
+                LongestObjCSelectorName;
+          }
+        }
 
         next();
         return true;
@@ -221,6 +220,9 @@ public:
         }
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
+        if (FirstObjCSelectorName != NULL)
+          FirstObjCSelectorName->LongestObjCSelectorName =
+              LongestObjCSelectorName;
         next();
         return true;
       }
@@ -295,12 +297,19 @@ public:
       break;
     case tok::colon:
       // Colons from ?: are handled in parseConditional().
-      if (Tok->Parent->is(tok::r_paren))
+      if (Tok->Parent->is(tok::r_paren)) {
         Tok->Type = TT_CtorInitializerColon;
-      else if (ColonIsObjCMethodExpr)
+      } else if (ColonIsObjCMethodExpr ||
+                 Line.First.Type == TT_ObjCMethodSpecifier) {
         Tok->Type = TT_ObjCMethodExpr;
-      else if (ColonIsForRangeExpr)
+        Tok->Parent->Type = TT_ObjCSelectorName;
+        if (Tok->Parent->FormatTok.TokenLength > LongestObjCSelectorName)
+          LongestObjCSelectorName = Tok->Parent->FormatTok.TokenLength;
+        if (FirstObjCSelectorName == NULL)
+          FirstObjCSelectorName = Tok->Parent;
+      } else if (ColonIsForRangeExpr) {
         Tok->Type = TT_RangeBasedForLoopColon;
+      }
       break;
     case tok::kw_if:
     case tok::kw_while:
@@ -452,6 +461,13 @@ public:
     if (PeriodsAndArrows >= 2 && CanBeBuilderTypeStmt)
       return LT_BuilderTypeCall;
 
+    if (Line.First.Type == TT_ObjCMethodSpecifier) {
+      if (FirstObjCSelectorName != NULL)
+        FirstObjCSelectorName->LongestObjCSelectorName =
+            LongestObjCSelectorName;
+      return LT_ObjCMethodDecl;
+    }
+
     return LT_Other;
   }
 
@@ -474,6 +490,8 @@ private:
   AnnotatedToken *CurrentToken;
   bool KeywordVirtualFound;
   bool ColonIsObjCMethodExpr;
+  unsigned LongestObjCSelectorName;
+  AnnotatedToken *FirstObjCSelectorName;
   bool ColonIsForRangeExpr;
   bool IsExpression;
   bool LookForFunctionName;
@@ -725,9 +743,9 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedToken &Tok) {
 
   // In Objective-C method expressions, prefer breaking before "param:" over
   // breaking after it.
-  if (isObjCSelectorName(Right))
+  if (Right.Type == TT_ObjCSelectorName)
     return 0;
-  if (Right.is(tok::colon) && Right.Type == TT_ObjCMethodExpr)
+  if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr)
     return 20;
 
   if (Left.is(tok::l_paren) || Left.is(tok::l_square) ||
@@ -885,7 +903,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedToken &Right) {
     return false;
   if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr)
     return true;
-  if (isObjCSelectorName(Right))
+  if (Right.Type == TT_ObjCSelectorName)
     return true;
   if (Left.ClosesTemplateDeclaration)
     return true;

diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index 0386b88..64c597e 100644 (file)
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -40,6 +40,7 @@ enum TokenType {
   TT_ObjCMethodSpecifier,
   TT_ObjCMethodExpr,
   TT_ObjCProperty,
+  TT_ObjCSelectorName,
   TT_OverloadedOperator,
   TT_PointerOrReference,
   TT_PureVirtualSpecifier,
@@ -69,7 +70,8 @@ public:
       : FormatTok(FormatTok), Type(TT_Unknown), SpaceRequiredBefore(false),
         CanBreakBefore(false), MustBreakBefore(false),
         ClosesTemplateDeclaration(false), MatchingParen(NULL),
-        ParameterCount(1), BindingStrength(0), SplitPenalty(0), Parent(NULL) {
+        ParameterCount(1), BindingStrength(0), SplitPenalty(0),
+        LongestObjCSelectorName(0), Parent(NULL) {
   }
 
   bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
@@ -109,6 +111,10 @@ public:
   /// \brief Penalty for inserting a line break before this token.
   unsigned SplitPenalty;
 
+  /// \brief If this is the first ObjC selector name in an ObjC method
+  /// definition or call, this contains the length of the longest name.
+  unsigned LongestObjCSelectorName;
+
   std::vector<AnnotatedToken> Children;
   AnnotatedToken *Parent;
 

diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index d1894e1..7d94636 100644 (file)
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1979,20 +1979,18 @@ TEST_F(FormatTest, FormatForObjectiveCMethodDecls) {
       format("- (void)sendAction:(SEL)aSelector to:(id)anObject forAllCells:(BOOL)flag;"));
 
   // Very long objectiveC method declaration.
-  EXPECT_EQ(
-      "- (NSUInteger)indexOfObject:(id)anObject inRange:(NSRange)range\n    "
-      "outRange:(NSRange)out_range outRange1:(NSRange)out_range1\n    "
-      "outRange2:(NSRange)out_range2 outRange3:(NSRange)out_range3\n    "
-      "outRange4:(NSRange)out_range4 outRange5:(NSRange)out_range5\n    "
-      "outRange6:(NSRange)out_range6 outRange7:(NSRange)out_range7\n    "
-      "outRange8:(NSRange)out_range8 outRange9:(NSRange)out_range9;",
-      format(
-          "- (NSUInteger)indexOfObject:(id)anObject inRange:(NSRange)range "
-          "outRange:(NSRange) out_range outRange1:(NSRange) out_range1 "
-          "outRange2:(NSRange) out_range2  outRange3:(NSRange) out_range3  "
-          "outRange4:(NSRange) out_range4  outRange5:(NSRange) out_range5 "
-          "outRange6:(NSRange) out_range6  outRange7:(NSRange) out_range7  "
-          "outRange8:(NSRange) out_range8  outRange9:(NSRange) out_range9;"));
+  verifyFormat("- (NSUInteger)indexOfObject:(id)anObject\n"
+               "                    inRange:(NSRange)range\n"
+               "                   outRange:(NSRange)out_range\n"
+               "                  outRange1:(NSRange)out_range1\n"
+               "                  outRange2:(NSRange)out_range2\n"
+               "                  outRange3:(NSRange)out_range3\n"
+               "                  outRange4:(NSRange)out_range4\n"
+               "                  outRange5:(NSRange)out_range5\n"
+               "                  outRange6:(NSRange)out_range6\n"
+               "                  outRange7:(NSRange)out_range7\n"
+               "                  outRange8:(NSRange)out_range8\n"
+               "                  outRange9:(NSRange)out_range9;");
 
   verifyFormat("- (int)sum:(vector<int>)numbers;");
   verifyGoogleFormat("- (void)setDelegate:(id<Protocol>)delegate;");
@@ -2218,6 +2216,18 @@ TEST_F(FormatTest, FormatObjCProtocol) {
                "@end\n");
 }
 
+TEST_F(FormatTest, FormatObjCMethodDeclarations) {
+  verifyFormat("- (void)doSomethingWith:(GTMFoo *)theFoo\n"
+               "                   rect:(NSRect)theRect\n"
+               "               interval:(float)theInterval {\n"
+               "}");
+  verifyFormat("- (void)shortf:(GTMFoo *)theFoo\n"
+               "          longKeyword:(NSRect)theRect\n"
+               "    evenLongerKeyword:(float)theInterval\n"
+               "                error:(NSError **)theError {\n"
+               "}");
+}
+
 TEST_F(FormatTest, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
   verifyFormat("return [foo bar:baz];");
@@ -2266,7 +2276,7 @@ TEST_F(FormatTest, FormatObjCMethodExpr) {
   verifyFormat("[cond ? obj1 : obj2 methodWithParam:param]");
   verifyFormat("[button setAction:@selector(zoomOut:)];");
   verifyFormat("[color getRed:&r green:&g blue:&b alpha:&a];");
-  
+
   verifyFormat("arr[[self indexForFoo:a]];");
   verifyFormat("throw [self errorFor:a];");
   verifyFormat("@throw [self errorFor:a];");
@@ -2275,12 +2285,27 @@ TEST_F(FormatTest, FormatObjCMethodExpr) {
   // which would be at 80 columns.
   verifyFormat(
       "void f() {\n"
-      "  if ((self = [super initWithContentRect:contentRect styleMask:styleMask\n"
-      "          backing:NSBackingStoreBuffered defer:YES]))");
-  
+      "  if ((self = [super initWithContentRect:contentRect\n"
+      "                               styleMask:styleMask\n"
+      "                                 backing:NSBackingStoreBuffered\n"
+      "                                   defer:YES]))");
+
   verifyFormat("[foo checkThatBreakingAfterColonWorksOk:\n"
-               "    [bar ifItDoes:reduceOverallLineLengthLikeInThisCase]];");
-  
+               "        [bar ifItDoes:reduceOverallLineLengthLikeInThisCase]];");
+
+  verifyFormat("[myObj short:arg1 // Force line break\n"
+               "          longKeyword:arg2\n"
+               "    evenLongerKeyword:arg3\n"
+               "                error:arg4];");
+  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"
+      "}");
 }
 
 TEST_F(FormatTest, ObjCAt) {