[clangd] Fix fixAll not shown when there is only one unused-include and missing-include diagnostics.

[hokein]

Summary

Discovered during the review in https://reviews.llvm.org/D149437#inline-1444851.

clang-tools-extra/clangd/IncludeCleaner.cpp

return MapInfo::getHashValue(LHS.Symbol) <
            MapInfo::getHashValue(RHS.Symbol);
   });
   MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector<const Inclusion *> UnusedIncludes =
       getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
   return {std::move(UnusedIncludes), std::move(MissingIncludes)};
 }
 
-Fix removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) {
-  assert(!UnusedIncludes.empty());
+std::optional<Fix> removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) {
+  if (UnusedIncludes.empty())
+    return std::nullopt;
 
   Fix RemoveAll;
   RemoveAll.Message = "remove all unused includes";
   for (const auto &Diag : UnusedIncludes) {
     assert(Diag.Fixes.size() == 1 && "Expected exactly one fix.");
     RemoveAll.Edits.insert(RemoveAll.Edits.end(),
          Diag.Fixes.front().Edits.begin(),
          Diag.Fixes.front().Edits.end());
   }
 
   // TODO(hokein): emit a suitable text for the label.
   ChangeAnnotation Annotation = {/*label=*/"",
                                  /*needsConfirmation=*/true,
                                  /*description=*/""};
   static const ChangeAnnotationIdentifier RemoveAllUnusedID =
       "RemoveAllUnusedIncludes";
   for (unsigned I = 0; I < RemoveAll.Edits.size(); ++I) {
     ChangeAnnotationIdentifier ID = RemoveAllUnusedID + std::to_string(I);
     RemoveAll.Edits[I].annotationId = ID;
     RemoveAll.Annotations.push_back({ID, Annotation});
   }
   return RemoveAll;
 }
