r324469 - [clang-format] Fix ObjC message arguments formatting.
Jacek Olesiak via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 7 02:35:08 PST 2018
Author: jolesiak
Date: Wed Feb 7 02:35:08 2018
New Revision: 324469
URL: http://llvm.org/viewvc/llvm-project?rev=324469&view=rev
Log:
[clang-format] Fix ObjC message arguments formatting.
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
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=324469&r1=324468&r2=324469&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed Feb 7 02:35:08 2018
@@ -266,6 +266,11 @@ bool ContinuationIndenter::mustBreak(con
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() &&
Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=324469&r1=324468&r2=324469&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Wed Feb 7 02:35:08 2018
@@ -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.
///
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=324469&r1=324468&r2=324469&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Feb 7 02:35:08 2018
@@ -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));
Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=324469&r1=324468&r2=324469&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Wed Feb 7 02:35:08 2018
@@ -693,6 +693,39 @@ TEST_F(FormatTestObjC, FormatObjCMethodE
" 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"
More information about the cfe-commits
mailing list