ioeric created this revision.
ioeric added a reviewer: djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D23274
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
test/Format/remove-duplicate-includes.cpp
tools/clang-format/ClangFormat.cpp
unittests/Format/SortIncludesTest.cpp
Index: unittests/Format/SortIncludesTest.cpp
===================================================================
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -26,8 +26,9 @@
std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
auto Ranges = GetCodeRange(Code);
- auto Sorted =
- applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));
+ auto Replaces = sortIncludes(Style, Code, Ranges, FileName);
+ Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+ auto Sorted = applyAllReplacements(Code, Replaces);
EXPECT_TRUE(static_cast<bool>(Sorted));
auto Result = applyAllReplacements(
*Sorted, reformat(Style, *Sorted, Ranges, FileName));
@@ -286,6 +287,87 @@
EXPECT_EQ(10u, newCursor(Code, 43));
}
+TEST_F(SortIncludesTest, DeduplicateIncludes) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include <a>\n"
+ "#include <b>\n"
+ "#include <c>\n",
+ sort("#include <a>\n"
+ "#include <b>\n"
+ "#include <b>\n"
+ "#include <b>\n"
+ "#include <b>\n"
+ "#include <c>\n"));
+}
+
+TEST_F(SortIncludesTest, SortAndDeduplicateIncludes) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include <a>\n"
+ "#include <b>\n"
+ "#include <c>\n",
+ sort("#include <b>\n"
+ "#include <a>\n"
+ "#include <b>\n"
+ "#include <b>\n"
+ "#include <c>\n"
+ "#include <b>\n"));
+}
+
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionAfterDeduplicate) {
+ Style.DeduplicateIncludes = true;
+ std::string Code = "#include <b>\n" // Start of line: 0
+ "#include <a>\n" // Start of line: 13
+ "#include <b>\n" // Start of line: 26
+ "#include <b>\n" // Start of line: 39
+ "#include <c>\n" // Start of line: 52
+ "#include <b>\n"; // Start of line: 65
+ std::string Expected = "#include <a>\n" // Start of line: 0
+ "#include <b>\n" // Start of line: 13
+ "#include <c>\n"; // Start of line: 26
+ EXPECT_EQ(Expected, sort(Code));
+ // Cursor on 'i' in "#include <a>".
+ EXPECT_EQ(1u, newCursor(Code, 14));
+ // Cursor on 'b' in "#include <b>".
+ EXPECT_EQ(23u, newCursor(Code, 10));
+ EXPECT_EQ(23u, newCursor(Code, 36));
+ EXPECT_EQ(23u, newCursor(Code, 49));
+ EXPECT_EQ(23u, newCursor(Code, 36));
+ EXPECT_EQ(23u, newCursor(Code, 75));
+ // Cursor on '#' in "#include <c>".
+ EXPECT_EQ(26u, newCursor(Code, 52));
+}
+
+TEST_F(SortIncludesTest, DeduplicateLocallyInEachBlock) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include <a>\n"
+ "#include <b>\n"
+ "\n"
+ "#include <b>\n"
+ "#include <c>\n",
+ sort("#include <a>\n"
+ "#include <b>\n"
+ "\n"
+ "#include <c>\n"
+ "#include <b>\n"
+ "#include <b>\n"));
+}
+
+TEST_F(SortIncludesTest, ValidAffactedRangesAfterDeduplicatingIncludes) {
+ Style.DeduplicateIncludes = true;
+ std::string Code = "#include <a>\n"
+ "#include <b>\n"
+ "#include <a>\n"
+ "#include <a>\n"
+ "\n"
+ " int x ;";
+ std::vector<tooling::Range> Ranges = {tooling::Range(0, 52)};
+ auto Replaces = sortIncludes(Style, Code, Ranges, "input.cpp");
+ Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+ EXPECT_EQ(1u, Ranges.size());
+ EXPECT_EQ(0u, Ranges[0].getOffset());
+ EXPECT_EQ(26u, Ranges[0].getLength());
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: tools/clang-format/ClangFormat.cpp
===================================================================
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -260,9 +260,8 @@
llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
return true;
}
- for (const auto &R : Replaces)
- Ranges.push_back({R.getOffset(), R.getLength()});
-
+ // Get new affected ranges after sorting `#includes`.
+ Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
bool IncompleteFormat = false;
Replacements FormatChanges = reformat(FormatStyle, *ChangedCode, Ranges,
AssumedFileName, &IncompleteFormat);
Index: test/Format/remove-duplicate-includes.cpp
===================================================================
--- /dev/null
+++ test/Format/remove-duplicate-includes.cpp
@@ -0,0 +1,13 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN: | clang-format -style="{BasedOnStyle: LLVM, SortIncludes: true, DeduplicateIncludes:true}" -lines=1:5 \
+// RUN: | FileCheck -strict-whitespace %s
+// CHECK: {{^#include\ <a>$}}
+#include <a>
+// CHECK: {{^#include\ <b>$}}
+#include <b>
+#include <a>
+#include <b>
+#include <b>
+{
+ int x ;
+}
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -337,6 +337,7 @@
IO.mapOptional("PointerAlignment", Style.PointerAlignment);
IO.mapOptional("ReflowComments", Style.ReflowComments);
IO.mapOptional("SortIncludes", Style.SortIncludes);
+ IO.mapOptional("DeduplicateIncludes", Style.DeduplicateIncludes);
IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
IO.mapOptional("SpaceBeforeAssignmentOperators",
Style.SpaceBeforeAssignmentOperators);
@@ -1238,37 +1239,67 @@
return std::tie(Includes[LHSI].Category, Includes[LHSI].Filename) <
std::tie(Includes[RHSI].Category, Includes[RHSI].Filename);
});
+ // The index of the include on which the cursor is currently put.
+ unsigned CurrentCursorIndex = UINT_MAX;
+ // The index of the include on which the cursor will be put after
+ // deduplicating.
+ unsigned CursorIndex = UINT_MAX;
+ if (Cursor)
+ // Find `CurrentCursorIndex` and `CursorIndex`.
+ for (int i = 0, e = Includes.size(); i != e; ++i) {
+ unsigned Start = Includes[Indices[i]].Offset;
+ unsigned End = Start + Includes[Indices[i]].Text.size();
+ if (*Cursor >= Start && *Cursor < End) {
+ CurrentCursorIndex = Indices[i];
+ CursorIndex = CurrentCursorIndex;
+ if (Style.DeduplicateIncludes) {
+ // Put the cursor on the only remaining #include among the duplicate
+ // #includes.
+ while (--i >= 0 &&
+ Includes[CursorIndex].Text == Includes[Indices[i]].Text)
+ CursorIndex = i;
+ }
+ break;
+ }
+ }
+
+ if (Style.DeduplicateIncludes)
+ Indices.erase(std::unique(Indices.begin(), Indices.end(),
+ [&](unsigned LHSI, unsigned RHSI) {
+ return Includes[LHSI].Text ==
+ Includes[RHSI].Text;
+ }),
+ Indices.end());
// If the #includes are out of order, we generate a single replacement fixing
// the entire block. Otherwise, no replacement is generated.
- if (std::is_sorted(Indices.begin(), Indices.end()))
+ if (Indices.size() == Includes.size() &&
+ std::is_sorted(Indices.begin(), Indices.end()))
return;
std::string result;
- bool CursorMoved = false;
for (unsigned Index : Indices) {
if (!result.empty())
result += "\n";
result += Includes[Index].Text;
- if (Cursor && !CursorMoved) {
- unsigned Start = Includes[Index].Offset;
- unsigned End = Start + Includes[Index].Text.size();
- if (*Cursor >= Start && *Cursor < End) {
- *Cursor = Includes.front().Offset + result.size() + *Cursor - End;
- CursorMoved = true;
- }
+ if (Cursor && CursorIndex == Index) {
+ unsigned CursorToEOLOffset = Includes[CurrentCursorIndex].Offset +
+ Includes[CurrentCursorIndex].Text.size() -
+ *Cursor;
+ *Cursor = Includes.front().Offset + result.size() - CursorToEOLOffset;
}
}
- // Sorting #includes shouldn't change their total number of characters.
- // This would otherwise mess up 'Ranges'.
- assert(result.size() ==
- Includes.back().Offset + Includes.back().Text.size() -
- Includes.front().Offset);
+ // If duplicate #includes are not deleted, sorting #includes shouldn't change
+ // their total number of characters.
+ unsigned num_chars_replaced = Includes.back().Offset +
+ Includes.back().Text.size() -
+ Includes.front().Offset;
+ assert(Style.DeduplicateIncludes || result.size() == num_chars_replaced);
auto Err = Replaces.add(tooling::Replacement(
- FileName, Includes.front().Offset, result.size(), result));
+ FileName, Includes.front().Offset, num_chars_replaced, result));
// FIXME: better error handling. For now, just skip the replacement for the
// release version.
if (Err)
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -546,6 +546,10 @@
/// \brief If ``true``, clang-format will sort ``#includes``.
bool SortIncludes;
+ /// \brief If ``true``, clang-format will remove duplicate ``#includes`` when
+ /// sorting ``#includes``.
+ bool DeduplicateIncludes;
+
/// \brief If ``true``, a space may be inserted after C style casts.
bool SpaceAfterCStyleCast;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