-Fix addAllMissingIncludes(llvm::ArrayRef<Diag> MissingIncludeDiags) {
-  assert(!MissingIncludeDiags.empty());
+std::optional<Fix>
+addAllMissingIncludes(llvm::ArrayRef<Diag> MissingIncludeDiags) {
+  if (MissingIncludeDiags.empty())
+    return std::nullopt;
 
   Fix AddAllMissing;
   AddAllMissing.Message = "add all missing includes";
   // A map to deduplicate the edits with the same new text.
   // newText (#include "my_missing_header.h") -> TextEdit.
   llvm::StringMap<TextEdit> Edits;
   for (const auto &Diag : MissingIncludeDiags) {
     assert(Diag.Fixes.size() == 1 && "Expected exactly one fix.");
Fix fixAll(const Fix& RemoveAllUnused, const Fix& AddAllMissing) {
   return FixAll;
 }
 
 std::vector<Diag> generateIncludeCleanerDiagnostic(
     ParsedAST &AST, const IncludeCleanerFindings &Findings,
     llvm::StringRef Code) {
   std::vector<Diag> UnusedIncludes = generateUnusedIncludeDiagnostics(
       AST.tuPath(), Findings.UnusedIncludes, Code);
-  std::optional<Fix> RemoveAllUnused;;
-  if (UnusedIncludes.size() > 1)
-    RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
+  std::optional<Fix> RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
 
   std::vector<Diag> MissingIncludeDiags = generateMissingIncludeDiagnostics(
       AST, Findings.MissingIncludes, Code);
-  std::optional<Fix> AddAllMissing;
-  if (MissingIncludeDiags.size() > 1)
-    AddAllMissing = addAllMissingIncludes(MissingIncludeDiags);
+  std::optional<Fix> AddAllMissing = addAllMissingIncludes(MissingIncludeDiags);
 
   std::optional<Fix> FixAll;
   if (RemoveAllUnused && AddAllMissing)
     FixAll = fixAll(*RemoveAllUnused, *AddAllMissing);
 
   auto AddBatchFix = [](const std::optional<Fix> &F, clang::clangd::Diag *Out) {
     if (!F) return;
     Out->Fixes.push_back(*F);
   };
   for (auto &Diag : MissingIncludeDiags) {
-    AddBatchFix(AddAllMissing, &Diag);
+    AddBatchFix(MissingIncludeDiags.size() > 1
+                    ? AddAllMissing
+                    : std::nullopt,
+                &Diag);
     AddBatchFix(FixAll, &Diag);
   }
   for (auto &Diag : UnusedIncludes) {
-    AddBatchFix(RemoveAllUnused, &Diag);
+    AddBatchFix(UnusedIncludes.size() > 1 ? RemoveAllUnused
+                                          : std::nullopt,
+                &Diag);
     AddBatchFix(FixAll, &Diag);
   }
 
   auto Result = std::move(MissingIncludeDiags);
   llvm::move(UnusedIncludes,
              std::back_inserter(Result));
   return Result;
 }

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

 #define DEF(X) const Foo *X;
 #define BAZ(X) const X x
 
   void foo() {
     $b[[b]]();
 
     ns::$bar[[Bar]] bar;
     bar.d();
-    $f[[f]](); 
+    $f[[f]]();
 
     // this should not be diagnosed, because it's ignored in the config
-    buzz(); 
+    buzz();
 
     $foobar[[foobar]]();
 
     std::$vector[[vector]] v;
 
     int var = $FOO[[FOO]];
 
     $DEF[[DEF]](a);
#define BAZ(X) const X x
   TU.AdditionalFiles["header.h"] = guard(R"cpp(
   namespace std { class vector {}; }
   )cpp");
 
   TU.AdditionalFiles["all.h"] = guard("#include \"foo.h\"");
   TU.AdditionalFiles["foo.h"] = guard(R"cpp(
     #define BAR(x) Foo *x
     #define FOO 1
-    struct Foo{}; 
+    struct Foo{};
   )cpp");
 
   TU.Code = MainFile.code();
   ParsedAST AST = TU.build();
 
   std::vector<clangd::Diag> Diags =
       issueIncludeCleanerDiagnostics(AST, TU.Code);
   EXPECT_THAT(
std::optional<include_cleaner::Header> MatchedProvider =
         firstMatchedProvider(
             convertIncludes(AST.getSourceManager(),
                             AST.getIncludeStructure().MainFileIncludes),
             Case.Providers);
     EXPECT_EQ(MatchedProvider, Case.ExpectedProvider);
   }
 }
 
+TEST(IncludeCleaner, BatchFix) {
+  Config Cfg;
+  Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+  WithContextValue Ctx(Config::Key, std::move(Cfg));
+
+  TestTU TU;
+  TU.Filename = "main.cpp";
+  TU.AdditionalFiles["foo.h"] = guard("class Foo;");
+  TU.AdditionalFiles["bar.h"] = guard("class Bar;");
+  TU.AdditionalFiles["all.h"] = guard(R"cpp(
+    #include "foo.h"
+    #include "bar.h"
+  )cpp");
+
+  TU.Code = R"cpp(
+  #include "all.h"
+
+  Foo* foo;
+  )cpp";
+  auto AST = TU.build();
+  EXPECT_THAT(
+      issueIncludeCleanerDiagnostics(AST, TU.Code),
+      UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""),
+                                    FixMessage("fix all includes")}),
+                           withFix({FixMessage("remove #include directive"),
+                                    FixMessage("fix all includes")})));
+
+  TU.Code = R"cpp(
+  #include "all.h"
+  #include "bar.h"
+
+  Foo* foo;
+  )cpp";
+  AST = TU.build();
+  EXPECT_THAT(
+      issueIncludeCleanerDiagnostics(AST, TU.Code),
+      UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""),
+                                    FixMessage("fix all includes")}),
+                           withFix({FixMessage("remove #include directive"),
+                                    FixMessage("remove all unused includes"),
+                                    FixMessage("fix all includes")}),
+                           withFix({FixMessage("remove #include directive"),
+                                   FixMessage("remove all unused includes"),
+                                    FixMessage("fix all includes")})));
+
+  TU.Code = R"cpp(
+  #include "all.h"
+
+  Foo* foo;
+  Bar* bar;
+  )cpp";
+  AST = TU.build();
+  EXPECT_THAT(
+      issueIncludeCleanerDiagnostics(AST, TU.Code),
+      UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""),
+                                    FixMessage("add all missing includes"),
+                                    FixMessage("fix all includes")}),
+                           withFix({FixMessage("#include \"bar.h\""),
+                                    FixMessage("add all missing includes"),
+                                    FixMessage("fix all includes")}),
+                           withFix({FixMessage("remove #include directive"),
+                                    FixMessage("fix all includes")})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang