[clangd] If an undocumented definition exists, don't accept documentation from other forward decls.

[sammccall]

Summary

This fixes cases like:

foo.h
  class Undocumented{}
bar.h
  // break an include cycle. we should refactor this!
  class Undocumented;

Where the comment doesn't describe the class.

Note that a forward decl that is *visible to the definition* will still have
its doc comment used, by SymbolCollector: Merge isn't involved here.

[ilya-biryukov]

LGTM

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;
-  if (S.Documentation == "")
+  // 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)
     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) {

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

TEST(MergeIndexTest, Refs) {
   RefSlab::Builder Results2;
   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) {
+  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";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(M.Documentation, "");
+}
+
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 
 TEST(MergeTest, MergeIncludesOnDifferentDefinitions) {
   Symbol L, R;
   L.Name = "left";
   R.Name = "right";