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,52 @@ /// \version 12 std::vector AttributeMacros; + enum BraceInsertionRemovalStyle { + /// Do not insert or remove braces. + BIS_Leave, + /// Always insert braces. + BIS_Always, + /// Insert braces if the line is likely to wrap. + BIS_WrapLikely, + /// Always remove braces. + BIS_Remove, + /// Remove if there is no comment + BIS_RemoveNoComment + }; + + /// The style of inserting braces in control flow statements. + struct AutomaticBracesStyle { + // Insert/Remove after if() + BraceInsertionRemovalStyle AfterIf; + + // Insert/Remove after for() + BraceInsertionRemovalStyle AfterFor; + + // Insert/Remove after while() + BraceInsertionRemovalStyle AfterWhile; + + // Insert/Remove after do + BraceInsertionRemovalStyle AfterDo; + + // Insert/Remove after else + BraceInsertionRemovalStyle AfterElse; + }; + + /// 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 + AutomaticBracesStyle AutomaticBraces; + /// If ``false``, a function call's arguments will either be all on the /// same line or will have one line each. /// \code Index: clang/lib/Format/BraceInserter.h =================================================================== --- /dev/null +++ clang/lib/Format/BraceInserter.h @@ -0,0 +1,45 @@ +//===--- BraceInserter.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_BRACEINSERTER_H +#define LLVM_CLANG_LIB_FORMAT_BRACEINSERTER_H + +#include "TokenAnalyzer.h" + +namespace clang { +namespace format { + +class BracesInserter : public TokenAnalyzer { +public: + BracesInserter(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/BraceInserter.cpp =================================================================== --- /dev/null +++ clang/lib/Format/BraceInserter.cpp @@ -0,0 +1,389 @@ +//===--- BraceInserter.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 "BraceInserter.h" +#include "clang/Format/Format.h" + +namespace clang { +namespace format { + +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 +BracesInserter::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 BracesInserter::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::BraceInsertionRemovalStyle::BIS_Always); + break; + case tok::kw_for: + ShouldInsert = (Style.AutomaticBraces.AfterFor == + FormatStyle::BraceInsertionRemovalStyle::BIS_Always); + break; + case tok::kw_while: + ShouldInsert = (Style.AutomaticBraces.AfterWhile == + FormatStyle::BraceInsertionRemovalStyle::BIS_Always); + break; + case tok::kw_do: + ShouldInsert = (Style.AutomaticBraces.AfterDo == + FormatStyle::BraceInsertionRemovalStyle::BIS_Always); + break; + case tok::kw_else: + ShouldInsert = (Style.AutomaticBraces.AfterElse == + FormatStyle::BraceInsertionRemovalStyle::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 BracesInserter::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; + if (!RParen->Next) + continue; + Next = RParen; + } else if (StartingToken->isOneOf(tok::kw_else, tok::kw_do)) { + // If we are an 'else' 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)) { + LBrace = nullptr; + continue; + } + + if (LBrace->startsSequence(tok::l_brace, tok::comment)) { + if ((StartingToken->is(tok::kw_if) && + Style.AutomaticBraces.AfterIf == FormatStyle::BIS_RemoveNoComment) || + (StartingToken->is(tok::kw_else) && + Style.AutomaticBraces.AfterElse == + FormatStyle::BIS_RemoveNoComment) || + (StartingToken->is(tok::kw_do) && + Style.AutomaticBraces.AfterDo == FormatStyle::BIS_RemoveNoComment) || + (StartingToken->is(tok::kw_while) && + Style.AutomaticBraces.AfterWhile == + FormatStyle::BIS_RemoveNoComment) || + (StartingToken->is(tok::kw_while) && + Style.AutomaticBraces.AfterFor == + FormatStyle::BIS_RemoveNoComment)) { + 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.AfterIf == FormatStyle::BIS_RemoveNoComment) { + 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 + BraceInserter.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 "BraceInserter.h" #include "BreakableToken.h" #include "ContinuationIndenter.h" #include "FormatInternal.h" @@ -214,6 +215,18 @@ } }; +template <> +struct ScalarEnumerationTraits { + static void enumeration(IO &IO, + FormatStyle::BraceInsertionRemovalStyle &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); + IO.enumCase(Value, "RemoveNoComment", FormatStyle::BIS_RemoveNoComment); + } +}; + template <> struct ScalarEnumerationTraits { static void enumeration(IO &IO, FormatStyle::BinaryOperatorStyle &Value) { IO.enumCase(Value, "All", FormatStyle::BOS_All); @@ -613,6 +626,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); @@ -822,6 +836,16 @@ } }; +template <> struct MappingTraits { + static void mapping(IO &IO, FormatStyle::AutomaticBracesStyle &BracesStyle) { + IO.mapOptional("AfterIf", BracesStyle.AfterIf); + IO.mapOptional("AfterFor", BracesStyle.AfterFor); + IO.mapOptional("AfterWhile", BracesStyle.AfterWhile); + IO.mapOptional("AfterDo", BracesStyle.AfterDo); + IO.mapOptional("AfterElse", BracesStyle.AfterElse); + } +}; + template <> struct MappingTraits { static void mapping(IO &IO, FormatStyle::BraceWrappingFlags &Wrapping) { IO.mapOptional("AfterCaseLabel", Wrapping.AfterCaseLabel); @@ -1137,6 +1161,12 @@ 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}; + LLVMStyle.PPIndentWidth = -1; LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None; LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave; @@ -2982,6 +3012,10 @@ }); } + Passes.emplace_back([&](const Environment &Env) { + return BracesInserter(Env, Expanded).process(); + }); + if (Style.Language == FormatStyle::LK_JavaScript && Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) Passes.emplace_back([&](const Environment &Env) { Index: clang/unittests/Format/BraceInserterTests.cpp =================================================================== --- /dev/null +++ clang/unittests/Format/BraceInserterTests.cpp @@ -0,0 +1,752 @@ +//===- unittest/Format/BraceInserterTest.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 "brace-inserter-test" + +using clang::tooling::ReplacementTest; +using clang::tooling::toReplacements; +using testing::ScopedTrace; + +namespace clang { +namespace format { +namespace { + +class BraceInserterTest : 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(BraceInserterTest, 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(BraceInserterTest, 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(BraceInserterTest, 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(BraceInserterTest, 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(BraceInserterTest, RemoveLLVMBraces) { + auto Style = getLLVMStyle(); + + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_RemoveNoComment; + Style.AutomaticBraces.AfterElse = FormatStyle::BIS_RemoveNoComment; + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_RemoveNoComment; + Style.AutomaticBraces.AfterDo = FormatStyle::BIS_RemoveNoComment; + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_RemoveNoComment; + + // Omit the braces, since the body is simple and clearly associated with the + // if. + verifyFormat("if (isa(D))\n" + " handleFunctionDecl(D);\n" + "else if (isa(D))\n" + " handleVarDecl(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" + " // Long 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 very long 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" + " // Long 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); + + // Removing as {} evem though we have a comment comment + Style.AutomaticBraces.AfterIf = FormatStyle::BIS_Remove; + 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.AfterDo = FormatStyle::BIS_Remove; + verifyFormat("do\n" + " // A very long comment\n" + " handleFunctionDecl(D);\n" + "while (true);", + "do {\n" + " // A very long comment\n" + " handleFunctionDecl(D);\n" + "} while (true);", + Style); + + Style.AutomaticBraces.AfterWhile = FormatStyle::BIS_Remove; + verifyFormat("while (isa(D))\n" + " // Long comment\n" + " handleFunctionDecl(D);\n", + "while (isa(D)) {\n" + " // Long comment\n" + " handleFunctionDecl(D);\n" + "}", + Style); + + Style.AutomaticBraces.AfterFor = FormatStyle::BIS_Remove; + verifyFormat("for (auto i : list)\n" + " // Long comment\n" + " handleFunctionDecl(D);\n", + "for (auto i : list) {\n" + " // Long 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 + BraceInserterTests.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. @@ -22396,7 +22398,6 @@ "}"; EXPECT_EQ(Code, format(Code, Style)); } - } // namespace } // namespace format } // namespace clang