[include-cleaner] Fix the fixIncludes API not respect main-file hea…

…der. The fixIncludes was using the `input` as the main file path, this will results in inserting header at wrong places. We need the main file path to so that we can get the real main-file header. Differential Revision: https://reviews.llvm.org/D154950
llvm · Jul 11, 2023 · 7f3d2cd · 7f3d2cd
1 parent 81bc7cf
commit 7f3d2cd
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 7 deletions.
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -79,7 +79,8 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
 /// Removes unused includes and inserts missing ones in the main file.
 /// Returns the modified main-file code.
 /// The FormatStyle must be C++ or ObjC (to support include ordering).
-std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
+std::string fixIncludes(const AnalysisResults &Results,
+                        llvm::StringRef FileName, llvm::StringRef Code,
                         const format::FormatStyle &IncludeStyle);
 
 /// Gets all the providers for a symbol by traversing each location.

diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -112,15 +112,16 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
   return Results;
 }
 
-std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
+std::string fixIncludes(const AnalysisResults &Results,
+                        llvm::StringRef FileName, llvm::StringRef Code,
                         const format::FormatStyle &Style) {
   assert(Style.isCpp() && "Only C++ style supports include insertions!");
   tooling::Replacements R;
   // Encode insertions/deletions in the magic way clang-format understands.
   for (const Include *I : Results.Unused)
-    cantFail(R.add(tooling::Replacement("input", UINT_MAX, 1, I->quote())));
+    cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 1, I->quote())));
   for (llvm::StringRef Spelled : Results.Missing)
-    cantFail(R.add(tooling::Replacement("input", UINT_MAX, 0,
+    cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 0,
                                         ("#include " + Spelled).str())));
   // "cleanup" actually turns the UINT_MAX replacements into concrete edits.
   auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));

diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -164,7 +164,7 @@ class Action : public clang::ASTFrontendAction {
       Results.Missing.clear();
     if (!Remove)
       Results.Unused.clear();
-    std::string Final = fixIncludes(Results, Code, getStyle(Path));
+    std::string Final = fixIncludes(Results, Path, Code, getStyle(Path));
 
     if (Print.getNumOccurrences()) {
       switch (Print) {

diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -274,7 +274,7 @@ TEST_F(AnalyzeTest, NoCrashWhenUnresolved) {
 }
 
 TEST(FixIncludes, Basic) {
-  llvm::StringRef Code = R"cpp(
+  llvm::StringRef Code = R"cpp(#include "d.h"
 #include "a.h"
 #include "b.h"
 #include <c.h>
@@ -300,11 +300,22 @@ TEST(FixIncludes, Basic) {
   Results.Unused.push_back(Inc.atLine(3));
   Results.Unused.push_back(Inc.atLine(4));
 
-  EXPECT_EQ(fixIncludes(Results, Code, format::getLLVMStyle()), R"cpp(
+  EXPECT_EQ(fixIncludes(Results, "d.cc", Code, format::getLLVMStyle()),
+R"cpp(#include "d.h"
 #include "a.h"
 #include "aa.h"
 #include "ab.h"
 #include <e.h>
+)cpp");
+
+  Results = {};
+  Results.Missing.push_back("\"d.h\"");
+  Code = R"cpp(#include "a.h")cpp";
+  // FIXME: this isn't correct, the main-file header d.h should be added before
+  // a.h.
+  EXPECT_EQ(fixIncludes(Results, "d.cc", Code, format::getLLVMStyle()),
+R"cpp(#include "a.h"
+#include "d.h"
 )cpp");
 }