[clang-format] append newline after code when inserting new headers at the end of the code which does not end with newline.

[ioeric]

Summary

append newline after code when inserting new headers at the end of the
code which does not end with newline.

[djasper]

Does this still work with the new Replacements?

lib/Format/Format.cpp

1572 ↗

(On Diff #59736)

"does not end"

1574 ↗

(On Diff #59736)

"whether"

[djasper]

Ping?

[ioeric]

• Merged with origin/master.
• Update newline insertion according to the new Replacements implementation.

[ioeric]

Sorry for the delay....

I've updated the patch to work with the new tooling::Replacements.

[djasper]

Looks good.

cfe/trunk/lib/Format/Format.cpp

fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
   // By this point, CategoryEndOffset[Highest] is always set appropriately:
   //  - to an appropriate location before/after existing #includes, or
   //  - to right after the header guard, or
   //  - to the beginning of the file.
   for (auto I = ++Priorities.begin(), E = Priorities.end(); I != E; ++I)
     if (CategoryEndOffsets.find(*I) == CategoryEndOffsets.end())
       CategoryEndOffsets[*I] = CategoryEndOffsets[*std::prev(I)];
 
+  bool NeedNewLineAtEnd = !Code.empty() && Code.back() != '\n';
   for (const auto &R : HeaderInsertions) {
     auto IncludeDirective = R.getReplacementText();
     bool Matched = IncludeRegex.match(IncludeDirective, &Matches);
     assert(Matched && "Header insertion replacement must have replacement text "
                       "'#include ...'");
     (void)Matched;
     auto IncludeName = Matches[2];
     if (ExistingIncludes.find(IncludeName) != ExistingIncludes.end()) {
       DEBUG(llvm::dbgs() << "Skip adding existing include : " << IncludeName
                          << "\n");
       continue;
     }
     int Category =
         Categories.getIncludePriority(IncludeName, /*CheckMainHeader=*/true);
     Offset = CategoryEndOffsets[Category];
     std::string NewInclude = !IncludeDirective.endswith("\n")
                                  ? (IncludeDirective + "\n").str()
                                  : IncludeDirective.str();
+    // When inserting headers at end of the code, also append '\n' to the code
+    // if it does not end with '\n'.
+    if (NeedNewLineAtEnd && Offset == Code.size()) {
+      NewInclude = "\n" + NewInclude;
+      NeedNewLineAtEnd = false;
+    }
     auto NewReplace = tooling::Replacement(FileName, Offset, 0, NewInclude);
     auto Err = Result.add(NewReplace);
     if (Err) {
       llvm::consumeError(std::move(Err));
+      unsigned NewOffset = Result.getShiftedCodePosition(Offset);
+      NewReplace = tooling::Replacement(FileName, NewOffset, 0, NewInclude);
       Result = Result.merge(tooling::Replacements(NewReplace));
     }
   }
   return Result;
 }
 
 } // anonymous namespace
 

cfe/trunk/unittests/Format/CleanupTest.cpp

 TEST_F(CleanUpReplacementsTest, EmptyCode) {
   std::string Code = "";
   std::string Expected = "#include <vector>\n";
   tooling::Replacements Replaces =
       toReplacements({createInsertion("#include <vector>")});
   EXPECT_EQ(Expected, apply(Code, Replaces));
 }
 
-// FIXME: although this case does not crash, the insertion is wrong. A '\n'
-// should be inserted between the two #includes.
 TEST_F(CleanUpReplacementsTest, NoNewLineAtTheEndOfCode) {
   std::string Code = "#include <map>";
-  std::string Expected = "#include <map>#include <vector>\n";
+  std::string Expected = "#include <map>\n#include <vector>\n";
   tooling::Replacements Replaces =
       toReplacements({createInsertion("#include <vector>")});
   EXPECT_EQ(Expected, apply(Code, Replaces));
 }
 
+TEST_F(CleanUpReplacementsTest, NoNewLineAtTheEndOfCodeMultipleInsertions) {
+  std::string Code = "#include <map>";
+  std::string Expected =
+      "#include <map>\n#include <string>\n#include <vector>\n";
+  tooling::Replacements Replaces =
+      toReplacements({createInsertion("#include <string>"),
+                      createInsertion("#include <vector>")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
 TEST_F(CleanUpReplacementsTest, SkipExistingHeaders) {
   std::string Code = "#include \"a.h\"\n"
                      "#include <vector>\n";
   std::string Expected = "#include \"a.h\"\n"
                          "#include <vector>\n";
   tooling::Replacements Replaces =
       toReplacements({createInsertion("#include <vector>"),
                       createInsertion("#include \"a.h\"")});