From fb7f5c08b91246935c1e226dab13bb4259e11743 Mon Sep 17 00:00:00 2001 From: Jacek Olesiak Date: Wed, 7 Feb 2018 10:35:08 +0000 Subject: [PATCH] [clang-format] Fix ObjC message arguments formatting. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Fixes formatting of ObjC message arguments when inline block is a first argument. Having inline block as a first argument when method has multiple parameters is discouraged by Apple: "It’s best practice to use only one block argument to a method. If the method also needs other non-block arguments, the block should come last" (https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html#//apple_ref/doc/uid/TP40011210-CH8-SW7), it should be correctly formatted nevertheless. Current formatting: ``` [object blockArgument:^{ a = 42; } anotherArg:42]; ``` Fixed (colon alignment): ``` [object blockArgument:^{ a = 42; } anotherArg:42]; ``` Test Plan: make -j12 FormatTests && tools/clang/unittests/Format/FormatTests Reviewers: krasimir, benhamilton Reviewed By: krasimir, benhamilton Subscribers: benhamilton, klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D42493 llvm-svn: 324469 --- clang/lib/Format/ContinuationIndenter.cpp | 5 ++++ clang/lib/Format/FormatToken.h | 4 +++ clang/lib/Format/TokenAnnotator.cpp | 12 ++++++++- clang/unittests/Format/FormatTestObjC.cpp | 33 +++++++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 717ebb8a8108..96281db95407 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -266,6 +266,11 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { return true; if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection) return true; + if (Style.Language == FormatStyle::LK_ObjC && + Current.ObjCSelectorNameParts > 1 && + Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) { + return true; + } if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) || (Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) && Style.isCpp() && diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index 071230a52623..79e8df95e78c 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -240,6 +240,10 @@ struct FormatToken { /// e.g. because several of them are block-type. unsigned LongestObjCSelectorName = 0; + /// \brief How many parts ObjC selector have (i.e. how many parameters method + /// has). + unsigned ObjCSelectorNameParts = 0; + /// \brief Stores the number of required fake parentheses and the /// corresponding operator precedence. /// diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index bb7c5ada51a9..993c937a7719 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -411,6 +411,8 @@ private: if (Contexts.back().FirstObjCSelectorName) { Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = Contexts.back().LongestObjCSelectorName; + Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts = + Left->ParameterCount; if (Left->BlockParameterCount > 1) Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0; } @@ -424,6 +426,11 @@ private: TT_DesignatedInitializerLSquare)) { Left->Type = TT_ObjCMethodExpr; StartsObjCMethodExpr = true; + // ParameterCount might have been set to 1 before expression was + // recognized as ObjCMethodExpr (as '1 + number of commas' formula is + // used for other expression types). Parameter counter has to be, + // therefore, reset to 0. + Left->ParameterCount = 0; Contexts.back().ColonIsObjCMethodExpr = true; if (Parent && Parent->is(tok::r_paren)) Parent->Type = TT_CastRParen; @@ -498,7 +505,10 @@ private: void updateParameterCount(FormatToken *Left, FormatToken *Current) { if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ++Left->BlockParameterCount; - if (Current->is(tok::comma)) { + if (Left->Type == TT_ObjCMethodExpr) { + if (Current->is(tok::colon)) + ++Left->ParameterCount; + } else if (Current->is(tok::comma)) { ++Left->ParameterCount; if (!Left->Role) Left->Role.reset(new CommaSeparatedList(Style)); diff --git a/clang/unittests/Format/FormatTestObjC.cpp b/clang/unittests/Format/FormatTestObjC.cpp index 16a4390203c8..a99f486adfb1 100644 --- a/clang/unittests/Format/FormatTestObjC.cpp +++ b/clang/unittests/Format/FormatTestObjC.cpp @@ -693,6 +693,39 @@ TEST_F(FormatTestObjC, FormatObjCMethodExpr) { " 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" + " }\n" + " notBlock:42\n" + " a:42];"); + verifyFormat("[object\n" + " firstBlock:^{\n" + " a = 42;\n" + " }\n" + " blockWithLongerName:^{\n" + " a = 42;\n" + " }];"); + verifyFormat("[object\n" + " blockWithLongerName:^{\n" + " a = 42;\n" + " }\n" + " secondBlock:^{\n" + " a = 42;\n" + " }];"); + verifyFormat("[object\n" + " firstBlock:^{\n" + " a = 42;\n" + " }\n" + " notBlock:42\n" + " secondBlock:^{\n" + " a = 42;\n" + " }];"); + Style.ColumnLimit = 70; verifyFormat( "void f() {\n"