r210097 - clang-format: Refactor indentation behavior for multiple nested blocks.
Daniel Jasper
djasper at google.com
Tue Jun 3 05:02:45 PDT 2014
Author: djasper
Date: Tue Jun 3 07:02:45 2014
New Revision: 210097
URL: http://llvm.org/viewvc/llvm-project?rev=210097&view=rev
Log:
clang-format: Refactor indentation behavior for multiple nested blocks.
This fixes a few oddities when formatting multiple nested JavaScript
blocks, e.g.:
Before:
promise.then(
function success() {
doFoo();
doBar();
},
[], function error() {
doFoo();
doBaz();
});
promise.then([],
function success() {
doFoo();
doBar();
},
function error() {
doFoo();
doBaz();
});
After:
promise.then(
function success() {
doFoo();
doBar();
},
[],
function error() {
doFoo();
doBaz();
});
promise.then([],
function success() {
doFoo();
doBar();
},
function error() {
doFoo();
doBaz();
});
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/ContinuationIndenter.h
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=210097&r1=210096&r2=210097&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Tue Jun 3 07:02:45 2014
@@ -112,6 +112,15 @@ bool ContinuationIndenter::canBreak(cons
return false;
if (Current.isMemberAccess() && State.Stack.back().ContainsUnwrappedBuilder)
return false;
+
+ // Don't create a 'hanging' indent if there are multiple blocks in a single
+ // statement.
+ if (Style.Language == FormatStyle::LK_JavaScript &&
+ Previous.is(tok::l_brace) && State.Stack.size() > 1 &&
+ State.Stack[State.Stack.size() - 2].JSFunctionInlined &&
+ State.Stack[State.Stack.size() - 2].HasMultipleNestedBlocks)
+ return false;
+
return !State.Stack.back().NoLineBreak;
}
@@ -294,7 +303,7 @@ void ContinuationIndenter::addTokenOnCur
// Treat the condition inside an if as if it was a second function
// parameter, i.e. let nested calls have a continuation indent.
State.Stack.back().LastSpace = State.Column;
- else if (Current.isNot(tok::comment) &&
+ else if (!Current.isOneOf(tok::comment, tok::caret) &&
(Previous.is(tok::comma) ||
(Previous.is(tok::colon) && Previous.Type == TT_ObjCMethodExpr)))
State.Stack.back().LastSpace = State.Column;
@@ -589,8 +598,6 @@ unsigned ContinuationIndenter::moveState
Current.LastOperator ? 0 : State.Column + Current.ColumnWidth;
if (Current.Type == TT_ObjCSelectorName)
State.Stack.back().ObjCSelectorNameFound = true;
- if (Current.Type == TT_LambdaLSquare)
- ++State.Stack.back().LambdasFound;
if (Current.Type == TT_CtorInitializerColon) {
// Indent 2 from the column, so:
// SomeClass::SomeClass()
@@ -767,24 +774,24 @@ static void consumeRParens(LineState& St
// SomeFunction(a, [] {
// f(); // break
// });
-static bool fakeRParenSpecialCase(const FormatToken& Tok) {
+static bool fakeRParenSpecialCase(const LineState &State) {
+ const FormatToken &Tok = *State.NextToken;
if (!Tok.MatchingParen)
return false;
const FormatToken *Left = &Tok;
if (Tok.isOneOf(tok::r_brace, tok::r_square))
Left = Tok.MatchingParen;
- return Left->isOneOf(tok::l_brace, tok::l_square) &&
+ return !State.Stack.back().HasMultipleNestedBlocks &&
+ Left->isOneOf(tok::l_brace, tok::l_square) &&
(Left->BlockKind == BK_Block ||
Left->Type == TT_ArrayInitializerLSquare ||
Left->Type == TT_DictLiteral);
}
void ContinuationIndenter::moveStatePastFakeRParens(LineState &State) {
- const FormatToken &Current = *State.NextToken;
-
// Don't remove FakeRParens attached to r_braces that surround nested blocks
// as they will have been removed early (see above).
- if (fakeRParenSpecialCase(Current))
+ if (fakeRParenSpecialCase(State))
return;
consumeRParens(State, *State.NextToken);
@@ -806,7 +813,7 @@ void ContinuationIndenter::moveStatePast
bool AvoidBinPacking;
bool BreakBeforeParameter = false;
if (Current.is(tok::l_brace) || Current.Type == TT_ArrayInitializerLSquare) {
- if (fakeRParenSpecialCase(Current))
+ if (fakeRParenSpecialCase(State))
consumeRParens(State, *Current.MatchingParen);
NewIndent = State.Stack.back().LastSpace;
@@ -848,6 +855,7 @@ void ContinuationIndenter::moveStatePast
State.Stack.back().LastSpace,
AvoidBinPacking, NoLineBreak));
State.Stack.back().BreakBeforeParameter = BreakBeforeParameter;
+ State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 1;
}
void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
@@ -874,10 +882,7 @@ void ContinuationIndenter::moveStatePast
void ContinuationIndenter::moveStateToNewBlock(LineState &State) {
// If we have already found more than one lambda introducers on this level, we
// opt out of this because similarity between the lambdas is more important.
- // FIXME: This should use fakeRParenSpecialCase() and fakeRParenSpecialCase()
- // Needs to include the LambdasFound check. Otherwise the corresponding
- // fake r_parens will never be consumed.
- if (State.Stack.back().LambdasFound <= 1)
+ if (fakeRParenSpecialCase(State))
consumeRParens(State, *State.NextToken->MatchingParen);
// For some reason, ObjC blocks are indented like continuations.
Modified: cfe/trunk/lib/Format/ContinuationIndenter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.h?rev=210097&r1=210096&r2=210097&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.h (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.h Tue Jun 3 07:02:45 2014
@@ -151,8 +151,8 @@ struct ParenState {
StartOfFunctionCall(0), StartOfArraySubscripts(0),
NestedNameSpecifierContinuation(0), CallContinuation(0), VariablePos(0),
ContainsLineBreak(false), ContainsUnwrappedBuilder(0),
- AlignColons(true), ObjCSelectorNameFound(false), LambdasFound(0),
- JSFunctionInlined(false) {}
+ AlignColons(true), ObjCSelectorNameFound(false),
+ HasMultipleNestedBlocks(false), JSFunctionInlined(false) {}
/// \brief The position to which a specific parenthesis level needs to be
/// indented.
@@ -247,11 +247,11 @@ struct ParenState {
/// the same token.
bool ObjCSelectorNameFound;
- /// \brief Counts the number of lambda introducers found on this level.
+ /// \brief \c true if there are multiple nested blocks inside these parens.
///
/// Not considered for memoization as it will always have the same value at
/// the same token.
- unsigned LambdasFound;
+ bool HasMultipleNestedBlocks;
// \brief The previous JavaScript 'function' keyword is not wrapped to a new
// line.
Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=210097&r1=210096&r2=210097&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Tue Jun 3 07:02:45 2014
@@ -103,9 +103,10 @@ struct FormatToken {
IsFirst(false), MustBreakBefore(false), IsUnterminatedLiteral(false),
BlockKind(BK_Unknown), Type(TT_Unknown), SpacesRequiredBefore(0),
CanBreakBefore(false), ClosesTemplateDeclaration(false),
- ParameterCount(0), PackingKind(PPK_Inconclusive), TotalLength(0),
- UnbreakableTailLength(0), BindingStrength(0), NestingLevel(0),
- SplitPenalty(0), LongestObjCSelectorName(0), FakeRParens(0),
+ ParameterCount(0), BlockParameterCount(0),
+ PackingKind(PPK_Inconclusive), TotalLength(0), UnbreakableTailLength(0),
+ BindingStrength(0), NestingLevel(0), SplitPenalty(0),
+ LongestObjCSelectorName(0), FakeRParens(0),
StartsBinaryExpression(false), EndsBinaryExpression(false),
OperatorIndex(0), LastOperator(false),
PartOfMultiVariableDeclStmt(false), IsForEachMacro(false),
@@ -191,6 +192,10 @@ struct FormatToken {
/// the number of commas.
unsigned ParameterCount;
+ /// \brief Number of parameters that are nested blocks,
+ /// if this is "(", "[" or "<".
+ unsigned BlockParameterCount;
+
/// \brief A token can have a special role that can carry extra information
/// about the token's formatting.
std::unique_ptr<TokenRole> Role;
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=210097&r1=210096&r2=210097&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Jun 3 07:02:45 2014
@@ -198,7 +198,6 @@ private:
return false;
else if (CurrentToken->is(tok::l_brace))
Left->Type = TT_Unknown; // Not TT_ObjCBlockLParen
- updateParameterCount(Left, CurrentToken);
if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
!CurrentToken->Next->HasUnescapedNewline &&
!CurrentToken->Next->isTrailingComment())
@@ -206,8 +205,10 @@ private:
if (CurrentToken->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->isSimpleTypeSpecifier())
Contexts.back().IsExpression = false;
+ FormatToken *Tok = CurrentToken;
if (!consumeToken())
return false;
+ updateParameterCount(Left, Tok);
if (CurrentToken && CurrentToken->HasUnescapedNewline)
HasMultipleLines = true;
}
@@ -266,7 +267,7 @@ private:
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- if (Contexts.back().NumBlockParameters > 1)
+ if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
next();
@@ -281,9 +282,10 @@ private:
(Left->Type == TT_ArraySubscriptLSquare ||
(Left->Type == TT_ObjCMethodExpr && !ColonFound)))
Left->Type = TT_ArrayInitializerLSquare;
- updateParameterCount(Left, CurrentToken);
+ FormatToken* Tok = CurrentToken;
if (!consumeToken())
return false;
+ updateParameterCount(Left, Tok);
}
return false;
}
@@ -324,6 +326,12 @@ private:
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+ if (Current->Type == TT_LambdaLSquare ||
+ (Current->is(tok::caret) && Current->Type == TT_UnaryOperator) ||
+ (Style.Language == FormatStyle::LK_JavaScript &&
+ Current->TokenText == "function")) {
+ ++Left->BlockParameterCount;
+ }
if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
@@ -639,17 +647,16 @@ private:
Context(tok::TokenKind ContextKind, unsigned BindingStrength,
bool IsExpression)
: ContextKind(ContextKind), BindingStrength(BindingStrength),
- LongestObjCSelectorName(0), NumBlockParameters(0),
- ColonIsForRangeExpr(false), ColonIsDictLiteral(false),
- ColonIsObjCMethodExpr(false), FirstObjCSelectorName(nullptr),
- FirstStartOfName(nullptr), IsExpression(IsExpression),
- CanBeExpression(true), InTemplateArgument(false),
- InCtorInitializer(false), CaretFound(false), IsForEachMacro(false) {}
+ LongestObjCSelectorName(0), ColonIsForRangeExpr(false),
+ ColonIsDictLiteral(false), ColonIsObjCMethodExpr(false),
+ FirstObjCSelectorName(nullptr), FirstStartOfName(nullptr),
+ IsExpression(IsExpression), CanBeExpression(true),
+ InTemplateArgument(false), InCtorInitializer(false),
+ CaretFound(false), IsForEachMacro(false) {}
tok::TokenKind ContextKind;
unsigned BindingStrength;
unsigned LongestObjCSelectorName;
- unsigned NumBlockParameters;
bool ColonIsForRangeExpr;
bool ColonIsDictLiteral;
bool ColonIsObjCMethodExpr;
@@ -742,10 +749,8 @@ private:
Contexts.back().InTemplateArgument);
} else if (Current.isOneOf(tok::minus, tok::plus, tok::caret)) {
Current.Type = determinePlusMinusCaretUsage(Current);
- if (Current.Type == TT_UnaryOperator && Current.is(tok::caret)) {
- ++Contexts.back().NumBlockParameters;
+ if (Current.Type == TT_UnaryOperator && Current.is(tok::caret))
Contexts.back().CaretFound = true;
- }
} else if (Current.isOneOf(tok::minusminus, tok::plusplus)) {
Current.Type = determineIncrementUsage(Current);
} else if (Current.is(tok::exclaim)) {
Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=210097&r1=210096&r2=210097&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Tue Jun 3 07:02:45 2014
@@ -146,6 +146,39 @@ TEST_F(FormatTestJS, Closures) {
getGoogleJSStyleWithColumns(37));
}
+TEST_F(FormatTestJS, MultipleFunctionLiterals) {
+ verifyFormat("promise.then(\n"
+ " function success() {\n"
+ " doFoo();\n"
+ " doBar();\n"
+ " },\n"
+ " function error() {\n"
+ " doFoo();\n"
+ " doBaz();\n"
+ " },\n"
+ " []);\n");
+ verifyFormat("promise.then(\n"
+ " function success() {\n"
+ " doFoo();\n"
+ " doBar();\n"
+ " },\n"
+ " [],\n"
+ " function error() {\n"
+ " doFoo();\n"
+ " doBaz();\n"
+ " });\n");
+ // FIXME: Here, we should probably break right after the "(" for consistency.
+ verifyFormat("promise.then([],\n"
+ " function success() {\n"
+ " doFoo();\n"
+ " doBar();\n"
+ " },\n"
+ " function error() {\n"
+ " doFoo();\n"
+ " doBaz();\n"
+ " });\n");
+}
+
TEST_F(FormatTestJS, ReturnStatements) {
verifyFormat("function() { return [hello, world]; }");
}
More information about the cfe-commits
mailing list