fix: Prefer doc comment on defn over forward decl (#298)

Author: varungandhi-src

indexer/Indexer.cc

@@ -239,7 +239,8 @@ void MacroIndexer::emitDocumentOccurrencesAndSymbols(
             switch (macroOcc.role) {
             case Role::Definition: {
               scip::SymbolInformation symbolInfo;
-              *symbolInfo.add_documentation() = "No documentation available.";
+              *symbolInfo.add_documentation() =
+                  scip::missingDocumentationPlaceholder;
               ENFORCE(!occ.symbol().empty());
               macroOcc.emitSymbolInformation(occ.symbol(), symbolInfo);
               *document.add_symbols() = std::move(symbolInfo);
@@ -967,7 +968,7 @@ void TuIndexer::saveDefinition(
     return;
   }
   if (optSymbolInfo.has_value() && optSymbolInfo->documentation_size() == 0) {
-    *optSymbolInfo->add_documentation() = "No documentation available.";
+    *optSymbolInfo->add_documentation() = scip::missingDocumentationPlaceholder;
   }
   if (optStableFileId->isInProject) {
     auto &doc = this->saveOccurrence(symbol, expansionLoc,

indexer/ScipExtras.cc

@@ -275,16 +275,25 @@ void IndexBuilder::addForwardDeclaration(
     this->externalSymbols.erase(extIt);
   }
   if (!forwardDeclSym.documentation().empty()) {
-    // FIXME(def: better-doc-merging): We shouldn't drop documentation
-    // attached to a definition, if present.
+    // FIXME(def: better-doc-merging): The missing documentation placeholder
+    // value is due to a bug in the backend which seemingly came up randomly
+    // and also got fixed somehow. We should remove the workaround if this
+    // is no longer an issue.
     if (auto *symbolInfo = it->second.dyn_cast<scip::SymbolInformation *>()) {
-      symbolInfo->mutable_documentation()->Clear();
-      for (auto &doc : *forwardDeclSym.mutable_documentation()) {
-        *symbolInfo->add_documentation() = std::move(doc);
+      bool isMissingDocumentation =
+          symbolInfo->documentation_size() == 0
+          || (symbolInfo->documentation_size() == 1
+              && symbolInfo->documentation()[0]
+                     == scip::missingDocumentationPlaceholder);
+      if (isMissingDocumentation) {
+        symbolInfo->mutable_documentation()->Clear();
+        for (auto &doc : *forwardDeclSym.mutable_documentation()) {
+          *symbolInfo->add_documentation() = std::move(doc);
+        }
       }
     } else {
       auto &symbolInfoBuilder = *it->second.get<SymbolInformationBuilder *>();
-      // FIXME(def: better-documentation-merging): We shouldn't drop
+      // FIXME(def: better-doc-merging): We shouldn't drop
       // documentation attached to a forward declaration.
       if (!symbolInfoBuilder.hasDocumentation()) {
         symbolInfoBuilder.setDocumentation(

indexer/ScipExtras.h

@@ -77,6 +77,9 @@ struct OccurrenceExt {
   }
 };
 
+constexpr char missingDocumentationPlaceholder[28] =
+    "No documentation available.";
+
 class SymbolInformationBuilder final {
   std::vector<std::string> documentation;
   absl::flat_hash_set<RelationshipExt> relationships;

test/index/fwd_decl/doc_check.cc

@@ -0,0 +1,9 @@
+// format-options: showDocs
+
+/// Doc comment on forward decl
+class F;
+
+/// From forward decl
+class UndocumentedClass;
+
+void f(F *) {}

test/index/fwd_decl/doc_check.snapshot.cc

@@ -0,0 +1,18 @@
+  // format-options: showDocs
+//^^^^^^^^^^^^^^^^^^^^^^^^^^^ definition [..] `<file>/doc_check.cc`/
+//documentation
+//| File: doc_check.cc
+  
+  /// Doc comment on forward decl
+  class F;
+//      ^ reference [..] F#
+  
+  /// From forward decl
+  class UndocumentedClass;
+//      ^^^^^^^^^^^^^^^^^ reference [..] UndocumentedClass#
+  
+  void f(F *) {}
+//     ^ definition [..] f(84962a9dbd25570f).
+//     documentation
+//     | No documentation available.
+//       ^ reference [..] F#

test/index/fwd_decl/doc_check_def.cc

@@ -0,0 +1,8 @@
+// format-options: showDocs
+
+/// Doc comment on definition
+class F {
+};
+
+class UndocumentedClass {
+};

test/index/fwd_decl/doc_check_def.snapshot.cc

@@ -0,0 +1,17 @@
+  // format-options: showDocs
+//^^^^^^^^^^^^^^^^^^^^^^^^^^^ definition [..] `<file>/doc_check_def.cc`/
+//documentation
+//| File: doc_check_def.cc
+  
+  /// Doc comment on definition
+  class F {
+//      ^ definition [..] F#
+//      documentation
+//      | Doc comment on definition
+  };
+  
+  class UndocumentedClass {
+//      ^^^^^^^^^^^^^^^^^ definition [..] UndocumentedClass#
+//      documentation
+//      | From forward decl
+  };