[cfe-commits] [PATCH] [patch] Formatter: Put spaces in ObjC method declarations in the right place for Google style

[Nico Weber]

Hi djasper,

Objective-C method declarations look like this:

- (returntype)name:(type)argname anothername:(type)arg2name;

In google style, there's no space after the leading '-' but one after "(returntype)" instead (but none after the argument types).

Not inserting the space after '-' is easy, but to insert the space after the return type, the formatter needs to know that a closing parenthesis ends the return type. To do this, I tweaked the code in parse() to check for this, which in turn required moving detection of TT_ObjCMethodSpecifier from annotate() to parse(), because parse() runs before annotate(). (Is there a reason parse() and annotate() is done in two distinct steps?) parse() sets a few other token types too, so I think that should be ok.

(To keep things interesting, the return type is optional, but it's almost always there in practice.)

http://llvm-reviews.chandlerc.com/D280

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  include/clang/Format/Format.h

Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -39,6 +39,7 @@
   TT_ObjCBlockLParen,
   TT_ObjCDecl,
   TT_ObjCMethodSpecifier,
+  TT_ObjCSelectorStart,
   TT_ObjCProperty,
   TT_OverloadedOperator,
   TT_PointerOrReference,
@@ -107,6 +108,7 @@
   LLVMStyle.IndentCaseLabels = false;
   LLVMStyle.SpacesBeforeTrailingComments = 1;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
+  LLVMStyle.ObjCSpaceBeforeReturnType = true;
   return LLVMStyle;
 }
 
@@ -120,6 +122,7 @@
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.SpacesBeforeTrailingComments = 2;
   GoogleStyle.ObjCSpaceBeforeProtocolList = false;
+  GoogleStyle.ObjCSpaceBeforeReturnType = false;
   return GoogleStyle;
 }
 
@@ -680,14 +683,26 @@
       AnnotatedToken *Tok = CurrentToken;
       next();
       switch (Tok->FormatTok.Tok.getKind()) {
-      case tok::l_paren:
+      case tok::plus:
+      case tok::minus:
+        // At the start of the line, +/- specific ObjectiveC method
+        // declarations.
+        if (Tok->Parent == NULL)
+          Tok->Type = TT_ObjCMethodSpecifier;
+        break;
+      case tok::l_paren: {
+        bool ParensWereObjCReturnType =
+            Tok->Parent && Tok->Parent->Type == TT_ObjCMethodSpecifier;
         if (!parseParens())
           return false;
         if (CurrentToken != NULL && CurrentToken->is(tok::colon)) {
           CurrentToken->Type = TT_CtorInitializerColon;
           next();
+        } else if (CurrentToken != NULL && ParensWereObjCReturnType) {
+          CurrentToken->Type = TT_ObjCSelectorStart;
+          next();
         }
-        break;
+      } break;
       case tok::l_square:
         if (!parseSquare())
           return false;
@@ -948,10 +963,6 @@
   }
 
   TokenType determinePlusMinusCaretUsage(const AnnotatedToken &Tok) {
-    // At the start of the line, +/- specific ObjectiveC method declarations.
-    if (Tok.Parent == NULL)
-      return TT_ObjCMethodSpecifier;
-
     // Use heuristics to recognize unary operators.
     if (Tok.Parent->is(tok::equal) || Tok.Parent->is(tok::l_paren) ||
         Tok.Parent->is(tok::comma) || Tok.Parent->is(tok::l_square) ||
@@ -1044,7 +1055,9 @@
       if (Tok.is(tok::colon))
         return false;
       if (Tok.Parent->Type == TT_ObjCMethodSpecifier)
-        return true;
+        return Style.ObjCSpaceBeforeReturnType || Tok.isNot(tok::l_paren);
+      if (Tok.Type == TT_ObjCSelectorStart)
+        return !Style.ObjCSpaceBeforeReturnType;
       if (Tok.Parent->is(tok::r_paren) && Tok.is(tok::identifier))
         // Don't space between ')' and <id>
         return false;
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1178,10 +1178,18 @@
           "outRange8:(NSRange) out_range8  outRange9:(NSRange) out_range9;"));
 
   verifyFormat("- (int)sum:(vector<int>)numbers;");
-  verifyGoogleFormat("- (void)setDelegate:(id<Protocol>)delegate;");
+  verifyGoogleFormat("-(void) setDelegate:(id<Protocol>)delegate;");
   // FIXME: In LLVM style, there should be a space in front of a '<' for ObjC
   // protocol lists (but not for template classes):
   //verifyFormat("- (void)setDelegate:(id <Protocol>)delegate;");
+
+  verifyFormat("- (int(*)())foo:(int(*)())f;");
+  verifyGoogleFormat("-(int(*)()) foo:(int(*)())foo;");
+
+  // If there's no return type (very rare in practice!), LLVM and Google style
+  // agree.
+  verifyFormat("- foo:(int)f;");
+  verifyGoogleFormat("- foo:(int)foo;");
 }
 
 TEST_F(FormatTest, FormatObjCBlocks) {
@@ -1191,7 +1199,6 @@
 
 TEST_F(FormatTest, FormatObjCInterface) {
   // FIXME: Handle comments like in "@interface /* wait for it */ Foo", PR14875
-  // FIXME: In google style, it's "+(id) init", not "+ (id)init".
   verifyFormat("@interface Foo : NSObject <NSSomeDelegate> {\n"
                "@public\n"
                "  int field1;\n"
@@ -1215,7 +1222,7 @@
                      " @package\n"
                      "  int field4;\n"
                      "}\n"
-                     "+ (id)init;\n"
+                     "+(id) init;\n"
                      "@end");
 
   verifyFormat("@interface Foo\n"
@@ -1238,7 +1245,7 @@
                "@end");
 
   verifyGoogleFormat("@interface Foo : Bar<Baz, Quux>\n"
-                     "+ (id)init;\n"
+                     "+(id) init;\n"
                      "@end");
 
   verifyFormat("@interface Foo (HackStuff)\n"
@@ -1254,7 +1261,7 @@
                "@end");
 
   verifyGoogleFormat("@interface Foo (HackStuff)<MyProtocol>\n"
-                     "+ (id)init;\n"
+                     "+(id) init;\n"
                      "@end");
 
   verifyFormat("@interface Foo {\n"
@@ -1318,7 +1325,7 @@
                      " @package\n"
                      "  int field4;\n"
                      "}\n"
-                     "+ (id)init {}\n"
+                     "+(id) init {}\n"
                      "@end");
 
   verifyFormat("@implementation Foo\n"
@@ -1369,7 +1376,7 @@
                "@end");
 
   verifyGoogleFormat("@protocol MyProtocol<NSObject>\n"
-                     "- (NSUInteger)numberOfThings;\n"
+                     "-(NSUInteger) numberOfThings;\n"
                      "@end");
 
   verifyFormat("@protocol Foo;\n"
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -59,6 +59,10 @@
   /// \brief Add a space in front of an Objective-C protocol list, i.e. use
   /// Foo <Protocol> instead of Foo<Protocol>.
   bool ObjCSpaceBeforeProtocolList;
+
+  /// \brief Add a space in front method return types, i.e. use
+  /// + (id)init instead of +(id) init
+  bool ObjCSpaceBeforeReturnType;
 };
 
 /// \brief Returns a format style complying with the LLVM coding standards:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D280.1.patch
Type: text/x-patch
Size: 5982 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130110/a36ce0ae/attachment.bin>