ioeric created this revision.
ioeric added reviewers: djasper, klimek.
ioeric added subscribers: bkramer, cfe-commits.
Herald added a subscriber: klimek.
When a replacement's offset is set to UINT_MAX or -1U, it is treated as
a header insertion replacement by cleanupAroundReplacements(). The new #include
directive is then inserted into the correct block.
http://reviews.llvm.org/D20734
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -281,6 +281,294 @@
EXPECT_EQ(Expected, applyAllReplacements(Code, FinalReplaces));
}
+TEST_F(CleanUpReplacementsTest, NoExistingIncludeWithoutDefine) {
+ std::string Code = "int main() {}";
+ std::string Expected = "#include \"a.h\"\n"
+ "int main() {}";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include \"a.h\""));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, NoExistingIncludeWithDefine) {
+ std::string Code = "#ifndef __A_H__\n"
+ "#define __A_H__\n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+ std::string Expected = "#ifndef __A_H__\n"
+ "#define __A_H__\n"
+ "\n"
+ "#include \"b.h\"\n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include \"b.h\""));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertAfterMainHeader) {
+ std::string Code = "#include \"fix.h\"\n"
+ "\n"
+ "int main() {}";
+ std::string Expected = "#include \"fix.h\"\n"
+ "#include <a>\n"
+ "\n"
+ "int main() {}";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include <a>"));
+ format::FormatStyle Style =
+ format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp);
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertBeforeSystemHeaderLLVM) {
+ std::string Code = "#include <memory>\n"
+ "\n"
+ "int main() {}";
+ std::string Expected = "#include \"z.h\"\n"
+ "#include <memory>\n"
+ "\n"
+ "int main() {}";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include \"z.h\""));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertAfterSystemHeaderGoogle) {
+ std::string Code = "#include <memory>\n"
+ "\n"
+ "int main() {}";
+ std::string Expected = "#include <memory>\n"
+ "#include \"z.h\"\n"
+ "\n"
+ "int main() {}";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include \"z.h\""));
+ format::FormatStyle Style =
+ format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp);
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertOneIncludeLLVMStyle) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"clang/Format/Format.h\"\n"
+ "#include <memory>\n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#include \"clang/Format/Format.h\"\n"
+ "#include \"llvm/x/y.h\"\n"
+ "#include <memory>\n";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include \"d.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"llvm/x/y.h\""));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesLLVMStyle) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"clang/Format/Format.h\"\n"
+ "#include <memory>\n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"new/new.h\"\n"
+ "#include \"clang/Format/Format.h\"\n"
+ "#include <memory>\n"
+ "#include <list>\n";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include <list>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"new/new.h\""));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertNewSystemIncludeGoogleStyle) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "\n"
+ "#include \"y/a.h\"\n"
+ "#include \"z/b.h\"\n";
+ // FIXME: inserting after the empty line following the main header might be
+ // prefered.
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include <vector>\n"
+ "\n"
+ "#include \"y/a.h\"\n"
+ "#include \"z/b.h\"\n";
+ Context.createInMemoryFile("x/fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include <vector>"));
+ format::FormatStyle Style =
+ format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp);
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesGoogleStyle) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "\n"
+ "#include <vector>\n"
+ "\n"
+ "#include \"y/a.h\"\n"
+ "#include \"z/b.h\"\n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "\n"
+ "#include <vector>\n"
+ "#include <list>\n"
+ "\n"
+ "#include \"y/a.h\"\n"
+ "#include \"z/b.h\"\n"
+ "#include \"x/x.h\"\n";
+ Context.createInMemoryFile("x/fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include <list>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"x/x.h\""));
+ format::FormatStyle Style =
+ format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp);
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortLLVM) {
+ std::string Code = "\nint x;";
+ std::string Expected = "#include \"fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"c.h\"\n"
+ "#include <list>\n"
+ "#include <vector>\n"
+ "\nint x;";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"a.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"c.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"b.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include <vector>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include <list>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"fix.h\""));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(
+ Code, formatReplacements(Code, NewReplaces, Style)));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
+ std::string Code = "\nint x;";
+ std::string Expected = "#include \"fix.h\"\n"
+ "#include <list>\n"
+ "#include <vector>\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"c.h\"\n"
+ "\nint x;";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"a.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"c.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"b.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include <vector>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include <list>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"fix.h\""));
+ format::FormatStyle Style =
+ format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp);
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(
+ Code, formatReplacements(Code, NewReplaces, Style)));
+}
+
+TEST_F(CleanUpReplacementsTest, FormatCorrectLineWhenHeadersAreInserted) {
+ std::string Code = "\n"
+ "int a;\n"
+ "int a;\n"
+ "int a;";
+
+ std::string Expected = "#include \"x.h\"\n"
+ "#include \"y.h\"\n"
+ "#include \"clang/x/x.h\"\n"
+ "#include <list>\n"
+ "#include <vector>\n"
+ "\n"
+ "int a;\n"
+ "int b;\n"
+ "int a;";
+ FileID ID = Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement(Context.Sources,
+ Context.getLocation(ID, 3, 8), 1, "b"));
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include <vector>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include <list>"));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"clang/x/x.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"y.h\""));
+ Replaces.insert(
+ tooling::Replacement("fix.cpp", -1U, 0, "#include \"x.h\""));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(
+ Code, formatReplacements(Code, NewReplaces, Style)));
+}
+
+TEST_F(CleanUpReplacementsTest, NotConfusedByDefine) {
+ std::string Code = "void f() {}\n"
+ "#define A \\\n"
+ " int i;";
+ std::string Expected = "#include <vector>\n"
+ "void f() {}\n"
+ "#define A \\\n"
+ " int i;";
+ Context.createInMemoryFile("fix.cpp", Code);
+ tooling::Replacements Replaces;
+ Replaces.insert(tooling::Replacement("fix.cpp", -1U, 0, "#include <vector>"));
+ format::FormatStyle Style = format::getLLVMStyle();
+ auto NewReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+ EXPECT_EQ(Expected, applyAllReplacements(Code, NewReplaces));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1402,6 +1402,170 @@
return processReplacements(Reformat, Code, SortedReplaces, Style);
}
+namespace {
+
+// Returns -1 if \p IncludeName does not match any category pattern.
+// FIXME: llvm::Regex::match is not a const function, so we can't pass
+// CategoryRegexs as const reference.
+int getIncludePriority(const FormatStyle &Style,
+ SmallVector<llvm::Regex, 4> &CategoryRegexs,
+ StringRef IncludeName) {
+ for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i) {
+ if (CategoryRegexs[i].match(IncludeName)) {
+ return Style.IncludeCategories[i].Priority;
+ }
+ }
+ return -1;
+}
+
+// FIXME: do not insert headers into conditional #include blocks, e.g. #includes
+// surrounded by compile condition "#if...".
+// FIXME: do not insert existing headers.
+// FIXME: insert empty lines between newly created blocks.
+tooling::Replacements
+fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
+ const FormatStyle &Style) {
+ if (Replaces.empty())
+ return tooling::Replacements();
+
+ llvm::Regex IncludeRegex(
+ R"(^[\t\ ]*#[\t\ ]*include[\t\ ]*(["<][^">]*[">]))");
+ llvm::Regex DefineRegex(R"(^[\t\ ]*#[\t\ ]*define[\t\ ]*[^\\]*$)");
+ SmallVector<StringRef, 4> Matches;
+ // Create pre-compiled regular expressions for the #include categories.
+ SmallVector<llvm::Regex, 4> CategoryRegexs;
+ for (const auto &Category : Style.IncludeCategories)
+ CategoryRegexs.emplace_back(Category.Regex);
+
+ StringRef FileName = Replaces.begin()->getFilePath();
+ StringRef FileStem = llvm::sys::path::stem(FileName);
+ bool IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
+ FileName.endswith(".cpp") || FileName.endswith(".c++") ||
+ FileName.endswith(".cxx") || FileName.endswith(".m") ||
+ FileName.endswith(".mm");
+ int MaxPriority = 0;
+ for (const auto &Category : Style.IncludeCategories) {
+ if (Category.Priority > MaxPriority)
+ MaxPriority = Category.Priority;
+ }
+
+ // Record the offset of the end of the last include in each category. Add an
+ // extra category (priority 0) for main include.
+ std::vector<int> CategoryEndOffsets(MaxPriority + 1, -1);
+ bool MainIncludeFound = false;
+ int FirstIncludeOffset = -1;
+ int Offset = 0;
+ int AfterFirstDefine = 0;
+ SmallVector<StringRef, 32> Lines;
+ Code.split(Lines, '\n');
+ for (auto Line : Lines) {
+ if (IncludeRegex.match(Line, &Matches)) {
+ StringRef IncludeName = Matches[1];
+ int Category = getIncludePriority(Style, CategoryRegexs, IncludeName);
+ if (IsMainFile && !MainIncludeFound && Category > 0 &&
+ FirstIncludeOffset < 0 && IncludeName.startswith("\"")) {
+ StringRef HeaderStem =
+ llvm::sys::path::stem(IncludeName.drop_front(1).drop_back(1));
+ if (FileStem.startswith(HeaderStem)) {
+ llvm::Regex MainIncludeRegex(
+ (HeaderStem + Style.IncludeIsMainRegex).str());
+ if (MainIncludeRegex.match(FileStem)) {
+ Category = 0;
+ MainIncludeFound = true;
+ }
+ }
+ }
+ if (Category < 0) {
+ llvm::errs() << "#include " << IncludeName
+ << " does not belong to any include category.\n";
+ } else {
+ CategoryEndOffsets[Category] = Offset + Line.size() + 1;
+ }
+ if (FirstIncludeOffset < 0)
+ FirstIncludeOffset = Offset;
+ }
+ Offset += Line.size() + 1;
+ if (AfterFirstDefine == 0 && DefineRegex.match(Line))
+ AfterFirstDefine = Offset;
+ }
+
+ tooling::Replacements Results;
+ bool AddNewLineBefore;
+ for (const auto &R : Replaces) {
+ auto IncludeDirective = R.getReplacementText();
+ if (!IncludeRegex.match(IncludeDirective, &Matches)) {
+ llvm::errs() << IncludeDirective
+ << " is not a valid #include directive.\n";
+ continue;
+ }
+ auto IncludeName = Matches[1];
+ int Category = getIncludePriority(Style, CategoryRegexs, IncludeName);
+ if (Category < 0) {
+ llvm::errs() << IncludeDirective
+ << " does not belong to any include category.\n";
+ continue;
+ }
+ // Append the new #include after the corresponding category if the category
+ // exists. Otherwise, append after the last category that has higher
+ // priority than the current category. If there is no category, insert after
+ // the first #define directive or top of the file is there is no #define
+ // directive.
+ // FIXME: skip comment section if there is no existing #include and #define.
+ AddNewLineBefore = false;
+ Offset = CategoryEndOffsets[Category];
+ if (Offset < 0) {
+ for (int P = Category - 1; P >= 0; P--) {
+ if (CategoryEndOffsets[P] >= 0) {
+ Offset = CategoryEndOffsets[P];
+ break;
+ }
+ }
+ if (Offset < 0) {
+ if (AfterFirstDefine > 0) {
+ Offset = AfterFirstDefine;
+ AddNewLineBefore = true;
+ } else {
+ Offset = 0;
+ }
+ }
+ }
+ std::string NewInclude =
+ (AddNewLineBefore && !IncludeDirective.startswith("\n"))
+ ? ("\n" + IncludeDirective).str()
+ : IncludeDirective.str();
+ if (!StringRef(NewInclude).endswith("\n"))
+ NewInclude.append("\n");
+ Results.insert(tooling::Replacement(FileName, Offset, 0, NewInclude));
+ }
+
+ return Results;
+}
+
+// Insert #include directives into the correct blocks.
+tooling::Replacements fixHeaderInsertions(StringRef Code,
+ const tooling::Replacements &Replaces,
+ const FormatStyle &Style) {
+ // FIXME: support other languages.
+ if (Style.Language != FormatStyle::LanguageKind::LK_Cpp)
+ return Replaces;
+ tooling::Replacements HeaderInsertionReplaces;
+ tooling::Replacements NewReplaces;
+ for (const auto &R : Replaces) {
+ if (R.getOffset() == -1U) {
+ HeaderInsertionReplaces.insert(R);
+ } else {
+ NewReplaces.insert(R);
+ }
+ }
+ HeaderInsertionReplaces =
+ fixCppIncludeInsertions(Code, HeaderInsertionReplaces, Style);
+ NewReplaces.insert(HeaderInsertionReplaces.begin(),
+ HeaderInsertionReplaces.end());
+ return NewReplaces;
+}
+
+} // anonymous namespace
+
tooling::Replacements
cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
const FormatStyle &Style) {
@@ -1412,7 +1576,10 @@
StringRef FileName) -> tooling::Replacements {
return cleanup(Style, Code, Ranges, FileName);
};
- return processReplacements(Cleanup, Code, Replaces, Style);
+ // Make header insertion replacements insert new headers into correct blocks.
+ tooling::Replacements NewReplaces =
+ fixHeaderInsertions(Code, Replaces, Style);
+ return processReplacements(Cleanup, Code, NewReplaces, Style);
}
tooling::Replacements reformat(const FormatStyle &Style, SourceManager &SM,
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -773,6 +773,9 @@
/// \brief Returns the replacements corresponding to applying \p Replaces and
/// cleaning up the code after that.
+/// This also inserts a C++ #include directive into the correct block if the
+/// replacement corresponding to the header insertion has offset UINT_MAX (i.e.
+/// -1U).
tooling::Replacements
cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
const FormatStyle &Style);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
