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();
               });

llvm-svn: 210097
This commit is contained in:
Daniel Jasper 2014-06-03 12:02:45 +00:00
parent be6d91f1ba
commit 114a2bc9d2
5 changed files with 81 additions and 33 deletions

View File

@ -112,6 +112,15 @@ bool ContinuationIndenter::canBreak(const LineState &State) {
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::addTokenOnCurrentLine(LineState &State, bool DryRun,
// 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::moveStateToNextToken(LineState &State,
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& State, const FormatToken &Tok) {
// 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::moveStatePastScopeOpener(LineState &State,
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::moveStatePastScopeOpener(LineState &State,
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::moveStatePastScopeCloser(LineState &State) {
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.

View File

@ -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.

View File

@ -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;

View File

@ -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)) {

View File

@ -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]; }");
}