[clang] [clang-format] Separate License text and include blocks (PR #77918)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 13 04:03:15 PST 2024
https://github.com/seranu updated https://github.com/llvm/llvm-project/pull/77918
>From 60a5851b40f03fb71b2a3d30972d51ba40244d68 Mon Sep 17 00:00:00 2001
From: Serban Ungureanu <serban.ungureanu at randstaddigital.com>
Date: Fri, 12 Jan 2024 14:33:32 +0200
Subject: [PATCH 1/3] [clang-format] Extend DefinitionBlockSeparatorStyle to
separate license text, include blocks and to support two empty lines between
blocks
---
clang/include/clang/Format/Format.h | 71 ++---
clang/lib/Format/DefinitionBlockSeparator.cpp | 56 +++-
clang/lib/Format/Format.cpp | 3 +-
clang/lib/Format/TokenAnnotator.h | 6 +
.../Format/DefinitionBlockSeparatorTest.cpp | 268 +++++++++++++-----
5 files changed, 292 insertions(+), 112 deletions(-)
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 5ffd63ee73fc36..794817a26879d5 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3853,46 +3853,51 @@ struct FormatStyle {
/// Leave definition blocks as they are.
SDS_Leave,
/// Insert an empty line between definition blocks.
- SDS_Always,
+ SDS_One,
+ /// Insert two empty lines between definition blocks.
+ SDS_Two,
/// Remove any empty line between definition blocks.
SDS_Never
};
/// Specifies the use of empty lines to separate definition blocks, including
- /// classes, structs, enums, and functions.
+ /// license text, includes, classes, structs, enums, and functions.
/// \code
/// Never v.s. Always
- /// #include <cstring> #include <cstring>
- /// struct Foo {
- /// int a, b, c; struct Foo {
- /// }; int a, b, c;
- /// namespace Ns { };
- /// class Bar {
- /// public: namespace Ns {
- /// struct Foobar { class Bar {
- /// int a; public:
- /// int b; struct Foobar {
- /// }; int a;
- /// private: int b;
- /// int t; };
- /// int method1() {
- /// // ... private:
- /// } int t;
- /// enum List {
- /// ITEM1, int method1() {
- /// ITEM2 // ...
- /// }; }
- /// template<typename T>
- /// int method2(T x) { enum List {
- /// // ... ITEM1,
- /// } ITEM2
- /// int i, j, k; };
- /// int method3(int par) {
- /// // ... template<typename T>
- /// } int method2(T x) {
- /// }; // ...
- /// class C {}; }
- /// }
+ /// /* License text /* License text
+ /// End license text */ End license text */
+ /// #include <cstring>
+ /// struct Foo { #include <cstring>
+ /// int a, b, c;
+ /// }; struct Foo {
+ /// namespace Ns { int a, b, c;
+ /// class Bar { };
+ /// public:
+ /// struct Foobar { namespace Ns {
+ /// int a; class Bar {
+ /// int b; public:
+ /// }; struct Foobar {
+ /// private: int a;
+ /// int t; int b;
+ /// int method1() { };
+ /// // ...
+ /// } private:
+ /// enum List { int t;
+ /// ITEM1,
+ /// ITEM2 int method1() {
+ /// }; // ...
+ /// template<typename T> }
+ /// int method2(T x) {
+ /// // ... enum List {
+ /// } ITEM1,
+ /// int i, j, k; ITEM2
+ /// int method3(int par) { };
+ /// // ...
+ /// } template<typename T>
+ /// }; int method2(T x) {
+ /// class C {}; // ...
+ /// } }
+ ///
/// int i, j, k;
///
/// int method3(int par) {
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..37d554fb678662 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -17,6 +17,22 @@
#include "llvm/Support/Debug.h"
#define DEBUG_TYPE "definition-block-separator"
+namespace {
+unsigned getNewlineCount(
+ clang::format::FormatStyle::SeparateDefinitionStyle separateDefinitions) {
+ switch (separateDefinitions) {
+ case clang::format::FormatStyle::SDS_One:
+ return 2;
+ case clang::format::FormatStyle::SDS_Two:
+ return 3;
+ case clang::format::FormatStyle::SDS_Never:
+ return 1;
+ case clang::format::FormatStyle::SDS_Leave:
+ assert(false);
+ }
+ return 1;
+}
+} // namespace
namespace clang {
namespace format {
std::pair<tooling::Replacements, unsigned> DefinitionBlockSeparator::analyze(
@@ -65,8 +81,7 @@ void DefinitionBlockSeparator::separateBlocks(
}
return false;
};
- unsigned NewlineCount =
- (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always ? 1 : 0) + 1;
+ unsigned NewlineCount = getNewlineCount(Style.SeparateDefinitionBlocks);
WhitespaceManager Whitespaces(
Env.getSourceManager(), Style,
Style.LineEnding > FormatStyle::LE_CRLF
@@ -74,9 +89,10 @@ void DefinitionBlockSeparator::separateBlocks(
Env.getSourceManager().getBufferData(Env.getFileID()),
Style.LineEnding == FormatStyle::LE_DeriveCRLF)
: Style.LineEnding == FormatStyle::LE_CRLF);
+ std::optional<bool> inLicenseText{};
for (unsigned I = 0; I < Lines.size(); ++I) {
const auto &CurrentLine = Lines[I];
- if (CurrentLine->InPPDirective)
+ if (CurrentLine->InMacroBody)
continue;
FormatToken *TargetToken = nullptr;
AnnotatedLine *TargetLine;
@@ -93,7 +109,7 @@ void DefinitionBlockSeparator::separateBlocks(
if (TargetToken->is(tok::eof))
return;
if (IsAccessSpecifierToken(TargetToken) ||
- (OpeningLineIndex > 0 &&
+ (OpeningLineIndex > 0 && OpeningLineIndex < Lines.size() &&
IsAccessSpecifierToken(Lines[OpeningLineIndex - 1]->First))) {
return;
}
@@ -171,6 +187,31 @@ void DefinitionBlockSeparator::separateBlocks(
return false;
};
+ // Separate License text.
+ const bool isComment = Lines[I]->isComment();
+ if (!inLicenseText.has_value()) {
+ inLicenseText = isComment;
+ if (isComment) {
+ while (I < Lines.size() && Lines[I]->isComment())
+ ++I;
+ if (I < Lines.size()) {
+ inLicenseText = false;
+ TargetLine = Lines[I];
+ TargetToken = TargetLine->First;
+ InsertReplacement(NewlineCount);
+ continue;
+ }
+ }
+ }
+
+ // Separate includes block.
+ if (I > 0 && Lines[I - 1]->isInclude() && !Lines[I]->isInclude()) {
+ TargetLine = Lines[I];
+ TargetToken = TargetLine->First;
+ InsertReplacement(NewlineCount);
+ continue;
+ }
+
if (HasEnumOnLine() &&
!LikelyDefinition(CurrentLine, /*ExcludeEnum=*/true)) {
// We have no scope opening/closing information for enum.
@@ -214,8 +255,10 @@ void DefinitionBlockSeparator::separateBlocks(
TargetToken = TargetLine->First;
if (!FollowingOtherOpening()) {
// Avoid duplicated replacement.
- if (TargetToken->isNot(tok::l_brace))
+ if (TargetToken->isNot(tok::l_brace) && OpeningLineIndex > 0 &&
+ !Lines[OpeningLineIndex - 1]->isInclude()) {
InsertReplacement(NewlineCount);
+ }
} else if (IsNeverStyle) {
InsertReplacement(OpeningLineIndex != 0);
}
@@ -238,7 +281,8 @@ void DefinitionBlockSeparator::separateBlocks(
++OpeningLineIndex;
}
TargetLine = Lines[OpeningLineIndex];
- if (!LikelyDefinition(TargetLine)) {
+ if (!LikelyDefinition(TargetLine) && OpeningLineIndex > 0 &&
+ !Lines[OpeningLineIndex - 1]->isInclude()) {
OpeningLineIndex = I + 1;
TargetLine = Lines[I + 1];
TargetToken = TargetLine->First;
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index ff5ed6c306f383..4e23fd8ed15e8f 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -586,7 +586,8 @@ template <>
struct ScalarEnumerationTraits<FormatStyle::SeparateDefinitionStyle> {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value) {
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
- IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+ IO.enumCase(Value, "One", FormatStyle::SDS_One);
+ IO.enumCase(Value, "Two", FormatStyle::SDS_Two);
IO.enumCase(Value, "Never", FormatStyle::SDS_Never);
}
};
diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index 05a6daa87d8034..cf40eea9026270 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -113,6 +113,12 @@ class AnnotatedLine {
return First && First->is(tok::comment) && !First->getNextNonComment();
}
+ bool isInclude() const {
+ const auto *nextToken = First->getNextNonComment();
+ return First && First->is(tok::hash) && nextToken &&
+ nextToken->is(tok::pp_include);
+ }
+
/// \c true if this line starts with the given tokens in order, ignoring
/// comments.
template <typename... Ts> bool startsWith(Ts... Tokens) const {
diff --git a/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp b/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
index f5489498a93b9e..9f64675d796ada 100644
--- a/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ b/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -17,80 +17,97 @@
namespace clang {
namespace format {
namespace {
+std::string
+separateDefinitionBlocks(llvm::StringRef Code,
+ const std::vector<tooling::Range> &Ranges,
+ const FormatStyle &Style = getLLVMStyle()) {
+ LLVM_DEBUG(llvm::errs() << "---\n");
+ LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+ tooling::Replacements Replaces = reformat(Style, Code, Ranges, "<stdin>");
+ auto Result = applyAllReplacements(Code, Replaces);
+ EXPECT_TRUE(static_cast<bool>(Result));
+ LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+ return *Result;
+}
-class DefinitionBlockSeparatorTest : public ::testing::Test {
-protected:
- static std::string
- separateDefinitionBlocks(llvm::StringRef Code,
- const std::vector<tooling::Range> &Ranges,
- const FormatStyle &Style = getLLVMStyle()) {
- LLVM_DEBUG(llvm::errs() << "---\n");
- LLVM_DEBUG(llvm::errs() << Code << "\n\n");
- tooling::Replacements Replaces = reformat(Style, Code, Ranges, "<stdin>");
- auto Result = applyAllReplacements(Code, Replaces);
- EXPECT_TRUE(static_cast<bool>(Result));
- LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
- return *Result;
- }
-
- static std::string
- separateDefinitionBlocks(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
- return separateDefinitionBlocks(
- Code,
- /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
- }
-
- static void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle(),
- llvm::StringRef ExpectedCode = "",
- bool Inverse = true) {
- ::testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
- bool HasOriginalCode = true;
- if (ExpectedCode == "") {
- ExpectedCode = Code;
- HasOriginalCode = false;
- }
+std::string
+separateDefinitionBlocks(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ return separateDefinitionBlocks(
+ Code,
+ /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+}
- EXPECT_EQ(ExpectedCode, separateDefinitionBlocks(ExpectedCode, Style))
- << "Expected code is not stable";
- if (Inverse) {
- FormatStyle InverseStyle = Style;
- if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always)
- InverseStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
- else
- InverseStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
- EXPECT_NE(ExpectedCode,
- separateDefinitionBlocks(ExpectedCode, InverseStyle))
- << "Inverse formatting makes no difference";
+std::string removeEmptyLines(llvm::StringRef Code) {
+ std::string Result = "";
+ for (auto Char : Code.str()) {
+ if (Result.size()) {
+ auto LastChar = Result.back();
+ if ((Char == '\n' && LastChar == '\n') ||
+ (Char == '\r' && (LastChar == '\r' || LastChar == '\n'))) {
+ continue;
+ }
}
- std::string CodeToFormat =
- HasOriginalCode ? Code.str() : removeEmptyLines(Code);
- std::string Result = separateDefinitionBlocks(CodeToFormat, Style);
- EXPECT_EQ(ExpectedCode, Result) << "Test failed. Formatted:\n" << Result;
+ Result.push_back(Char);
+ }
+ return Result;
+}
+void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(),
+ llvm::StringRef ExpectedCode = "", bool Inverse = true) {
+ ::testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
+ bool HasOriginalCode = true;
+ if (ExpectedCode == "") {
+ ExpectedCode = Code;
+ HasOriginalCode = false;
}
- static std::string removeEmptyLines(llvm::StringRef Code) {
- std::string Result = "";
- for (auto Char : Code.str()) {
- if (Result.size()) {
- auto LastChar = Result.back();
- if ((Char == '\n' && LastChar == '\n') ||
- (Char == '\r' && (LastChar == '\r' || LastChar == '\n'))) {
- continue;
- }
- }
- Result.push_back(Char);
+ EXPECT_EQ(ExpectedCode, separateDefinitionBlocks(ExpectedCode, Style))
+ << "Expected code is not stable";
+
+ auto checkInverseStyle = [&](FormatStyle::SeparateDefinitionStyle newStyle) {
+ FormatStyle InverseStyle = Style;
+ InverseStyle.SeparateDefinitionBlocks = newStyle;
+ if (newStyle == FormatStyle::SDS_Two)
+ InverseStyle.MaxEmptyLinesToKeep = 2;
+ EXPECT_NE(ExpectedCode,
+ separateDefinitionBlocks(ExpectedCode, InverseStyle))
+ << "Changing formatting makes no difference";
+ };
+ if (Inverse) {
+ switch (Style.SeparateDefinitionBlocks) {
+ case FormatStyle::SDS_Never:
+ checkInverseStyle(FormatStyle::SDS_One);
+ checkInverseStyle(FormatStyle::SDS_Two);
+ break;
+ case FormatStyle::SDS_One:
+ checkInverseStyle(FormatStyle::SDS_Never);
+ checkInverseStyle(FormatStyle::SDS_Two);
+ break;
+ case FormatStyle::SDS_Two:
+ checkInverseStyle(FormatStyle::SDS_Never);
+ checkInverseStyle(FormatStyle::SDS_One);
+ break;
+ case FormatStyle::SDS_Leave:
+ break;
}
- return Result;
}
-};
+ std::string CodeToFormat =
+ HasOriginalCode ? Code.str() : removeEmptyLines(Code);
+ std::string Result = separateDefinitionBlocks(CodeToFormat, Style);
+ EXPECT_EQ(ExpectedCode, Result) << "Test failed. Formatted:\n" << Result;
+}
+class DefinitionBlockSeparatorTest : public ::testing::Test {};
+
+class LicenseTest : public ::testing::TestWithParam<std::string> {};
+class IncludesTest : public ::testing::TestWithParam<std::string> {};
+class NoNewLineAtEofTest : public ::testing::TestWithParam<std::string> {};
#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
TEST_F(DefinitionBlockSeparatorTest, Basic) {
FormatStyle Style = getLLVMStyle();
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
verifyFormat("int foo(int i, int j) {\n"
" int r = i + j;\n"
" return r;\n"
@@ -164,7 +181,7 @@ TEST_F(DefinitionBlockSeparatorTest, Basic) {
TEST_F(DefinitionBlockSeparatorTest, FormatConflict) {
FormatStyle Style = getLLVMStyle();
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
llvm::StringRef Code = "class Test {\n"
"public:\n"
" static void foo() {\n"
@@ -178,7 +195,7 @@ TEST_F(DefinitionBlockSeparatorTest, FormatConflict) {
TEST_F(DefinitionBlockSeparatorTest, CommentBlock) {
FormatStyle Style = getLLVMStyle();
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
std::string Prefix = "enum Foo { FOO, BAR };\n"
"\n"
"/*\n"
@@ -248,18 +265,18 @@ TEST_F(DefinitionBlockSeparatorTest, UntouchBlockStartStyle) {
};
FormatStyle AlwaysStyle = getLLVMStyle();
- AlwaysStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ AlwaysStyle.SeparateDefinitionBlocks = FormatStyle::SDS_One;
FormatStyle NeverStyle = getLLVMStyle();
NeverStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
- auto TestKit = MakeUntouchTest("/* FOOBAR */\n"
+ auto TestKit = MakeUntouchTest("/* FOOBAR */\n\n"
"#ifdef FOO\n\n",
"\n#elifndef BAR\n\n", "\n#endif\n\n", false);
verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
- TestKit = MakeUntouchTest("/* FOOBAR */\n"
+ TestKit = MakeUntouchTest("/* FOOBAR */\n\n"
"#ifdef FOO\n",
"#elifndef BAR\n", "#endif\n", false);
verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
@@ -282,7 +299,7 @@ TEST_F(DefinitionBlockSeparatorTest, UntouchBlockStartStyle) {
TEST_F(DefinitionBlockSeparatorTest, Always) {
FormatStyle Style = getLLVMStyle();
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
verifyFormat("// clang-format off\n"
"template<class T>\n"
@@ -400,7 +417,7 @@ TEST_F(DefinitionBlockSeparatorTest, Never) {
TEST_F(DefinitionBlockSeparatorTest, OpeningBracketOwnsLine) {
FormatStyle Style = getLLVMStyle();
Style.BreakBeforeBraces = FormatStyle::BS_Allman;
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
verifyFormat("namespace NS\n"
"{\n"
"// Enum test1\n"
@@ -464,7 +481,7 @@ TEST_F(DefinitionBlockSeparatorTest, OpeningBracketOwnsLine) {
TEST_F(DefinitionBlockSeparatorTest, TryBlocks) {
FormatStyle Style = getLLVMStyle();
Style.BreakBeforeBraces = FormatStyle::BS_Allman;
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
verifyFormat("void FunctionWithInternalTry()\n"
"{\n"
" try\n"
@@ -540,7 +557,7 @@ TEST_F(DefinitionBlockSeparatorTest, Leave) {
TEST_F(DefinitionBlockSeparatorTest, CSharp) {
FormatStyle Style = getLLVMStyle(FormatStyle::LK_CSharp);
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Style.AllowShortEnumsOnASingleLine = false;
verifyFormat("namespace {\r\n"
@@ -581,7 +598,7 @@ TEST_F(DefinitionBlockSeparatorTest, CSharp) {
TEST_F(DefinitionBlockSeparatorTest, JavaScript) {
FormatStyle Style = getLLVMStyle(FormatStyle::LK_JavaScript);
- Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Style.AllowShortEnumsOnASingleLine = false;
verifyFormat("export const enum Foo {\n"
@@ -610,6 +627,113 @@ TEST_F(DefinitionBlockSeparatorTest, JavaScript) {
"}",
Style);
}
+
+TEST_P(LicenseTest, SeparateLicenseFromBlock) {
+ constexpr StringRef LicenseSingleLineCommentStyle = {"// start license\n"
+ "// license text\n"
+ "// more license text\n"
+ "// end license\n"};
+ constexpr StringRef LicenseMultipleLineCommentStyle{"/*\n"
+ "start license\n"
+ "license text\n"
+ "more license text\n"
+ "end license */\n"};
+
+ const auto Block = GetParam();
+ FormatStyle Style = getLLVMStyle();
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
+ Style.MaxEmptyLinesToKeep = 2;
+ verifyFormat(LicenseSingleLineCommentStyle.str() + "\n" + Block, Style);
+ verifyFormat(LicenseMultipleLineCommentStyle.str() + "\n" + Block, Style);
+
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Two;
+ verifyFormat(LicenseSingleLineCommentStyle.str() + "\n\n" + Block, Style);
+ verifyFormat(LicenseMultipleLineCommentStyle.str() + "\n\n" + Block, Style);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ DefinitionSeparator, LicenseTest,
+ ::testing::Values(std::string{"class Test {};"},
+ std::string{"class Test {\n"
+ "public:\n"
+ " void test() const {}\n"
+ "};\n"},
+ std::string{"namespace tests {};"},
+ std::string{"static int variable = 10;"},
+ std::string{"#ifnef __TEST__\n"
+ "#define __TEST__\n"
+ "#endif"}));
+
+TEST_P(IncludesTest, SeparateIncludeFromBlock) {
+ constexpr StringRef Includes = {"#include <string>\n"
+ "#include <cstdio>\n"};
+
+ const auto Block = GetParam();
+ FormatStyle Style = getLLVMStyle();
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
+ Style.MaxEmptyLinesToKeep = 2;
+ verifyFormat(Includes.str() + "\n" + Block, Style);
+
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Two;
+ verifyFormat(Includes.str() + "\n\n" + Block, Style);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ DefinitionSeparator, IncludesTest,
+ ::testing::Values(std::string{"class Test {};"},
+ std::string{"class Test {\n"
+ "public:\n"
+ " void test() const {}\n"
+ "};\n"},
+ std::string{"namespace tests {};"},
+ std::string{"static int variable = 10;"},
+ std::string{"#ifnef __TEST__\n"
+ "#define __TEST__\n"
+ "#endif"}));
+
+TEST_P(NoNewLineAtEofTest, NoNewLineAfterBlock) {
+ FormatStyle Style = getLLVMStyle();
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
+ Style.MaxEmptyLinesToKeep = 2;
+ const auto Code = GetParam();
+ verifyFormat(Code, Style, Code,
+ /* Inverse = */ false);
+
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Two;
+ verifyFormat(Code, Style, Code,
+ /* Inverse = */ false);
+}
+
+INSTANTIATE_TEST_SUITE_P(DefinitionSeparator, NoNewLineAtEofTest,
+ ::testing::Values(std::string{"// start license\n"
+ "// license text\n"
+ "// more license text\n"
+ "// end license\n"},
+ std::string{"// start license"},
+ std::string{"#include <string>"},
+ std::string{"#include <string>\n"
+ "#include <cstdio>"}));
+
+TEST_F(DefinitionBlockSeparatorTest,
+ NoNewLinesWhenThereIsNoCodeAfterLicenseText) {
+ FormatStyle Style = getLLVMStyle();
+ constexpr StringRef Code = {"// start license\n"
+ "// license text\n"
+ "// end license"};
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_One;
+ verifyFormat(Code, Style,
+ "// start license\n"
+ "// license text\n"
+ "// end license",
+ /* Inverse = */ false);
+
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Two;
+ verifyFormat(Code, Style,
+ "// start license\n"
+ "// license text\n"
+ "// end license",
+ /* Inverse = */ false);
+}
} // namespace
} // namespace format
} // namespace clang
>From 291c05994202393a858de1aafa8eeaf958223964 Mon Sep 17 00:00:00 2001
From: Serban Ungureanu <serban.ungureanu at randstaddigital.com>
Date: Sat, 13 Jan 2024 13:54:51 +0200
Subject: [PATCH 2/3] review comments
---
clang/lib/Format/DefinitionBlockSeparator.cpp | 12 ++++++------
clang/lib/Format/Format.cpp | 1 +
clang/unittests/Format/ConfigParseTest.cpp | 7 +++++++
.../Format/DefinitionBlockSeparatorTest.cpp | 1 -
4 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 37d554fb678662..0bed68f0e6dd0e 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -89,7 +89,7 @@ void DefinitionBlockSeparator::separateBlocks(
Env.getSourceManager().getBufferData(Env.getFileID()),
Style.LineEnding == FormatStyle::LE_DeriveCRLF)
: Style.LineEnding == FormatStyle::LE_CRLF);
- std::optional<bool> inLicenseText{};
+ bool InLicenseText { true };
for (unsigned I = 0; I < Lines.size(); ++I) {
const auto &CurrentLine = Lines[I];
if (CurrentLine->InMacroBody)
@@ -188,14 +188,14 @@ void DefinitionBlockSeparator::separateBlocks(
};
// Separate License text.
- const bool isComment = Lines[I]->isComment();
- if (!inLicenseText.has_value()) {
- inLicenseText = isComment;
- if (isComment) {
+ const bool IsComment = Lines[I]->isComment();
+ if (InLicenseText) {
+ InLicenseText = IsComment;
+ if (IsComment) {
while (I < Lines.size() && Lines[I]->isComment())
++I;
if (I < Lines.size()) {
- inLicenseText = false;
+ InLicenseText = false;
TargetLine = Lines[I];
TargetToken = TargetLine->First;
InsertReplacement(NewlineCount);
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 4e23fd8ed15e8f..91e86678a84582 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -586,6 +586,7 @@ template <>
struct ScalarEnumerationTraits<FormatStyle::SeparateDefinitionStyle> {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value) {
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
+ IO.enumCase(Value, "Always", FormatStyle::SDS_One);
IO.enumCase(Value, "One", FormatStyle::SDS_One);
IO.enumCase(Value, "Two", FormatStyle::SDS_Two);
IO.enumCase(Value, "Never", FormatStyle::SDS_Never);
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 18ecba270e3455..8c8581f11d5ea8 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -994,6 +994,13 @@ TEST(ConfigParseTest, ParsesConfiguration) {
FormatStyle::BBNSS_OnlyWithParen);
CHECK_PARSE("AllowBreakBeforeNoexceptSpecifier: Never",
AllowBreakBeforeNoexceptSpecifier, FormatStyle::BBNSS_Never);
+
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
+ CHECK_PARSE("SeparateDefinitionBlocks: One", SeparateDefinitionBlocks, FormatStyle::SDS_One);
+ CHECK_PARSE("SeparateDefinitionBlocks: Two", SeparateDefinitionBlocks, FormatStyle::SDS_Two);
+ CHECK_PARSE("SeparateDefinitionBlocks: Leave", SeparateDefinitionBlocks, FormatStyle::SDS_Leave);
+ CHECK_PARSE("SeparateDefinitionBlocks: Always", SeparateDefinitionBlocks, FormatStyle::SDS_One);
+ CHECK_PARSE("SeparateDefinitionBlocks: Never", SeparateDefinitionBlocks, FormatStyle::SDS_Never);
}
TEST(ConfigParseTest, ParsesConfigurationWithLanguages) {
diff --git a/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp b/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
index 9f64675d796ada..8d413901c971a1 100644
--- a/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ b/clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -98,7 +98,6 @@ void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
EXPECT_EQ(ExpectedCode, Result) << "Test failed. Formatted:\n" << Result;
}
class DefinitionBlockSeparatorTest : public ::testing::Test {};
-
class LicenseTest : public ::testing::TestWithParam<std::string> {};
class IncludesTest : public ::testing::TestWithParam<std::string> {};
class NoNewLineAtEofTest : public ::testing::TestWithParam<std::string> {};
>From 258aff2e0e3349cc6668b173766201d8a2aeb736 Mon Sep 17 00:00:00 2001
From: Serban Ungureanu <serban.ungureanu at randstaddigital.com>
Date: Sat, 13 Jan 2024 14:03:03 +0200
Subject: [PATCH 3/3] formatting
---
clang/lib/Format/DefinitionBlockSeparator.cpp | 2 +-
clang/unittests/Format/ConfigParseTest.cpp | 15 ++++++++++-----
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 0bed68f0e6dd0e..635adc14356214 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -89,7 +89,7 @@ void DefinitionBlockSeparator::separateBlocks(
Env.getSourceManager().getBufferData(Env.getFileID()),
Style.LineEnding == FormatStyle::LE_DeriveCRLF)
: Style.LineEnding == FormatStyle::LE_CRLF);
- bool InLicenseText { true };
+ bool InLicenseText{true};
for (unsigned I = 0; I < Lines.size(); ++I) {
const auto &CurrentLine = Lines[I];
if (CurrentLine->InMacroBody)
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 8c8581f11d5ea8..2f751ab0d8f89a 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -996,11 +996,16 @@ TEST(ConfigParseTest, ParsesConfiguration) {
AllowBreakBeforeNoexceptSpecifier, FormatStyle::BBNSS_Never);
Style.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
- CHECK_PARSE("SeparateDefinitionBlocks: One", SeparateDefinitionBlocks, FormatStyle::SDS_One);
- CHECK_PARSE("SeparateDefinitionBlocks: Two", SeparateDefinitionBlocks, FormatStyle::SDS_Two);
- CHECK_PARSE("SeparateDefinitionBlocks: Leave", SeparateDefinitionBlocks, FormatStyle::SDS_Leave);
- CHECK_PARSE("SeparateDefinitionBlocks: Always", SeparateDefinitionBlocks, FormatStyle::SDS_One);
- CHECK_PARSE("SeparateDefinitionBlocks: Never", SeparateDefinitionBlocks, FormatStyle::SDS_Never);
+ CHECK_PARSE("SeparateDefinitionBlocks: One", SeparateDefinitionBlocks,
+ FormatStyle::SDS_One);
+ CHECK_PARSE("SeparateDefinitionBlocks: Two", SeparateDefinitionBlocks,
+ FormatStyle::SDS_Two);
+ CHECK_PARSE("SeparateDefinitionBlocks: Leave", SeparateDefinitionBlocks,
+ FormatStyle::SDS_Leave);
+ CHECK_PARSE("SeparateDefinitionBlocks: Always", SeparateDefinitionBlocks,
+ FormatStyle::SDS_One);
+ CHECK_PARSE("SeparateDefinitionBlocks: Never", SeparateDefinitionBlocks,
+ FormatStyle::SDS_Never);
}
TEST(ConfigParseTest, ParsesConfigurationWithLanguages) {
More information about the cfe-commits
mailing list