From e167891bd8ce378c40f9e78051b5ffeb9dec6284 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Tue, 3 Jun 2014 12:02:45 +0000 Subject: [PATCH] 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(); }); git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@210097 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/ContinuationIndenter.cpp | 31 +++++++++++++++------------ lib/Format/ContinuationIndenter.h | 8 +++---- lib/Format/FormatToken.h | 11 +++++++--- lib/Format/TokenAnnotator.cpp | 31 +++++++++++++++------------ unittests/Format/FormatTestJS.cpp | 33 +++++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 33 deletions(-) diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp index 01467f995d..5e63d0cb9b 100644 --- a/lib/Format/ContinuationIndenter.cpp +++ b/lib/Format/ContinuationIndenter.cpp @@ -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. diff --git a/lib/Format/ContinuationIndenter.h b/lib/Format/ContinuationIndenter.h index 4733bda0a0..0969a8cec9 100644 --- a/lib/Format/ContinuationIndenter.h +++ b/lib/Format/ContinuationIndenter.h @@ -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. diff --git a/lib/Format/FormatToken.h b/lib/Format/FormatToken.h index f2bba324e6..90208b6065 100644 --- a/lib/Format/FormatToken.h +++ b/lib/Format/FormatToken.h @@ -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 Role; diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index 849dfb02f8..cbb9d7f964 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/lib/Format/TokenAnnotator.cpp @@ -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)) { diff --git a/unittests/Format/FormatTestJS.cpp b/unittests/Format/FormatTestJS.cpp index ecf4e69999..ddd594bb6e 100644 --- a/unittests/Format/FormatTestJS.cpp +++ b/unittests/Format/FormatTestJS.cpp @@ -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]; }"); } -- 2.40.0