[clang-tools-extra] 7f3d2cd - [include-cleaner] Fix the `fixIncludes` API not respect main-file header.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2023-07-11T15:02:26+02:00
New Revision: 7f3d2cd7ec254cda659c5e5a19a42105e370e04c

URL: https://github.com/llvm/llvm-project/commit/7f3d2cd7ec254cda659c5e5a19a42105e370e04c
DIFF: https://github.com/llvm/llvm-project/commit/7f3d2cd7ec254cda659c5e5a19a42105e370e04c.diff

LOG: [include-cleaner] Fix the `fixIncludes` API not respect main-file header.

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

Added: 
    

Modified: 
    clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
    clang-tools-extra/include-cleaner/lib/Analysis.cpp
    clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
    clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Removed: 
    


################################################################################
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
index 547e9dd7261ca3..77be4f1dad5212 100644
--- 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
index 616eeae43a87ee..e76929f6824ce7 100644
--- 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
index e7ae0c68c88214..c6cd7995d2e829 100644
--- 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
index b6521d56bcff44..64dd6bc1e84cae 100644
--- 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");
 }