From 63d7cedca8616921c1908c88c2f23fcd67bbab99 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Tue, 5 Feb 2013 10:07:47 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174364 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/Format.cpp | 32 +++++++++++++++- lib/Format/TokenAnnotator.cpp | 50 +++++++++++++++++-------- lib/Format/TokenAnnotator.h | 8 +++- unittests/Format/FormatTest.cpp | 65 +++++++++++++++++++++++---------- 4 files changed, 117 insertions(+), 38 deletions(-) diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 06ebc7808c..bbebec3652 100644 --- a/lib/Format/Format.cpp +++ b/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/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index f34bc89cfb..03717e91c9 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/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/lib/Format/TokenAnnotator.h b/lib/Format/TokenAnnotator.h index 0386b88f91..64c597e0d9 100644 --- a/lib/Format/TokenAnnotator.h +++ b/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 Children; AnnotatedToken *Parent; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index d1894e1e42..7d9463603b 100644 --- a/unittests/Format/FormatTest.cpp +++ b/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)numbers;"); verifyGoogleFormat("- (void)setDelegate:(id)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) { -- 2.40.0