diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 85715fb44f17..b7d3c55061a7 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1223,15 +1223,50 @@ static bool affectsRange(ArrayRef Ranges, unsigned Start, return false; } -// Sorts a block of includes given by 'Includes' alphabetically adding the -// necessary replacement to 'Replaces'. 'Includes' must be in strict source -// order. +// Returns a pair (Index, OffsetToEOL) describing the position of the cursor +// before sorting/deduplicating. Index is the index of the include under the +// cursor in the original set of includes. If this include has duplicates, it is +// the index of the first of the duplicates as the others are going to be +// removed. OffsetToEOL describes the cursor's position relative to the end of +// its current line. +// If `Cursor` is not on any #include, `Index` will be UINT_MAX. +static std::pair +FindCursorIndex(const SmallVectorImpl &Includes, + const SmallVectorImpl &Indices, unsigned Cursor) { + unsigned CursorIndex = UINT_MAX; + unsigned OffsetToEOL = 0; + 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)) + continue; + CursorIndex = Indices[i]; + OffsetToEOL = End - Cursor; + // 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; + } + return std::make_pair(CursorIndex, OffsetToEOL); +} + +// Sorts and deduplicate a block of includes given by 'Includes' alphabetically +// adding the necessary replacement to 'Replaces'. 'Includes' must be in strict +// source order. +// #include directives with the same text will be deduplicated, and only the +// first #include in the duplicate #includes remains. If the `Cursor` is +// provided and put on a deleted #include, it will be moved to the remaining +// #include in the duplicate #includes. static void sortCppIncludes(const FormatStyle &Style, - const SmallVectorImpl &Includes, - ArrayRef Ranges, StringRef FileName, - tooling::Replacements &Replaces, unsigned *Cursor) { - if (!affectsRange(Ranges, Includes.front().Offset, - Includes.back().Offset + Includes.back().Text.size())) + const SmallVectorImpl &Includes, + ArrayRef Ranges, StringRef FileName, + tooling::Replacements &Replaces, unsigned *Cursor) { + unsigned IncludesBeginOffset = Includes.front().Offset; + unsigned IncludesBlockSize = Includes.back().Offset + + Includes.back().Text.size() - + IncludesBeginOffset; + if (!affectsRange(Ranges, IncludesBeginOffset, IncludesBlockSize)) return; SmallVector Indices; for (unsigned i = 0, e = Includes.size(); i != e; ++i) @@ -1241,37 +1276,39 @@ static void sortCppIncludes(const FormatStyle &Style, 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 will be put after + // sorting/deduplicating. + unsigned CursorIndex; + // The offset from cursor to the end of line. + unsigned CursorToEOLOffset; + if (Cursor) + std::tie(CursorIndex, CursorToEOLOffset) = + FindCursorIndex(Includes, Indices, *Cursor); + + // Deduplicate #includes. + 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) + *Cursor = IncludesBeginOffset + 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); - auto Err = Replaces.add(tooling::Replacement( - FileName, Includes.front().Offset, result.size(), result)); + FileName, Includes.front().Offset, IncludesBlockSize, result)); // FIXME: better error handling. For now, just skip the replacement for the // release version. if (Err) diff --git a/clang/test/Format/remove-duplicate-includes.cpp b/clang/test/Format/remove-duplicate-includes.cpp new file mode 100644 index 000000000000..dedb1f4d7ae7 --- /dev/null +++ b/clang/test/Format/remove-duplicate-includes.cpp @@ -0,0 +1,14 @@ +// RUN: grep -Ev "// *[A-Z-]+:" %s \ +// RUN: | clang-format -style="{BasedOnStyle: LLVM, SortIncludes: true}" -lines=1:5 \ +// RUN: | FileCheck -strict-whitespace %s +// CHECK: {{^#include\ $}} +#include +// CHECK: {{^#include\ $}} +#include +#include +#include +#include +{ +// CHECK: {{^\ \ int x\ \ ;$}} + int x ; +} diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp index 3b893e04b323..c09723928b79 100644 --- a/clang/tools/clang-format/ClangFormat.cpp +++ b/clang/tools/clang-format/ClangFormat.cpp @@ -260,9 +260,8 @@ static bool format(StringRef FileName) { 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); diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp index 13a1b9adeeec..b6ee2ddf6693 100644 --- a/clang/unittests/Format/SortIncludesTest.cpp +++ b/clang/unittests/Format/SortIncludesTest.cpp @@ -26,8 +26,9 @@ protected: 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(Sorted)); auto Result = applyAllReplacements( *Sorted, reformat(Style, *Sorted, Ranges, FileName)); @@ -57,10 +58,10 @@ TEST_F(SortIncludesTest, NoReplacementsForValidIncludes) { // Identical #includes have led to a failure with an unstable sort. std::string Code = "#include \n" "#include \n" - "#include \n" - "#include \n" - "#include \n" - "#include \n"; + "#include \n" + "#include \n" + "#include \n" + "#include \n"; EXPECT_TRUE(sortIncludes(Style, Code, GetCodeRange(Code), "a.cc").empty()); } @@ -286,6 +287,82 @@ TEST_F(SortIncludesTest, CalculatesCorrectCursorPosition) { EXPECT_EQ(10u, newCursor(Code, 43)); } +TEST_F(SortIncludesTest, DeduplicateIncludes) { + EXPECT_EQ("#include \n" + "#include \n" + "#include \n", + sort("#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n")); +} + +TEST_F(SortIncludesTest, SortAndDeduplicateIncludes) { + EXPECT_EQ("#include \n" + "#include \n" + "#include \n", + sort("#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n")); +} + +TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionAfterDeduplicate) { + std::string Code = "#include \n" // Start of line: 0 + "#include \n" // Start of line: 13 + "#include \n" // Start of line: 26 + "#include \n" // Start of line: 39 + "#include \n" // Start of line: 52 + "#include \n"; // Start of line: 65 + std::string Expected = "#include \n" // Start of line: 0 + "#include \n" // Start of line: 13 + "#include \n"; // Start of line: 26 + EXPECT_EQ(Expected, sort(Code)); + // Cursor on 'i' in "#include ". + EXPECT_EQ(1u, newCursor(Code, 14)); + // Cursor on 'b' in "#include ". + 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 ". + EXPECT_EQ(26u, newCursor(Code, 52)); +} + +TEST_F(SortIncludesTest, DeduplicateLocallyInEachBlock) { + EXPECT_EQ("#include \n" + "#include \n" + "\n" + "#include \n" + "#include \n", + sort("#include \n" + "#include \n" + "\n" + "#include \n" + "#include \n" + "#include \n")); +} + +TEST_F(SortIncludesTest, ValidAffactedRangesAfterDeduplicatingIncludes) { + std::string Code = "#include \n" + "#include \n" + "#include \n" + "#include \n" + "\n" + " int x ;"; + std::vector 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