Index: clang/docs/ClangFormatStyleOptions.rst =================================================================== --- clang/docs/ClangFormatStyleOptions.rst +++ clang/docs/ClangFormatStyleOptions.rst @@ -1101,6 +1101,37 @@ AttributeMacros: ['__capability', '__output', '__ununsed'] +**AutomaticBraces** (``BraceInsertionStyle``) :versionbadge:`clang-format 14` + If set to ``WrapLikely`` will insert braces if the line after the + control flow statement is likely to wrap. + If set to ``Always`` will always insert braces. + If set to ``Remove`` will remove braces where possible. + The default is the disabled value of ``BIS_Leave``. + + .. warning:: + + Setting ``AutomaticBraces`` to something other than `Leave`, COULD + lead to incorrect code formatting due to incorrect decisions made due to + clang-formats lack of complete semantic information. + As such extra care should be taken to review code changes made by the use + of this option. + + Possible values: + + * ``BIS_Leave`` (in configuration: ``Leave``) + Do not insert or remove braces. + + * ``BIS_Always`` (in configuration: ``Always``) + Always insert braces. + + * ``BIS_WrapLikely`` (in configuration: ``WrapLikely``) + Insert braces if the line is likely to wrap. + + * ``BIS_Remove`` (in configuration: ``Remove``) + Always remove braces. + + + **BinPackArguments** (``Boolean``) :versionbadge:`clang-format 3.7` If ``false``, a function call's arguments will either be all on the same line or will have one line each. Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -228,6 +228,9 @@ `const` `volatile` `static` `inline` `constexpr` `restrict` to be controlled relative to the `type`. +- Option ``AutomaticBraces`` has been added to allow the insertion + and removal of {} from if/for/while scope + libclang -------- Index: clang/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -933,6 +933,72 @@ /// \version 12 std::vector AttributeMacros; + // Number of comments lines that signal the braces should + // be kept on removal . + enum AutomaticBraceCommentAllowance { + // The presense of a any number of comment means we will not + // remove the brace + ABCA_Any, + // The braces will be removed for single line comments only + ABCA_SingleLineOnly, + // The braces will be removed for only multi line comments only + ABCA_MultipleLinesOnly + }; + + enum AutomaticBracesStyle { + /// Do not insert or remove braces. + BIS_Leave = 0, + /// Always insert braces. + BIS_Always, + /// Insert braces if the line is likely to wrap. + BIS_WrapLikely, + /// Always remove braces. + BIS_Remove + }; + + /// The style of inserting braces in control flow statements. + struct AutomaticBracesOptions { + // Insert/Remove after if() + AutomaticBracesStyle AfterIf; + + // Insert/Remove after for() + AutomaticBracesStyle AfterFor; + + // Insert/Remove after while() + AutomaticBracesStyle AfterWhile; + + // Insert/Remove after do + AutomaticBracesStyle AfterDo; + + // Insert/Remove after else + AutomaticBracesStyle AfterElse; + + // Controls Removal of braces from commented scope + AutomaticBraceCommentAllowance BraceRemovalComments; + + bool operator==(const AutomaticBracesOptions &Other) const { + return AfterIf == Other.AfterIf && AfterFor == Other.AfterFor && + AfterWhile == Other.AfterWhile && AfterDo == Other.AfterDo && + AfterElse == Other.AfterElse && + BraceRemovalComments == Other.BraceRemovalComments; + } + }; + + /// If set to ``WrapLikely`` will insert braces if the line after the + /// control flow statement is likely to wrap. + /// If set to ``Always`` will always insert braces. + /// If set to ``Remove`` will remove braces where possible. + /// The default is the disabled value of ``BIS_Leave``. + /// \warning + /// Setting ``AutomaticBraces`` to something other than `Leave`, COULD + /// lead to incorrect code formatting due to incorrect decisions made due to + /// clang-formats lack of complete semantic information. + /// As such extra care should be taken to review code changes made by the use + /// of this option. + /// \endwarning + /// \version 14 + AutomaticBracesOptions AutomaticBraces; + /// If ``false``, a function call's arguments will either be all on the /// same line or will have one line each. /// \code @@ -3627,6 +3693,7 @@ AlwaysBreakTemplateDeclarations == R.AlwaysBreakTemplateDeclarations && AttributeMacros == R.AttributeMacros && + AutomaticBraces == R.AutomaticBraces && BinPackArguments == R.BinPackArguments && BinPackParameters == R.BinPackParameters && BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators && @@ -3665,16 +3732,16 @@ IndentGotoLabels == R.IndentGotoLabels && IndentPPDirectives == R.IndentPPDirectives && IndentExternBlock == R.IndentExternBlock && - IndentRequires == R.IndentRequires && IndentWidth == R.IndentWidth && - Language == R.Language && + IndentRequires == R.IndentRequires && IndentWrappedFunctionNames == R.IndentWrappedFunctionNames && + IndentWidth == R.IndentWidth && JavaImportGroups == R.JavaImportGroups && JavaScriptQuotes == R.JavaScriptQuotes && JavaScriptWrapImports == R.JavaScriptWrapImports && KeepEmptyLinesAtTheStartOfBlocks == R.KeepEmptyLinesAtTheStartOfBlocks && LambdaBodyIndentation == R.LambdaBodyIndentation && - MacroBlockBegin == R.MacroBlockBegin && + Language == R.Language && MacroBlockBegin == R.MacroBlockBegin && MacroBlockEnd == R.MacroBlockEnd && MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep && NamespaceIndentation == R.NamespaceIndentation && Index: clang/lib/Format/AutomaticBraces.h =================================================================== --- /dev/null +++ clang/lib/Format/AutomaticBraces.h @@ -0,0 +1,45 @@ +//===--- AutomaticBraces.h - Format C++ code ------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines the class for inserting braces +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_FORMAT_AUTOMATICBRACES_H +#define LLVM_CLANG_LIB_FORMAT_AUTOMATICBRACES_H + +#include "TokenAnalyzer.h" + +namespace clang { +namespace format { + +class AutomaticBraces : public TokenAnalyzer { +public: + AutomaticBraces(const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + + std::pair + analyze(TokenAnnotator &Annotator, + SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) override; + +private: + void insertBraces(SmallVectorImpl &Lines, + tooling::Replacements &Result); + + void removeBraces(SmallVectorImpl &Lines, + tooling::Replacements &Result); + + std::set Done; +}; + +} // namespace format +} // namespace clang + +#endif Index: clang/lib/Format/AutomaticBraces.cpp =================================================================== --- /dev/null +++ clang/lib/Format/AutomaticBraces.cpp @@ -0,0 +1,390 @@ +//===--- AutmaticBraces.cpp - Format C++ code -----------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file implements functions declared in Format.h. This will be +/// split into separate files as we go. +/// +//===----------------------------------------------------------------------===// + +#include "AutomaticBraces.h" +#include "clang/Format/Format.h" + +namespace clang { +namespace format { +// Are any of the options set for insertion/removal? +bool SupportsAnyAutomaticBraces(FormatStyle &Style, bool insertion) { + if (insertion) { + if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Always) + return true; + if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_WrapLikely) + return true; + if (Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Always) + return true; + if (Style.AutomaticBraces.AfterFor == FormatStyle::BIS_WrapLikely) + return true; + if (Style.AutomaticBraces.AfterWhile == FormatStyle::BIS_Always) + return true; + if (Style.AutomaticBraces.AfterWhile == FormatStyle::BIS_WrapLikely) + return true; + if (Style.AutomaticBraces.AfterDo == FormatStyle::BIS_Always) + return true; + if (Style.AutomaticBraces.AfterDo == FormatStyle::BIS_WrapLikely) + return true; + if (Style.AutomaticBraces.AfterElse == FormatStyle::BIS_Always) + return true; + if (Style.AutomaticBraces.AfterElse == FormatStyle::BIS_WrapLikely) + return true; + } else { + if (Style.AutomaticBraces.AfterIf >= FormatStyle::BIS_Remove) + return true; + if (Style.AutomaticBraces.AfterFor >= FormatStyle::BIS_Remove) + return true; + if (Style.AutomaticBraces.AfterWhile >= FormatStyle::BIS_Remove) + return true; + if (Style.AutomaticBraces.AfterDo >= FormatStyle::BIS_Remove) + return true; + if (Style.AutomaticBraces.AfterElse >= FormatStyle::BIS_Remove) + return true; + } + return false; +} + +std::pair +AutomaticBraces::analyze(TokenAnnotator &Annotator, + SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) { + tooling::Replacements Result; + if (AffectedRangeMgr.computeAffectedLines(AnnotatedLines)) { + if (SupportsAnyAutomaticBraces(Style, /*insertion=*/true)) + insertBraces(AnnotatedLines, Result); + + if (SupportsAnyAutomaticBraces(Style, /*insertion=*/false)) + removeBraces(AnnotatedLines, Result); + } + return {Result, 0}; +} + +void AutomaticBraces::insertBraces(SmallVectorImpl &Lines, + tooling::Replacements &Result) { + bool InsideCtrlFlowStmt = false; + + tok::TokenKind ControlFlowType = tok::unknown; + for (AnnotatedLine *Line : Lines) { + insertBraces(Line->Children, Result); + + // Get first token that is not a comment. + const FormatToken *FirstTok = Line->First; + if (FirstTok->is(tok::comment)) + FirstTok = FirstTok->getNextNonComment(); + // The line is a comment. + if (!FirstTok) + continue; + // Get last token that is not a comment. + const FormatToken *LastTok = Line->Last; + if (LastTok->is(tok::comment)) + LastTok = LastTok->getPreviousNonComment(); + + // If this token starts a control flow stmt, mark it. + if (FirstTok->isOneOf(tok::kw_if, tok::kw_else, tok::kw_for, tok::kw_do, + tok::kw_while)) { + ControlFlowType = FirstTok->Tok.getKind(); + InsideCtrlFlowStmt = !LastTok->isOneOf(tok::l_brace, tok::semi); + continue; + } + + // If the previous line started a ctrl flow block and this line does not + // start with curly. + if (Line->Affected && !FirstTok->Finalized && InsideCtrlFlowStmt && + FirstTok->isNot(tok::l_brace)) { + const SourceLocation startBraceLoc = Line->First->Tok.getLocation(); + // In some cases clang-format will run the same transform twice which + // could lead to adding the curlies twice, skip if we already added one + // at this location. + if (Done.count(startBraceLoc)) + break; + + bool ShouldInsert = false; + + switch (ControlFlowType) { + case tok::kw_if: + ShouldInsert = (Style.AutomaticBraces.AfterIf == + FormatStyle::AutomaticBracesStyle::BIS_Always); + break; + case tok::kw_for: + ShouldInsert = (Style.AutomaticBraces.AfterFor == + FormatStyle::AutomaticBracesStyle::BIS_Always); + break; + case tok::kw_while: + ShouldInsert = (Style.AutomaticBraces.AfterWhile == + FormatStyle::AutomaticBracesStyle::BIS_Always); + break; + case tok::kw_do: + ShouldInsert = (Style.AutomaticBraces.AfterDo == + FormatStyle::AutomaticBracesStyle::BIS_Always); + break; + case tok::kw_else: + ShouldInsert = (Style.AutomaticBraces.AfterElse == + FormatStyle::AutomaticBracesStyle::BIS_Always); + break; + default: + ShouldInsert = false; + break; + } + + if (ShouldInsert || Line->Last->OriginalColumn > Style.ColumnLimit) { + Done.insert(startBraceLoc); + cantFail(Result.add(tooling::Replacement(Env.getSourceManager(), + startBraceLoc, 0, "{"))); + + // Mostly we can ignore putting a \n before the } and let the + // Formatter handle it, unless the last token is a // comment, Doing + // so avoids issues with macro continuation. And a line comment + // cannot be part of a continuation. + std::string ending = "}"; + if (Line->Last->is(tok::comment)) + ending = "\n}"; + cantFail(Result.add( + tooling::Replacement(Env.getSourceManager(), + Line->Last->Tok.getLocation().getLocWithOffset( + Line->Last->TokenText.size()), + 0, ending))); + } + } + InsideCtrlFlowStmt = false; + } +} + +// Class that allows getting Next over multiple annotated lines +class NextTokenFetcher { + SmallVectorImpl &Lines; + +public: + NextTokenFetcher(SmallVectorImpl &Lines) : Lines(Lines) {} + + const FormatToken *getNext(int &Line, const FormatToken *current) { + if (current == nullptr) + return Lines[0]->First; + + if (current && current->Next) + return current->Next; + + // I'm at the end so I can't take the next + if (current && !current->Next) { + // I'm at the end of all the lines + if (current && Lines[Lines.size() - 1]->Last == current) + return nullptr; + + for (int NLine = 0; NLine < Lines.size(); NLine++) { + FormatToken *tok = Lines[NLine]->Last; + if (current == tok) { + // if I'm at the end of a line then the next is the + // First of the next line + return Lines[NLine + 1]->First; + } + } + return nullptr; + } + + if (current && !current->Next && Line < (Lines.size() - 1)) { + Line++; + return Lines[Line]->First; + } + return current->Next; + } + + bool startsSequence(int LineNumber, const FormatToken *RBrace, + tok::TokenKind tok1, tok::TokenKind tok2, + tok::TokenKind tok3, tok::TokenKind tok4) { + + if (!RBrace) + return false; + + if (!RBrace->is(tok1)) + return false; + const FormatToken *Next = getNext(LineNumber, RBrace); + if (!Next->is(tok2)) + return false; + Next = getNext(LineNumber, Next); + if (!Next->is(tok3)) + return false; + Next = getNext(LineNumber, Next); + if (!Next->is(tok4)) + return false; + return true; + } +}; + +// Scan thorough tokens looking for if/for/while etc... +// find matching () e.g. if(), for(), while() +// if { follows then find matching } +// count number of ; between { ... } +// if number of ; > 1 leave braces +// if number of ; <= 1 remove braces +// if {} contain "if" or { before seeing } then don't attempt to +// remove (This could leads to some false negatives) +void AutomaticBraces::removeBraces(SmallVectorImpl &Lines, + tooling::Replacements &Result) { + + NextTokenFetcher fetcher(Lines); + int LineNumber = 0; + + const FormatToken *NextToken = fetcher.getNext(LineNumber, nullptr); + while (NextToken) { + const FormatToken *StartingToken = NextToken; + NextToken = fetcher.getNext(LineNumber, NextToken); + + // Does the line start with one of the tokens we are expecting to + // remove a brace from + if (!StartingToken->isOneOf(tok::kw_if, tok::kw_for, tok::kw_else, + tok::kw_while, tok::kw_do)) + continue; + + const FormatToken *Next = fetcher.getNext(LineNumber, StartingToken); + if (!Next) { + // We are are the end of the annotated lines. + continue; + } + + // if/for and while are handled a little differently + if (StartingToken->isOneOf(tok::kw_if, tok::kw_for, tok::kw_while)) { + if (!Next->is(tok::l_paren) || !Next->MatchingParen) + continue; + FormatToken *RParen = Next->MatchingParen; + const FormatToken *RParenNext = fetcher.getNext(LineNumber, RParen); + if (!RParenNext) + continue; + Next = RParen; + } else if (StartingToken->isOneOf(tok::kw_else, tok::kw_do)) { + // If we are an `else` or `do` then simple move onto removing the braces. + Next = StartingToken; + } + + const FormatToken *LBrace = fetcher.getNext(LineNumber, Next); + if (!LBrace) + continue; + + // We could be `while(x);` or `else if` just continue from + // this point if we are. + if (LBrace && LBrace->isOneOf(tok::semi, tok::kw_if)) { + NextToken = LBrace; + continue; + } + + if (LBrace && !LBrace->is(tok::l_brace)) { + NextToken = LBrace; + LBrace = nullptr; + continue; + } + + // If any single comment stops the removal of braces + if (LBrace->startsSequence(tok::l_brace, tok::comment)) { + if (Style.AutomaticBraces.BraceRemovalComments == + FormatStyle::AutomaticBraceCommentAllowance::ABCA_Any) { + NextToken = LBrace; + continue; + } + } + // If any multiple comments stops the removal of braces + if (LBrace->startsSequence(tok::l_brace, tok::comment, tok::comment)) { + if (Style.AutomaticBraces.BraceRemovalComments == + FormatStyle::AutomaticBraceCommentAllowance::ABCA_SingleLineOnly) { + NextToken = LBrace; + continue; + } + } + + // Shoot along until we get to the end brace, but count + // the number of functional lines by counting semi's + const FormatToken *InnerScopeToken = LBrace; + + int CountSemi = 0; + int ScopeInducingTokens = 0; + int Scope = 0; + while (InnerScopeToken) { + InnerScopeToken = fetcher.getNext(LineNumber, InnerScopeToken); + if (InnerScopeToken->is(tok::l_brace)) + Scope++; + if (InnerScopeToken->is(tok::r_brace)) { + if (Scope == 0) + break; + Scope--; + } + if (InnerScopeToken->is(tok::semi)) + CountSemi++; + if (InnerScopeToken->is(tok::comment) && + Style.AutomaticBraces.BraceRemovalComments == + FormatStyle::AutomaticBraceCommentAllowance::ABCA_Any) { + LBrace = nullptr; + break; + } + // This can cause a dangling if + if (InnerScopeToken->is(tok::kw_if)) { + NextToken = InnerScopeToken; + ScopeInducingTokens++; + break; + } + if (InnerScopeToken->is(tok::kw_for)) { + ScopeInducingTokens++; + continue; + } + } + + // We should now be at the end of at an }. + const FormatToken *RBrace = InnerScopeToken; + if (!RBrace || !RBrace->is(tok::r_brace)) + continue; + + // Too many lines. + if (CountSemi != 1) { + NextToken = RBrace; + continue; + } + + // Too many levels of scope + if (ScopeInducingTokens >= 2) { + // Move back to the inner scope. + NextToken = LBrace; + continue; + } + + // Don't remove the braces from if(){} if the else already has braces. + if (fetcher.startsSequence(LineNumber, RBrace, tok::r_brace, tok::kw_else, + tok::l_brace, tok::comment)) { + RBrace = nullptr; + LBrace = nullptr; + continue; + } + + if (LBrace && RBrace) { + // We are finally at the point where we know the positions of {} + // And we think this is for just a single line of scope. + const SourceLocation startBraceLoc = LBrace->Tok.getLocation(); + const SourceLocation endBraceLoc = RBrace->Tok.getLocation(); + + const char *text = Env.getSourceManager().getCharacterData( + endBraceLoc.getLocWithOffset(1)); + + // Determine if we need to remove the "}\n" or just "}". + int endSize = 1; + if (text && text[0] == '\n') + endSize++; + + // Remove the { and } (or }\n). + cantFail(Result.add( + tooling::Replacement(Env.getSourceManager(), startBraceLoc, 1, ""))); + cantFail(Result.add(tooling::Replacement(Env.getSourceManager(), + endBraceLoc, endSize, ""))); + // Move on from where we got to. + } + NextToken = LBrace; + } +} + +} // namespace format +} // namespace clang Index: clang/lib/Format/CMakeLists.txt =================================================================== --- clang/lib/Format/CMakeLists.txt +++ clang/lib/Format/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangFormat AffectedRangeManager.cpp BreakableToken.cpp + AutomaticBraces.cpp ContinuationIndenter.cpp Format.cpp FormatToken.cpp Index: clang/lib/Format/Format.cpp =================================================================== --- clang/lib/Format/Format.cpp +++ clang/lib/Format/Format.cpp @@ -14,6 +14,7 @@ #include "clang/Format/Format.h" #include "AffectedRangeManager.h" +#include "AutomaticBraces.h" #include "BreakableToken.h" #include "ContinuationIndenter.h" #include "FormatInternal.h" @@ -214,6 +215,15 @@ } }; +template <> struct ScalarEnumerationTraits { + static void enumeration(IO &IO, FormatStyle::AutomaticBracesStyle &Value) { + IO.enumCase(Value, "Leave", FormatStyle::BIS_Leave); + IO.enumCase(Value, "Always", FormatStyle::BIS_Always); + IO.enumCase(Value, "WrapLikely", FormatStyle::BIS_WrapLikely); + IO.enumCase(Value, "Remove", FormatStyle::BIS_Remove); + } +}; + template <> struct ScalarEnumerationTraits { static void enumeration(IO &IO, FormatStyle::BinaryOperatorStyle &Value) { IO.enumCase(Value, "All", FormatStyle::BOS_All); @@ -613,6 +623,7 @@ IO.mapOptional("AlwaysBreakTemplateDeclarations", Style.AlwaysBreakTemplateDeclarations); IO.mapOptional("AttributeMacros", Style.AttributeMacros); + IO.mapOptional("AutomaticBraces", Style.AutomaticBraces); IO.mapOptional("BinPackArguments", Style.BinPackArguments); IO.mapOptional("BinPackParameters", Style.BinPackParameters); IO.mapOptional("BraceWrapping", Style.BraceWrapping); @@ -655,13 +666,12 @@ IO.mapOptional("QualifierAlignment", Style.QualifierAlignment); // Default Order for Left/Right based Qualifier alignment. - if (Style.QualifierAlignment == FormatStyle::QAS_Right) { + if (Style.QualifierAlignment == FormatStyle::QAS_Right) Style.QualifierOrder = {"type", "const", "volatile"}; - } else if (Style.QualifierAlignment == FormatStyle::QAS_Left) { + else if (Style.QualifierAlignment == FormatStyle::QAS_Left) Style.QualifierOrder = {"const", "volatile", "type"}; - } else if (Style.QualifierAlignment == FormatStyle::QAS_Custom) { + else if (Style.QualifierAlignment == FormatStyle::QAS_Custom) IO.mapOptional("QualifierOrder", Style.QualifierOrder); - } IO.mapOptional("CompactNamespaces", Style.CompactNamespaces); IO.mapOptional("ConstructorInitializerIndentWidth", @@ -699,11 +709,10 @@ IO.mapOptional("AllowAllConstructorInitializersOnNextLine", OnNextLine); if (!IsGoogleOrChromium) { if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack && - OnCurrentLine) { + OnCurrentLine) Style.PackConstructorInitializers = OnNextLine ? FormatStyle::PCIS_NextLine : FormatStyle::PCIS_CurrentLine; - } } else if (Style.PackConstructorInitializers == FormatStyle::PCIS_NextLine) { if (!OnCurrentLine) @@ -822,6 +831,27 @@ } }; +template <> struct MappingTraits { + static void mapping(IO &IO, FormatStyle::AutomaticBracesOptions &Style) { + IO.mapOptional("AfterIf", Style.AfterIf); + IO.mapOptional("AfterFor", Style.AfterFor); + IO.mapOptional("AfterWhile", Style.AfterWhile); + IO.mapOptional("AfterDo", Style.AfterDo); + IO.mapOptional("AfterElse", Style.AfterElse); + IO.mapOptional("BraceRemovalComments", Style.BraceRemovalComments); + } +}; + +template <> +struct ScalarEnumerationTraits { + static void enumeration(IO &IO, + FormatStyle::AutomaticBraceCommentAllowance &Value) { + IO.enumCase(Value, "None", FormatStyle::ABCA_Any); + IO.enumCase(Value, "Single", FormatStyle::ABCA_SingleLineOnly); + IO.enumCase(Value, "Multiple", FormatStyle::ABCA_MultipleLinesOnly); + } +}; + template <> struct MappingTraits { static void mapping(IO &IO, FormatStyle::BraceWrappingFlags &Wrapping) { IO.mapOptional("AfterCaseLabel", Wrapping.AfterCaseLabel); @@ -863,9 +893,8 @@ IO.mapOptional("Maximum", signedMaximum); Space.Maximum = static_cast(signedMaximum); - if (Space.Maximum != -1u) { + if (Space.Maximum != -1u) Space.Minimum = std::min(Space.Minimum, Space.Maximum); - } } }; @@ -883,9 +912,9 @@ if (Index >= Seq.size()) { assert(Index == Seq.size()); FormatStyle Template; - if (!Seq.empty() && Seq[0].Language == FormatStyle::LK_None) { + if (!Seq.empty() && Seq[0].Language == FormatStyle::LK_None) Template = Seq[0]; - } else { + else { Template = *((const FormatStyle *)IO.getContext()); Template.Language = FormatStyle::LK_None; } @@ -1137,6 +1166,13 @@ LLVMStyle.IndentRequires = false; LLVMStyle.IndentWrappedFunctionNames = false; LLVMStyle.IndentWidth = 2; + LLVMStyle.AutomaticBraces = {/*AfterIf=*/FormatStyle::BIS_Leave, + /*AfterFor=*/FormatStyle::BIS_Leave, + /*AfterWhile=*/FormatStyle::BIS_Leave, + /*AfterDo=*/FormatStyle::BIS_Leave, + /*AfterElse=*/FormatStyle::BIS_Leave, + /*BraceRemovalComments*/ FormatStyle::ABCA_Any}; + LLVMStyle.PPIndentWidth = -1; LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None; LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave; @@ -1206,12 +1242,10 @@ LLVMStyle.WhitespaceSensitiveMacros.push_back("CF_SWIFT_NAME"); // Defaults that differ when not C++. - if (Language == FormatStyle::LK_TableGen) { + if (Language == FormatStyle::LK_TableGen) LLVMStyle.SpacesInContainerLiterals = false; - } - if (LLVMStyle.isJson()) { + if (LLVMStyle.isJson()) LLVMStyle.ColumnLimit = 0; - } return LLVMStyle; } @@ -1520,27 +1554,26 @@ bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language, FormatStyle *Style) { - if (Name.equals_insensitive("llvm")) { + if (Name.equals_insensitive("llvm")) *Style = getLLVMStyle(Language); - } else if (Name.equals_insensitive("chromium")) { + else if (Name.equals_insensitive("chromium")) *Style = getChromiumStyle(Language); - } else if (Name.equals_insensitive("mozilla")) { + else if (Name.equals_insensitive("mozilla")) *Style = getMozillaStyle(); - } else if (Name.equals_insensitive("google")) { + else if (Name.equals_insensitive("google")) *Style = getGoogleStyle(Language); - } else if (Name.equals_insensitive("webkit")) { + else if (Name.equals_insensitive("webkit")) *Style = getWebKitStyle(); - } else if (Name.equals_insensitive("gnu")) { + else if (Name.equals_insensitive("gnu")) *Style = getGNUStyle(); - } else if (Name.equals_insensitive("microsoft")) { + else if (Name.equals_insensitive("microsoft")) *Style = getMicrosoftStyle(Language); - } else if (Name.equals_insensitive("none")) { + else if (Name.equals_insensitive("none")) *Style = getNoStyle(); - } else if (Name.equals_insensitive("inheritparentconfig")) { + else if (Name.equals_insensitive("inheritparentconfig")) Style->InheritsParentConfig = true; - } else { + else return false; - } Style->Language = Language; return true; @@ -1786,9 +1819,8 @@ tooling::Replacements Result; deriveLocalStyle(AnnotatedLines); AffectedRangeMgr.computeAffectedLines(AnnotatedLines); - for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { + for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) Annotator.calculateFormattingInformation(*AnnotatedLines[i]); - } Annotator.setCommentLineLevels(AnnotatedLines); WhitespaceManager Whitespaces( @@ -1998,9 +2030,8 @@ private: void cleanupLine(AnnotatedLine *Line) { - for (auto *Child : Line->Children) { + for (auto *Child : Line->Children) cleanupLine(Child); - } if (Line->Affected) { cleanupRight(Line->First, tok::comma, tok::comma); @@ -2026,9 +2057,8 @@ std::set DeletedLines; for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { auto &Line = *AnnotatedLines[i]; - if (Line.startsWithNamespace()) { + if (Line.startsWithNamespace()) checkEmptyNamespace(AnnotatedLines, i, i, DeletedLines); - } } for (auto Line : DeletedLines) { @@ -2056,9 +2086,8 @@ NewLine = CurrentLine; return false; } - } else if (!AnnotatedLines[CurrentLine]->endsWith(tok::l_brace)) { + } else if (!AnnotatedLines[CurrentLine]->endsWith(tok::l_brace)) return false; - } while (++CurrentLine < End) { if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace)) break; @@ -2090,9 +2119,8 @@ AnnotatedLines[CurrentLine]->Last->Tok.getEndLoc()))) return false; - for (unsigned i = InitLine; i <= CurrentLine; ++i) { + for (unsigned i = InitLine; i <= CurrentLine; ++i) DeletedLines.insert(i); - } return true; } @@ -2157,9 +2185,8 @@ while (Idx < Tokens.size()) { unsigned St = Idx, End = Idx; while ((End + 1) < Tokens.size() && - Tokens[End]->Next == Tokens[End + 1]) { + Tokens[End]->Next == Tokens[End + 1]) End++; - } auto SR = CharSourceRange::getCharRange(Tokens[St]->Tok.getLocation(), Tokens[End]->Tok.getEndLoc()); auto Err = @@ -2428,11 +2455,10 @@ if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset)) return; SmallVector Indices; - for (unsigned i = 0, e = Includes.size(); i != e; ++i) { + for (unsigned i = 0, e = Includes.size(); i != e; ++i) Indices.push_back(i); - } - if (Style.SortIncludes == FormatStyle::SI_CaseInsensitive) { + if (Style.SortIncludes == FormatStyle::SI_CaseInsensitive) llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) { const auto LHSFilenameLower = Includes[LHSI].Filename.lower(); const auto RHSFilenameLower = Includes[RHSI].Filename.lower(); @@ -2441,12 +2467,11 @@ std::tie(Includes[RHSI].Priority, RHSFilenameLower, Includes[RHSI].Filename); }); - } else { + else llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) { return std::tie(Includes[LHSI].Priority, Includes[LHSI].Filename) < std::tie(Includes[RHSI].Priority, Includes[RHSI].Filename); }); - } // The index of the include on which the cursor will be put after // sorting/deduplicating. @@ -2588,10 +2613,9 @@ Prev = Pos + 1; SearchFrom = Pos + 1; } - if (!IncludesInBlock.empty()) { + if (!IncludesInBlock.empty()) sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code, Replaces, Cursor); - } return Replaces; } @@ -2729,9 +2753,8 @@ StringRef Static = Matches[1]; StringRef Identifier = Matches[2]; bool IsStatic = false; - if (Static.contains("static")) { + if (Static.contains("static")) IsStatic = true; - } ImportsInBlock.push_back( {Identifier, Line, Prev, AssociatedCommentLines, IsStatic}); AssociatedCommentLines.clear(); @@ -2849,15 +2872,14 @@ // Replacements from \p Replaces must be conflict-free already, so we can // simply consume the error. llvm::consumeError(HeaderInsertions.add(R)); - } else if (isHeaderDeletion(R)) { + } else if (isHeaderDeletion(R)) HeadersToDelete.insert(R.getReplacementText()); - } else if (R.getOffset() == UINT_MAX) { + else if (R.getOffset() == UINT_MAX) llvm::errs() << "Insertions other than header #include insertion are " "not supported! " << R.getReplacementText() << "\n"; - } else { + else llvm::consumeError(Result.add(R)); - } } if (HeaderInsertions.empty() && HeadersToDelete.empty()) return Replaces; @@ -2949,9 +2971,8 @@ // add a replacement to remove the "x = " from the result. if (!Replaces.add(tooling::Replacement(FileName, 0, 4, ""))) { // apply the reformatting changes and the removal of "x = ". - if (applyAllReplacements(Code, Replaces)) { + if (applyAllReplacements(Code, Replaces)) return {Replaces, 0}; - } } return {tooling::Replacements(), 0}; } @@ -2961,14 +2982,13 @@ AnalyzerPass; SmallVector Passes; - if (Style.isCpp() && Style.QualifierAlignment != FormatStyle::QAS_Leave) { + if (Style.isCpp() && Style.QualifierAlignment != FormatStyle::QAS_Leave) Passes.emplace_back([&](const Environment &Env) { return QualifierAlignmentFixer(Env, Expanded, Code, Ranges, FirstStartColumn, NextStartColumn, LastStartColumn, FileName) .process(); }); - } if (Style.Language == FormatStyle::LK_Cpp) { if (Style.FixNamespaceComments) @@ -2982,6 +3002,10 @@ }); } + Passes.emplace_back([&](const Environment &Env) { + return AutomaticBraces(Env, Expanded).process(); + }); + if (Style.Language == FormatStyle::LK_JavaScript && Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) Passes.emplace_back([&](const Environment &Env) { @@ -3162,9 +3186,8 @@ StringRef FallbackStyleName, StringRef Code, llvm::vfs::FileSystem *FS, bool AllowUnknownOptions) { - if (!FS) { + if (!FS) FS = llvm::vfs::getRealFileSystem().get(); - } FormatStyle Style = getLLVMStyle(guessLanguage(FileName, Code)); FormatStyle FallbackStyle = getNoStyle(); @@ -3218,9 +3241,8 @@ auto Status = FS->status(Directory); if (!Status || - Status->getType() != llvm::sys::fs::file_type::directory_file) { + Status->getType() != llvm::sys::fs::file_type::directory_file) continue; - } for (const auto &F : FilesToLookFor) { SmallString<128> ConfigFile(Directory); Index: clang/unittests/Format/AutomaticBracesTest.cpp =================================================================== --- /dev/null +++ clang/unittests/Format/AutomaticBracesTest.cpp @@ -0,0 +1,810 @@ +//===- unittest/Format/AutomaticBracesTest.cpp - brace insertion unit tests +//-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Format/Format.h" + +#include "../Tooling/ReplacementTest.h" +#include "FormatTestUtils.h" + +#include "llvm/Support/Debug.h" +#include "llvm/Support/MemoryBuffer.h" +#include "gtest/gtest.h" + +#define DEBUG_TYPE "automatic-braces-test" + +using clang::tooling::ReplacementTest; +using clang::tooling::toReplacements; +using testing::ScopedTrace; + +namespace clang { +namespace format { +namespace { + +class AutomaticBracesTest : public ::testing::Test { +protected: + enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck }; + + std::string format(llvm::StringRef Code, + const FormatStyle &Style = getLLVMStyle(), + StatusCheck CheckComplete = SC_ExpectComplete) { + LLVM_DEBUG(llvm::errs() << "---\n"); + LLVM_DEBUG(llvm::errs() << Code << "\n\n"); + std::vector Ranges(1, tooling::Range(0, Code.size())); + FormattingAttemptStatus Status; + tooling::Replacements Replaces = + reformat(Style, Code, Ranges, "", &Status); + if (CheckComplete != SC_DoNotCheck) { + bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete; + EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete) + << Code << "\n\n"; + } + auto Result = applyAllReplacements(Code, Replaces); + EXPECT_TRUE(static_cast(Result)); + LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; + } + + FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) { + Style.ColumnLimit = ColumnLimit; + return Style; + } + + FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { + return getStyleWithColumns(getLLVMStyle(), ColumnLimit); + } + + void _verifyFormat(const char *File, int Line, llvm::StringRef Expected, + llvm::StringRef Code, + const FormatStyle &Style = getLLVMStyle()) { + ScopedTrace t(File, Line, ::testing::Message() << Code.str()); + EXPECT_EQ(Expected.str(), format(Expected, Style)) + << "Expected code is not stable"; + EXPECT_EQ(Expected.str(), format(Code, Style)); + } + + void _verifyFormat(const char *File, int Line, llvm::StringRef Code, + const FormatStyle &Style = getLLVMStyle()) { + _verifyFormat(File, Line, Code, test::messUp(Code), Style); + } +}; + +#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__) + +TEST_F(AutomaticBracesTest, InsertBraces) { + FormatStyle Style = getLLVMStyle(); + + StringRef IfSourceShort = "if (condition)\n" + " call_function(arg, arg);"; + verifyFormat(IfSourceShort); + + StringRef IfSourceLong = "if (condition)\n" + " call_function(arg, arg, arg, arg, arg, arg);"; + verifyFormat(IfSourceLong); + + StringRef IfElseSourceShort = "if (condition)\n" + " a_function(arg, arg);\n" + "else\n" + " another_function(arg, arg);"; + verifyFormat(IfElseSourceShort); + + StringRef IfElseSourceLong = + "if (condition)\n" + " a_function(arg, arg, arg, arg, arg, arg);\n" + "else\n" + " another_function(arg, arg, arg, arg, arg, arg);"; + verifyFormat(IfElseSourceLong); + + StringRef ForSourceShort = "for (auto val : container)\n" + " call_function(arg, arg);"; + verifyFormat(ForSourceShort); + + StringRef ForSourceLong = "for (auto val : container)\n" + " call_function(arg, arg, arg, arg, arg, arg);"; + verifyFormat(ForSourceLong); + + StringRef WhileShort = "while (condition)\n" + " call_function(arg, arg);"; + verifyFormat(WhileShort); + + StringRef WhileLong = "while (condition)\n" + " call_function(arg, arg, arg, arg, arg, arg);"; + verifyFormat(WhileLong); + + StringRef DoShort = "do\n" + " call_function(arg, arg);\n" + "while (condition);"; + verifyFormat(DoShort); + + StringRef DoLong = "do\n" + " call_function(arg, arg, arg, arg, arg, arg);\n" + "while (condition);"; + verifyFormat(DoLong); + + StringRef ChainedConditionals = "if (A)\n" + " if (B)\n" + " callAB();\n" + " else\n" + " callA();\n" + "else if (B)\n" + " callB();\n" + "else\n" + " call();"; + verifyFormat(ChainedConditionals); + + StringRef ChainedDoWhiles = "do\n" + " while (arg++)\n" + " do\n" + " while (arg++)\n" + " call_function(arg, arg);\n" + " while (condition);\n" + "while (condition);"; + verifyFormat(ChainedDoWhiles); + + StringRef IfWithMultilineComment = + "if /*condition*/ (condition) /*condition*/\n" + " you_do_you(); /*condition*/"; + verifyFormat(IfWithMultilineComment); + + StringRef IfSinglelineCommentOnConditional = "if (condition) // my test\n" + " you_do_you();"; + verifyFormat(IfSinglelineCommentOnConditional); + + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterDo = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterElse = FormatStyle::BIS_Always; + + verifyFormat("while (condition) {\n" + " call_function(arg, arg);\n" + "}", + WhileShort, Style); + + verifyFormat("while (condition) {\n" + " call_function(arg, arg, arg, arg, arg, arg);\n" + "}", + WhileLong, Style); + + verifyFormat("do {\n" + " call_function(arg, arg);\n" + "} while (condition);", + DoShort, Style); + + verifyFormat("do {\n" + " call_function(arg, arg, arg, arg, arg, arg);\n" + "} while (condition);", + DoLong, Style); + + verifyFormat("if (condition) {\n" + " call_function(arg, arg);\n" + "}", + IfSourceShort, Style); + + verifyFormat("if (condition) {\n" + " call_function(arg, arg);\n" + "}", + IfSourceShort, Style); + + verifyFormat("if (condition) {\n" + " call_function(arg, arg, arg, arg, arg, arg);\n" + "}", + IfSourceLong, Style); + + verifyFormat("if (condition) {\n" + " a_function(arg, arg);\n" + "} else {\n" + " another_function(arg, arg);\n" + "}", + IfElseSourceShort, Style); + + verifyFormat("if (condition) {\n" + " a_function(arg, arg, arg, arg, arg, arg);\n" + "} else {\n" + " another_function(arg, arg, arg, arg, arg, arg);\n" + "}", + IfElseSourceLong, Style); + + verifyFormat("for (auto val : container) {\n" + " call_function(arg, arg);\n" + "}", + ForSourceShort, Style); + + verifyFormat("for (auto val : container) {\n" + " call_function(arg, arg, arg, arg, arg, arg);\n" + "}", + ForSourceLong, Style); + + verifyFormat("if (A)\n" + " if (B) {\n" + " callAB();\n" + " } else {\n" + " callA();\n" + " }\n" + "else if (B) {\n" + " callB();\n" + "} else {\n" + " call();\n" + "}", + ChainedConditionals, Style); + + verifyFormat("do\n" + " while (arg++)\n" + " do\n" + " while (arg++) {\n" + " call_function(arg, arg);\n" + " }\n" + " while (condition);\n" + "while (condition);", + ChainedDoWhiles, Style); + + verifyFormat("if /*condition*/ (condition) /*condition*/\n" + "{\n" + " you_do_you(); /*condition*/\n" + "}", + IfWithMultilineComment, Style); + + verifyFormat("if (condition) // my test\n" + "{\n" + " you_do_you();\n" + "}", + IfSinglelineCommentOnConditional, Style); + + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_WrapLikely; + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_WrapLikely; + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_WrapLikely; + Style.AutomaticBraces.AfterDo = FormatStyle::BIS_WrapLikely; + Style.AutomaticBraces.AfterElse = FormatStyle::BIS_WrapLikely; + Style.ColumnLimit = 35; + + verifyFormat(IfSourceShort, IfSourceShort, Style); + + verifyFormat("if (condition) {\n" + " call_function(arg, arg, arg, arg,\n" + " arg, arg);\n" + "}", + IfSourceLong, Style); + + verifyFormat(IfElseSourceShort, IfElseSourceShort, Style); + + verifyFormat("if (condition) {\n" + " a_function(arg, arg, arg, arg,\n" + " arg, arg);\n" + "} else {\n" + " another_function(arg, arg, arg,\n" + " arg, arg, arg);\n" + "}", + IfElseSourceLong, Style); + + verifyFormat(ForSourceShort, ForSourceShort, Style); + + verifyFormat("for (auto val : container) {\n" + " call_function(arg, arg, arg, arg,\n" + " arg, arg);\n" + "}", + ForSourceLong, Style); + + format("while (foo) ", Style); +} + +TEST_F(AutomaticBracesTest, InsertBracesMacro) { + auto Style = getLLVMStyleWithColumns(20); + + verifyFormat("#define FOO1(x) \\\n" + " if (x) \\\n" + " return true; \\\n" + " else \\\n" + " return false;", + Style); + + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterDo = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterElse = FormatStyle::BIS_Always; + + verifyFormat("#define FOO2(x) \\\n" + " if (x) { \\\n" + " return true; \\\n" + " } else { \\\n" + " return false; \\\n" + " }", + "#define FOO2(x) \\\n" + " if (x) \\\n" + " return true; \\\n" + " else \\\n" + " return false;", + Style); + + verifyFormat("#define FOO3(x) \\\n" + " if (x) { \\\n" + " return true; \\\n" + " } else { \\\n" + " return false; \\\n" + " }", + Style); +} + +TEST_F(AutomaticBracesTest, InsertBracesComments) { + auto Style = getLLVMStyle(); + + verifyFormat("if (x)\n" + " return true;\n" + "else\n" + " return false;", + Style); + + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterDo = FormatStyle::BIS_Always; + Style.AutomaticBraces.AfterElse = FormatStyle::BIS_Always; + + verifyFormat("if (x) {\n" + " return true; // Comment\n" + "} else {\n" + " return false; // Comment\n" + "}", + "if (x)\n" + " return true; // Comment\n" + "else\n" + " return false; // Comment", + Style); +} + +TEST_F(AutomaticBracesTest, RemoveBraces) { + auto Style = getLLVMStyle(); + + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterDo = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterElse = FormatStyle::BIS_Remove; + + verifyFormat("if (x)\n" + " return true;\n", + "if (x) {\n" + " return true;\n" + "}", + Style); + + verifyFormat("while (x)\n" + " foo();\n", + "while (x) {\n" + " foo();\n" + "}", + Style); + + verifyFormat("if (x)\n" + " return true;\n" + "else\n" + " return false;\n", + "if (x) {\n" + " return true;\n" + "} else {\n" + " return false;\n" + "}\n", + Style); + + verifyFormat("if (x)\n" + " return true;\n" + "else if (x == 1)\n" + " return false;\n", + "if (x) {\n" + " return true;\n" + "} else if (x == 1) {\n" + " return false;\n" + "}\n", + Style); + + verifyFormat("if (x) {\n" + " foo();\n" + " bar();\n" + "} else if (x == 1)\n" + " return false;\n", + "if (x) {\n" + " foo();\n" + " bar();\n" + "} else if (x == 1) {\n" + " return false;\n" + "}\n", + Style); + + verifyFormat("if (x)\n" + " foo();\n" + "else if (x == 1) {\n" + " baz();\n" + " bar();\n" + "}\n", + "if (x) {\n" + " foo();\n" + "} else if (x == 1) {\n" + " baz();\n" + " bar();\n" + "}\n", + Style); + + verifyFormat("if (x) {\n" + " if (y)\n" + " foo();\n" + "} else {\n" + " baz();\n" + " bar();\n" + "}\n", + Style); + + verifyFormat("if (a)\n" + " b;\n" + "else if (c)\n" + " d;\n" + "else\n" + " e;\n", + "if (a) {\n" + " b;\n" + "} else if (c) {\n" + " d;\n" + "} else {\n" + " e;\n" + "}\n", + Style); + + verifyFormat("if (a) {\n" + " if (b)\n" + " c;\n" + "} else if (d)\n" + " e;\n", + "if (a) {\n" + " if (b) {\n" + " c;\n" + " }\n" + "} else if (d) {\n" + " e;\n" + "}", + Style); +} + +TEST_F(AutomaticBracesTest, RemoveLLVMBraces) { + auto Style = getLLVMStyle(); + + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterElse = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterDo = FormatStyle::BIS_Remove; + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_Remove; + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_Any; + + // Omit the braces, since the body is simple and clearly associated with the + // if. + verifyFormat("if (isa(D))\n" + " foo(D);\n" + "else if (isa(D))\n" + " bar(D);\n", + Style); + + verifyFormat("if (isa(D))\n" + " handleFunctionDecl(D);\n" + "else if (isa(D))\n" + " handleVarDecl(D);\n", + "if (isa(D)) {\n" + " handleFunctionDecl(D);\n" + "} else if (isa(D)) {\n" + " handleVarDecl(D);\n" + "}", + Style); + + verifyFormat("while (isa(D))\n" + " handleFunctionDecl(D);\n", + Style); + + verifyFormat("while (isa(D))\n" + " handleFunctionDecl(D);\n", + "while (isa(D)) {\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + verifyFormat("while (isa(D)) {\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + verifyFormat("do\n" + " handleFunctionDecl(D);\n" + "while (true);", + Style); + + verifyFormat("do\n" + " handleFunctionDecl(D);\n" + "while (true);", + "do {\n" + " handleFunctionDecl(D);\n" + "} while (true);", + Style); + + verifyFormat("do {\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n" + "} while (true);", + Style); + + verifyFormat("for (auto i : list)\n" + " handleFunctionDecl(D);\n", + Style); + + verifyFormat("for (auto i : list)\n" + " handleFunctionDecl(D);\n", + "for(auto i : list) {\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + verifyFormat("for (auto i : list) {\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + // Here we document the condition itself and not the body. + verifyFormat( + "if (isa(D)) {\n" + " // It is necessary that we explain the situation with this " + "surprisingly long\n" + " // comment, so it would be unclear without the braces whether the " + "following\n" + " // statement is in the scope of the `if`.\n" + " // Because the condition is documented, we can't really hoist this\n" + " // comment that applies to the body above the if.\n" + " handleOtherDecl(D);\n" + "}", + Style); + + // Use braces on the outer `if` to avoid a potential dangling else situation. + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " if (shouldProcessAttr(A))\n" + " handleAttr(A);\n" + "}\n", + Style); + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " if (shouldProcessAttr(A))\n" + " handleAttr(A);\n" + " }\n" + "}\n", + "if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " if (shouldProcessAttr(A)) {\n" + " handleAttr(A);\n" + " }\n" + " }\n" + "}\n", + Style); + + // if the for or if have {} here should they be removed? + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " if (shouldProcessAttr(A))\n" + " handleAttr(A);\n" + "}\n", + "if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " if (shouldProcessAttr(A))\n" + " handleAttr(A);\n" + "}\n", + Style); + + // Use braces for the `if` block to keep it uniform with the else block. + verifyFormat( + "if (isa(D)) {\n" + " handleFunctionDecl(D);\n" + "} else {\n" + " // In this else case, it is necessary that we explain the situation " + "with\n" + " // this surprisingly long comment, so it would be unclear without the " + "braces\n" + " // whether the following statement is in the scope of the `if`.\n" + " handleOtherDecl(D);\n" + "}\n", + Style); + + // This should also omit braces. The `for` loop contains only a single + // statement, so it shouldn't have braces. The `if` also only contains a + // single simple statement (the for loop), so it also should omit braces. + verifyFormat("if (isa(D))\n" + " for (auto *A : D.attrs())\n" + " handleAttr(A);\n", + Style); + + verifyFormat("if (isa(D))\n" + " for (auto *A : D.attrs())\n" + " handleAttr(A);\n", + "if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " handleAttr(A);\n" + " }\n" + "}", + Style); + + // Use braces for the outer `if` since the nested `for` is braced. + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " // In this for loop body, it is necessary that we explain " + "the situation\n" + " // with this surprisingly long comment, forcing braces on " + "the `for` block.\n" + " handleAttr(A);\n" + " }\n" + "}", + Style); + + // Use braces on the outer block because there are more than two levels of + // nesting. + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " for (ssize_t i : llvm::seq(count))\n" + " handleAttrOnDecl(D, A, i);\n" + "}\n", + Style); + + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " for (ssize_t i : llvm::seq(count))\n" + " handleAttrOnDecl(D, A, i);\n" + "}\n", + "if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " for (ssize_t i : llvm::seq(count)) {\n" + " handleAttrOnDecl(D, A, i);\n" + " }\n" + " }\n" + "}\n", + Style); + + // Use braces on the outer block because of a nested `if`, otherwise the + // compiler would warn: `add explicit braces to avoid dangling else` + verifyFormat("if (auto *D = dyn_cast(D)) {\n" + " if (shouldProcess(D))\n" + " handleVarDecl(D);\n" + " else\n" + " markAsIgnored(D);\n" + "}\n", + Style); + + verifyFormat("if (auto *D = dyn_cast(D)) {\n" + " if (shouldProcess(D))\n" + " handleVarDecl(D);\n" + " else\n" + " markAsIgnored(D);\n" + "}\n", + "if (auto *D = dyn_cast(D)) {\n" + " if (shouldProcess(D)) {\n" + " handleVarDecl(D);\n" + " } else {\n" + " markAsIgnored(D);\n" + " }\n" + "}\n", + Style); + + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_SingleLineOnly; + verifyFormat("if (isa(D))\n" + " // It is necessary that we explain the situation with this " + "surprisingly long\n" + " handleOtherDecl(D);\n" + "}", + Style); + + // Removing as {} evem though we have a comment comment + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_MultipleLinesOnly; + verifyFormat( + "if (isa(D))\n" + " // It is necessary that we explain the situation with this " + "surprisingly long\n" + " // comment, so it would be unclear without the braces whether the " + "following\n" + " // statement is in the scope of the `if`.\n" + " // Because the condition is documented, we can't really hoist this\n" + " // comment that applies to the body above the if.\n" + " handleOtherDecl(D);\n", + "if (isa(D)) {\n" + " // It is necessary that we explain the situation with this " + "surprisingly long\n" + " // comment, so it would be unclear without the braces whether the " + "following\n" + " // statement is in the scope of the `if`.\n" + " // Because the condition is documented, we can't really hoist this\n" + " // comment that applies to the body above the if.\n" + " handleOtherDecl(D);\n" + "}", + Style); + + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_SingleLineOnly; + verifyFormat("do\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n" + "while (true);", + "do {\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n" + "} while (true);", + Style); + + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_MultipleLinesOnly; + verifyFormat("do\n" + " // A multi line comment\n" + " // A multi line comment\n" + " handleFunctionDecl(D);\n" + "while (true);", + "do {\n" + " // A multi line comment\n" + " // A multi line comment\n" + " handleFunctionDecl(D);\n" + "} while (true);", + Style); + + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_SingleLineOnly; + + verifyFormat("while (isa(D))\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n", + "while (isa(D)) {\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_MultipleLinesOnly; + + verifyFormat("while (isa(D))\n" + " // A multi line comment\n" + " // A multi line comment\n" + " handleFunctionDecl(D);\n", + "while (isa(D)) {\n" + " // A multi line comment\n" + " // A multi line comment\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_SingleLineOnly; + + verifyFormat("for (auto i : list)\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n", + "for (auto i : list) {\n" + " // A single line comment\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + Style.AutomaticBraces.BraceRemovalComments = + FormatStyle::AutomaticBraceCommentAllowance::ABCA_MultipleLinesOnly; + + verifyFormat("for (auto i : list)\n" + " // A multi line comment\n" + " // A multi line comment\n" + " handleFunctionDecl(D);\n", + "for (auto i : list) {\n" + " // A multi line comment\n" + " // A multi line comment\n" + " handleFunctionDecl(D);\n" + "}", + Style); +} + +} // namespace +} // namespace format +} // namespace clang Index: clang/unittests/Format/CMakeLists.txt =================================================================== --- clang/unittests/Format/CMakeLists.txt +++ clang/unittests/Format/CMakeLists.txt @@ -3,6 +3,7 @@ ) add_clang_unittest(FormatTests + AutomaticBracesTest.cpp CleanupTest.cpp FormatTest.cpp FormatTestComments.cpp Index: clang/unittests/Format/FormatTest.cpp =================================================================== --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -72,6 +72,8 @@ EXPECT_EQ(Expected.str(), format(Expected, Style)) << "Expected code is not stable"; EXPECT_EQ(Expected.str(), format(Code, Style)); + // clang::format::internal::reformat does not run any of the options that + // modify code for ObjC if (Style.Language == FormatStyle::LK_Cpp) { // Objective-C++ is a superset of C++, so everything checked for C++ // needs to be checked for Objective-C++ as well. @@ -6530,9 +6532,8 @@ OnePerLine.BinPackParameters = false; std::string input = "Constructor()\n" " : aaaa(a,\n"; - for (unsigned i = 0, e = 80; i != e; ++i) { + for (unsigned i = 0, e = 80; i != e; ++i) input += " a,\n"; - } input += " a) {}"; verifyFormat(input, OnePerLine); } @@ -22396,7 +22397,6 @@ "}"; EXPECT_EQ(Code, format(Code, Style)); } - } // namespace } // namespace format } // namespace clang