[clang] [clang-format] Separate License text and include blocks (PR #77918)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 12 04:46:03 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: serbanu (seranu)
<details>
<summary>Changes</summary>
Extend SeparateDefinitionStyle to support spacing license text, include blocks and to also support two empty lines between blocks.
Fixes #<!-- -->42112 .
---
Patch is 24.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77918.diff
5 Files Affected:
- (modified) clang/include/clang/Format/Format.h (+38-33)
- (modified) clang/lib/Format/DefinitionBlockSeparator.cpp (+50-6)
- (modified) clang/lib/Format/Format.cpp (+2-1)
- (modified) clang/lib/Format/TokenAnnotator.h (+6)
- (modified) clang/unittests/Format/DefinitionBlockSeparatorTest.cpp (+196-72)
``````````diff
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"
+ ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/77918
More information about the cfe-commits
mailing list