Formatter: Prefer breaking before ObjC selector names over breaking at their ':'

Before:
  if ((self = [super initWithContentRect:contentRect styleMask:
                  styleMask backing:NSBackingStoreBuffered defer:YES])) {

Now:
  if ((self = [super initWithContentRect:contentRect styleMask:styleMask
                  backing:NSBackingStoreBuffered defer:YES])) {

llvm-svn: 172333

2 files changed, 31 insertions, 5 deletions

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 8ca0a823fe4..de870900b41 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -170,7 +170,7 @@ static void replacePPWhitespace(
 
 /// \brief Checks whether the (remaining) \c UnwrappedLine starting with
 /// \p RootToken fits into \p Limit columns.
-bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned Limit) {
+static bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned Limit) {
   unsigned Columns = RootToken.FormatTok.TokenLength;
   bool FitsOnALine = true;
   const AnnotatedToken *Tok = &RootToken;
@@ -188,6 +188,15 @@ bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned Limit) {
   return FitsOnALine;
 }
 
+/// \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;
+}
+
 class UnwrappedLineFormatter {
 public:
   UnwrappedLineFormatter(const FormatStyle &Style, SourceManager &SourceMgr,
@@ -479,6 +488,14 @@ private:
     if (Left.is(tok::semi) || Left.is(tok::comma) ||
         Left.ClosesTemplateDeclaration)
       return 0;
+
+    // In Objective-C method expressions, prefer breaking before "param:" over
+    // breaking after it.
+    if (isObjCSelectorName(Right))
+      return 0;
+    if (Right.is(tok::colon) && Right.Type == TT_ObjCMethodExpr)
+      return 20;
+
     if (Left.is(tok::l_paren))
       return 20;
 
@@ -1188,6 +1205,8 @@ private:
       return false;
     if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr)
       return true;
+    if (isObjCSelectorName(Right))
+      return true;
     if (Left.ClosesTemplateDeclaration)
       return true;
     if (Left.Type == TT_PointerOrReference || Left.Type == TT_TemplateCloser ||
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 668e8af76bc..cba25fcf8f4 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1537,12 +1537,18 @@ TEST_F(FormatTest, FormatObjCMethodExpr) {
   verifyFormat("throw [self errorFor:a];");
   verifyFormat("@throw [self errorFor:a];");
 
-  // The formatting of this isn't ideal yet. It tests that the formatter doesn't
-  // break after "backing" but before ":", which would be at 80 columns.
+  // This tests that the formatter doesn't break after "backing" but before ":",
+  // which would be at 80 columns.
   verifyFormat(
       "void f() {\n"
-      "  if ((self = [super initWithContentRect:contentRect styleMask:\n"
-      "                  styleMask backing:NSBackingStoreBuffered defer:YES]))");
+      "  if ((self = [super initWithContentRect:contentRect styleMask:styleMask\n"
+      "                  backing:NSBackingStoreBuffered defer:YES]))");
+  
+  verifyFormat("[foo setasdfasdffffffffffffadfasdfasdf:\n"
+               "    [bar dowith:asdfdsfasdfasdfasfasfasfsafasdfsfad]];");
+
+  verifyFormat("[foo checkThatBreakingAfterColonWorksOk:\n"
+               "    [bar ifItDoes:reduceOverallLineLengthLikeInThisCase]];");
   
 }
 
@@ -1582,6 +1588,7 @@ TEST_F(FormatTest, ObjCAt) {
   verifyFormat("@'c'");
   verifyFormat("@true");
   verifyFormat("NSNumber *smallestInt = @(-INT_MAX - 1);");
+  // FIXME: Array and dictionary literals need more work.
   verifyFormat("@[");
   verifyFormat("@{");