[clangd] Fix a regression of not showing documentation from forward declarations.

[hokein]

Summary

There is a regression from https://reviews.llvm.org/D68467. Unlike class
forward declarations, function ducomentation is written in the declaration in
headers, the function definition doesn't contain any documentation, cases like:

foo.h
// this is foo.
void foo();
foo.cc

void foo() {}

we should still show documentation from the foo declaration.

[ilya-biryukov]

clang-tools-extra/clangd/index/Merge.cpp

195

NIT: || Kind == Union
I guess it's not very common, though

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

415

Could you add a similar test with a non-class symbol that checks the documentation is still there?

[hokein]

• include union type;
• add testcase for non-class symbols;

[ilya-biryukov]

LGTM

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

416

NIT: could you also test struct and union?

[hokein]

test union and struct kinds.

[merge_guards_bot]

Build result: pass - 59966 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt, CMakeCache.txt

clang-tools-extra/clangd/index/Merge.cpp

if (prefer(O.CanonicalDeclaration, S.CanonicalDeclaration))
     S.CanonicalDeclaration = O.CanonicalDeclaration;
   if (prefer(O.Definition, S.Definition))
     S.Definition = O.Definition;
   S.References += O.References;
   if (S.Signature == "")
     S.Signature = O.Signature;
   if (S.CompletionSnippetSuffix == "")
     S.CompletionSnippetSuffix = O.CompletionSnippetSuffix;
-  // Don't accept documentation from bare forward declarations, if there is a
-  // definition and it didn't provide one. S is often an undocumented class,
-  // and O is a non-canonical forward decl preceded by an irrelevant comment.
-  if (S.Documentation == "" && !S.Definition)
+  if (S.Documentation == "") {
+    // Don't accept documentation from bare forward class declarations, if there
+    // is a definition and it didn't provide one. S is often an undocumented
+    // class, and O is a non-canonical forward decl preceded by an irrelevant
+    // comment.
+    bool IsClass = S.SymInfo.Kind == index::SymbolKind::Class ||
+                   S.SymInfo.Kind == index::SymbolKind::Struct ||
+                   S.SymInfo.Kind == index::SymbolKind::Union;
+    if (!IsClass || !S.Definition)
       S.Documentation = O.Documentation;
+  }
   if (S.ReturnType == "")
     S.ReturnType = O.ReturnType;
   if (S.Type == "")
     S.Type = O.Type;
   for (const auto &OI : O.IncludeHeaders) {
     bool Found = false;
     for (auto &SI : S.IncludeHeaders) {
       if (SI.IncludeHeader == OI.IncludeHeader) {

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

TEST(MergeIndexTest, Refs) {
   Merge.refs(Request, [&](const Ref &O) { Results2.insert(Foo.ID, O); });
   EXPECT_THAT(std::move(Results2).build(),
               ElementsAre(Pair(
                   _, ElementsAre(AnyOf(FileURI("unittest:///test.cc"),
                                        FileURI("unittest:///test2.cc"))))));
 }
 
 TEST(MergeIndexTest, NonDocumentation) {
+  using index::SymbolKind;
   Symbol L, R;
   L.ID = R.ID = SymbolID("x");
   L.Definition.FileURI = "file:/x.h";
   R.Documentation = "Forward declarations because x.h is too big to include";
+  for (auto ClassLikeKind :
+       {SymbolKind::Class, SymbolKind::Struct, SymbolKind::Union}) {
+    L.SymInfo.Kind = ClassLikeKind;
+    EXPECT_EQ(mergeSymbol(L, R).Documentation, "");
+  }
 
-  Symbol M = mergeSymbol(L, R);
-  EXPECT_EQ(M.Documentation, "");
+  L.SymInfo.Kind = SymbolKind::Function;
+  R.Documentation = "Documentation from non-class symbols should be included";
+  EXPECT_EQ(mergeSymbol(L, R).Documentation, R.Documentation);
 }
 
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 
 TEST(MergeTest, MergeIncludesOnDifferentDefinitions) {
   Symbol L, R;