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

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 539045.
hokein added a comment.

add a testcase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154950/new/

https://reviews.llvm.org/D154950

Files:
  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


Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -274,7 +274,7 @@
 }
 
 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 @@
   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");
 }
 
Index: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -164,7 +164,7 @@
       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) {
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -112,15 +112,16 @@
   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));
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -79,7 +79,8 @@
 /// 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D154950.539045.patch
Type: text/x-patch
Size: 3886 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230711/c3083fb7/attachment.bin>