[ExtractAPI] Cherry-pick multiple correctness fixes

clang/lib/ExtractAPI/DeclarationFragments.cpp

@@ -331,10 +331,15 @@ DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForType(
 
   // Declaration fragments of a pointer type is the declaration fragments of
   // the pointee type followed by a `*`,
-  if (T->isPointerType() && !T->isFunctionPointerType())
-    return Fragments
-        .append(getFragmentsForType(T->getPointeeType(), Context, After))
-        .append(" *", DeclarationFragments::FragmentKind::Text);
+  if (T->isPointerType() && !T->isFunctionPointerType()) {
+    QualType PointeeT = T->getPointeeType();
+    Fragments.append(getFragmentsForType(PointeeT, Context, After));
+    // If the pointee is itself a pointer, we do not want to insert a space
+    // before the `*` as the preceding character in the type name is a `*`.
+    if (!PointeeT->isAnyPointerType())
+      Fragments.appendSpace();
+    return Fragments.append("*", DeclarationFragments::FragmentKind::Text);
+  }
 
   // For Objective-C `id` and `Class` pointers
   // we do not spell out the `*`.
@@ -638,7 +643,7 @@ DeclarationFragmentsBuilder::getFragmentsForParam(const ParmVarDecl *Param) {
                 DeclarationFragments::FragmentKind::InternalParam);
   } else {
     Fragments.append(std::move(TypeFragments));
-    if (!T->isBlockPointerType())
+    if (!T->isAnyPointerType() && !T->isBlockPointerType())
       Fragments.appendSpace();
     Fragments
         .append(Param->getName(),
@@ -713,18 +718,20 @@ DeclarationFragmentsBuilder::getFragmentsForFunction(const FunctionDecl *Func) {
 
   // FIXME: Is `after` actually needed here?
   DeclarationFragments After;
+  QualType ReturnType = Func->getReturnType();
   auto ReturnValueFragment =
-      getFragmentsForType(Func->getReturnType(), Func->getASTContext(), After);
+      getFragmentsForType(ReturnType, Func->getASTContext(), After);
   if (StringRef(ReturnValueFragment.begin()->Spelling)
           .starts_with("type-parameter")) {
-    std::string ProperArgName = Func->getReturnType().getAsString();
+    std::string ProperArgName = ReturnType.getAsString();
     ReturnValueFragment.begin()->Spelling.swap(ProperArgName);
   }
 
-  Fragments.append(std::move(ReturnValueFragment))
-      .appendSpace()
-      .append(Func->getNameAsString(),
-              DeclarationFragments::FragmentKind::Identifier);
+  Fragments.append(std::move(ReturnValueFragment));
+  if (!ReturnType->isAnyPointerType())
+    Fragments.appendSpace();
+  Fragments.append(Func->getNameAsString(),
+                   DeclarationFragments::FragmentKind::Identifier);
 
   if (Func->getTemplateSpecializationInfo()) {
     Fragments.append("<", DeclarationFragments::FragmentKind::Text);
@@ -890,6 +897,9 @@ DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForCXXMethod(
   if (Method->isVolatile())
     Fragments.append("volatile", DeclarationFragments::FragmentKind::Keyword)
         .appendSpace();
+  if (Method->isVirtual())
+    Fragments.append("virtual", DeclarationFragments::FragmentKind::Keyword)
+        .appendSpace();
 
   // Build return type
   DeclarationFragments After;
@@ -1615,10 +1625,13 @@ DeclarationFragmentsBuilder::getFunctionSignature(const ObjCMethodDecl *);
 DeclarationFragments
 DeclarationFragmentsBuilder::getSubHeading(const NamedDecl *Decl) {
   DeclarationFragments Fragments;
-  if (isa<CXXConstructorDecl>(Decl) || isa<CXXDestructorDecl>(Decl))
+  if (isa<CXXConstructorDecl>(Decl)) {
     Fragments.append(cast<CXXRecordDecl>(Decl->getDeclContext())->getName(),
                      DeclarationFragments::FragmentKind::Identifier);
-  else if (isa<CXXConversionDecl>(Decl)) {
+  } else if (isa<CXXDestructorDecl>(Decl)) {
+    Fragments.append(cast<CXXDestructorDecl>(Decl)->getNameAsString(),
+                     DeclarationFragments::FragmentKind::Identifier);
+  } else if (isa<CXXConversionDecl>(Decl)) {
     Fragments.append(
         cast<CXXConversionDecl>(Decl)->getConversionType().getAsString(),
         DeclarationFragments::FragmentKind::Identifier);
@@ -1632,9 +1645,11 @@ DeclarationFragmentsBuilder::getSubHeading(const NamedDecl *Decl) {
   } else if (Decl->getIdentifier()) {
     Fragments.append(Decl->getName(),
                      DeclarationFragments::FragmentKind::Identifier);
-  } else
+  } else {
     Fragments.append(Decl->getDeclName().getAsString(),
                      DeclarationFragments::FragmentKind::Identifier);
+  }
+
   return Fragments;
 }
 

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

@@ -344,6 +344,22 @@ Object serializeNames(const APIRecord *Record) {
   serializeArray(Names, "subHeading",
                  serializeDeclarationFragments(Record->SubHeading));
   DeclarationFragments NavigatorFragments;
+  // The +/- prefix for Objective-C methods is important information, and
+  // should be included in the navigator fragment. The entire subheading is
+  // not included as it can contain too much information for other records.
+  switch (Record->getKind()) {
+  case APIRecord::RK_ObjCClassMethod:
+    NavigatorFragments.append("+ ", DeclarationFragments::FragmentKind::Text,
+                              /*PreciseIdentifier*/ "");
+    break;
+  case APIRecord::RK_ObjCInstanceMethod:
+    NavigatorFragments.append("- ", DeclarationFragments::FragmentKind::Text,
+                              /*PreciseIdentifier*/ "");
+    break;
+  default:
+    break;
+  }
+
   NavigatorFragments.append(Record->Name,
                             DeclarationFragments::FragmentKind::Identifier,
                             /*PreciseIdentifier*/ "");

clang/test/ExtractAPI/constructor_destructor.cpp

@@ -213,7 +213,7 @@ class Foo {
         "subHeading": [
           {
             "kind": "identifier",
-            "spelling": "Foo"
+            "spelling": "~Foo"
           }
         ],
         "title": "~Foo"

clang/test/ExtractAPI/global_record.c

@@ -185,7 +185,7 @@ char unavailable __attribute__((unavailable));
         },
         {
           "kind": "text",
-          "spelling": " * "
+          "spelling": " *"
         },
         {
           "kind": "internalParam",
@@ -341,7 +341,7 @@ char unavailable __attribute__((unavailable));
               },
               {
                 "kind": "text",
-                "spelling": " * "
+                "spelling": " *"
               },
               {
                 "kind": "internalParam",

clang/test/ExtractAPI/global_record_multifile.c

@@ -187,7 +187,7 @@ char unavailable __attribute__((unavailable));
         },
         {
           "kind": "text",
-          "spelling": " * "
+          "spelling": " *"
         },
         {
           "kind": "internalParam",
@@ -343,7 +343,7 @@ char unavailable __attribute__((unavailable));
               },
               {
                 "kind": "text",
-                "spelling": " * "
+                "spelling": " *"
               },
               {
                 "kind": "internalParam",

clang/test/ExtractAPI/macro_undefined.c

@@ -148,7 +148,7 @@ FUNC_GEN(bar, const int *, unsigned);
         },
         {
           "kind": "text",
-          "spelling": " * "
+          "spelling": " *"
         },
         {
           "kind": "internalParam",
@@ -195,7 +195,7 @@ FUNC_GEN(bar, const int *, unsigned);
               },
               {
                 "kind": "text",
-                "spelling": " * "
+                "spelling": " *"
               },
               {
                 "kind": "internalParam",

clang/test/ExtractAPI/methods.cpp

@@ -45,11 +45,19 @@ class Foo {
   // GETCOUNT-NEXT: ]
 
   // RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix SETL
-  void setLength(int length) noexcept;
+  virtual void setLength(int length) noexcept;
   // SETL: "!testRelLabel": "memberOf $ c:@S@Foo@F@setLength#I# $ c:@S@Foo"
   // SETL-LABEL: "!testLabel": "c:@S@Foo@F@setLength#I#"
   // SETL:      "declarationFragments": [
   // SETL-NEXT:   {
+  // SETL-NEXT:     "kind": "keyword",
+  // SETL-NEXT:     "spelling": "virtual"
+  // SETL-NEXT:   },
+  // SETL-NEXT:   {
+  // SETL-NEXT:     "kind": "text",
+  // SETL-NEXT:     "spelling": " "
+  // SETL-NEXT:   },
+  // SETL-NEXT:   {
   // SETL-NEXT:     "kind": "typeIdentifier",
   // SETL-NEXT:     "preciseIdentifier": "c:v",
   // SETL-NEXT:     "spelling": "void"

clang/test/ExtractAPI/objc_instancetype.m

@@ -157,6 +157,10 @@ - (id) reset;
       },
       "names": {
         "navigator": [
+          {
+            "kind": "text",
+            "spelling": "- "
+          },
           {
             "kind": "identifier",
             "spelling": "init"
@@ -228,6 +232,10 @@ - (id) reset;
       },
       "names": {
         "navigator": [
+          {
+            "kind": "text",
+            "spelling": "- "
+          },
           {
             "kind": "identifier",
             "spelling": "reset"