[clang][ExtractAPI] Add support for Objective-C categories

[Ruturaj4]

Summary

Introducing support for Objective-C Categories for ExtractAPI. The suuport for Objective-C Categories is currently lacking
in terms of categories that extend a type defined in current module and categories that extend types that belong to external
modules such as NSString. Thus the objective of this patch is two fold, to fix former and the later.

This is achieved by introducing two new symbols ->

1. objective-c.module.extension.
2. objective-c.class.extension.

Note that the extension represents objective-c "category" (orienting well with Swift's Extension, which in spite that fact
that is different, however "broadly" serves similar purpose). The original idea is inspired by Max's @theMomax initial
post - https://forums.swift.org/t/symbol-graph-adaptions-for-documenting-extensions-to-external-types-in-docc/56684,
and Daniel's helpful comments and feedback. The implementation nonetheless is different serving purpose for this project.

The following diagram well represents the purpose ->

[Ruturaj4]

1. Updating D152770: [clang][ExtractAPI] Add support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Introducing support for Objective-C Categories for ExtractAPI. The suuport for Objective-C Categories is currently lacking
in terms of categories that extend a type defined in current module and categories that extend types that belong to external
modules such as NSString. Thus the objective of this patch is two fold, to fix former and the later.

This is achieved by introducing two new symbols ->

objective-c.module.extension.
objective-c.class.extension.
Note that the extension represents objective-c "category" (orienting well with Swift's Extension, which in spite that fact
that is different, however "broadly" serves similar purpose). The original idea is inspired by Max's @theMomax initial
post - https://forums.swift.org/t/symbol-graph-adaptions-for-documenting-extensions-to-external-types-in-docc/56684,
and Daniel's helpful comments and feedback. The implementation nonetheless is different serving purpose for this project.

[zixuw]

Thanks for the patch!

Like I mentioned in the inline comments, I'm not sure I understand the need of the "abstract" (in the sense that it's not really a concrete API) subtype of APIRecord. The design doesn't feel right within the existing structure.
If I recall the original issue correctly the problem is that a category might extend an interface that is not declared within the current compilation unit and the SymbolGraph output has references to an USR it cannot resolve. But that's why we have the SymbolReference Interface for the category record, and all the information we could possibly have from extract-api for that interface is already in the SymbolReference. Couldn't we use that to design an output format to indicate the external reference?

Also a side note that the word "module" has been overloaded, especially within LLVM/clang where "module" is used for clang modules or C++ modules. I would avoid creating a *ModuleRecord without qualifying it like *ExtractAPIModule and providing documentation to clarify the meaning.

clang/include/clang/ExtractAPI/API.h

148–150 ↗

(On Diff #537064)

I would really try to avoid patching the base APIRecord to get ways of creating "fake" records. The API records in the API set should really be all concrete symbols mapping back to an entity that is an API.
This doesn't feel right and might open up to future bad designs and bugs.

497–510 ↗

(On Diff #537064)

I'm still not convinced by this concept and design. Why do we need to have a subtype of APIRecord to represent relationships with external types from another "module"? We have SymbolReference for that purpose with the name and USR information.

Even if this is necessary, the name is extremely confusing. By the look of the rest of the change and the test cases, I believe this is meant to represent the external *interface* that a category is extending, not a category.

clang/include/clang/ExtractAPI/ExtractAPIVisitor.h

128–135 ↗

(On Diff #537064)

This looks very concerning..
It's way too generic for its name and it's not doing what it's describing.
By the look of the call site below, this seems to be supposed to check if an *interface*, not category, is recorded in this API set. There's no reason to have a separate template function for that. You can just do the check inline and also probably covered by one of the existing llvm STLExtra utilities.

[Ruturaj4]

1. Updating D152770: [clang][ExtractAPI] Add support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Thanks for your comments and sorry for delay. I have changed the design and moved everything in SymbolGraphSerializer.cpp.

[Ruturaj4]

• [clang][ExtractAPI] Add support for Objective-C categories
• [clang][ExtractAPI] Add support for Objective-C categories

1. Updating D152770: [clang][ExtractAPI] Add support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Upload the correct patch and update.

[dang]

clang/include/clang/ExtractAPI/ExtractAPIVisitor.h

488 ↗

(On Diff #546074)

I think this is fine for now but there must be a better way of doing this that doesn't involve traversing the whole APISet

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

218–219

I don't think this is right. My expectation is that category records that need to get emitted would still have their own unique USR.

757

What happens when there are multiple categories to the same type though?

773

I would expect this to be a new relationship kind that reads as "extensionTo"

[Ruturaj4]

1. Updating D152770: [clang][ExtractAPI] Add support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Made all the changes according to comments.

1. Removed check on line #219 to include unique USR for categories.
2. Explained how multiple categories to the same type are supported.
3. Added new relationship "extensionTo".

clang/include/clang/ExtractAPI/ExtractAPIVisitor.h

488 ↗

(On Diff #546074)

I looked at different api methods, but still not able to figure it out. I will do it once this gets approved. In a following patch.

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

218–219

Thanks. I removed the check.

757

Yes! I added this check keeping that in mind. If the parent symbol of the category is not added before then it pushes it to serialize (but skips if already present). Otherwise, just adds the category.

For e.g. in example test - objc_various_categories.m, there are two categories extending the same type:

@interface NSString (Category1)
-(void) StringMethod;
@end

@interface NSString (Category2)
-(void) StringMethod2;
@end

In this case, it adds the symbol for NSString (parent of categories Category1 and Category2). I fixed the comments to make that clear.

773

Thanks, that makes sense. I fixed it.

[Ruturaj4]

1. Updating D152770: [clang][ExtractAPI] Add support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Upload the correct patch and update.

[dang]

LGTM

[Ruturaj4]

• [clang][ExtractAPI] Add support for Objective-C categories
• [clang][ExtractAPI] Add support for Objective-C categories
• [clang][ExtractAPI] Add support for Objective-C categories
• [clang][ExtractAPI] Add support for Objective-C categories

1. Updating D152770: [clang][ExtractAPI] Add support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Fix test cases, and simulate the library interface.

[dang]

clang/test/ExtractAPI/objc_various_categories.m

8

I don't think you need to have MyClass2 and the associated myclass2.h. It's fine to check that everything looks right just for the NSString categories.

[Ruturaj4]

• [clang][ExtractAPI] Add support for Objective-C categories

1. Updating D152770: [clang][ExtractAPI] Add support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Update test to remove MyClass2.

[dang]

LGTM

clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h

enum RelationshipKind {
     MemberOf,
 
     /// The source symbol is inherited from the target symbol.
     InheritsFrom,
 
     /// The source symbol conforms to the target symbol.
     /// For example Objective-C protocol conformances.
     ConformsTo,
+
+    /// The source symbol is an extension to the target symbol.
+    /// For example Objective-C categories extending an external type.
+    ExtensionTo,
   };
 
   /// Get the string representation of the relationship kind.
   static StringRef getRelationshipString(RelationshipKind Kind);
 
 private:
   /// Just serialize the currently recorded objects in Symbol Graph format.
   Object serializeCurrentGraph();

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

StringRef getLanguageName(Language Lang) {
   llvm_unreachable("Unhandled language kind");
 }
 
 /// Serialize the identifier object as specified by the Symbol Graph format.
 ///
 /// The identifier property of a symbol contains the USR for precise and unique
 /// references, and the interface language name.
 Object serializeIdentifier(const APIRecord &Record, Language Lang) {
   Object Identifier;
-  if (auto *CategoryRecord =
-          dyn_cast_or_null<const ObjCCategoryRecord>(&Record))
-    Identifier["precise"] = CategoryRecord->Interface.USR;
-  else
-    Identifier["precise"] = Record.USR;
   Identifier["interfaceLanguage"] = getLanguageName(Lang);
 
   return Identifier;
 }
 
 /// Serialize the documentation comments attached to a symbol, as specified by
 /// the Symbol Graph format.
 ///

 StringRef SymbolGraphSerializer::getRelationshipString(RelationshipKind Kind) {
   switch (Kind) {
   case RelationshipKind::MemberOf:
     return "memberOf";
   case RelationshipKind::InheritsFrom:
     return "inheritsFrom";
   case RelationshipKind::ConformsTo:
     return "conformsTo";
+  case RelationshipKind::ExtensionTo:
+    return "extensionTo";
   }
   llvm_unreachable("Unhandled relationship kind");
 }
 
 void SymbolGraphSerializer::serializeRelationship(RelationshipKind Kind,
                                                   SymbolReference Source,
                                                   SymbolReference Target) {
   Object Relationship;
void SymbolGraphSerializer::visitObjCContainerRecord(
   }
 }
 
 void SymbolGraphSerializer::visitObjCCategoryRecord(
     const ObjCCategoryRecord &Record) {
   if (!Record.IsFromExternalModule)
     return;
 
-  // Check if the current Category Record has been visited before, if not add.
+  // Check if the current Category' parent has been visited before, if so skip.
   if (!(visitedCategories.contains(Record.Interface.Name) > 0)) {
     visitedCategories.insert(Record.Interface.Name);
     Object Obj;
     serializeObject(Obj, "identifier",
                     serializeIdentifier(Record, API.getLanguage()));
     serializeObject(Obj, "kind",
                     serializeSymbolKind(APIRecord::RK_ObjCCategoryModule,
                                         API.getLanguage()));
     Obj["accessLevel"] = "public";
     Symbols.emplace_back(std::move(Obj));
   }
 
   Object Relationship;
   Relationship["source"] = Record.USR;
   Relationship["target"] = Record.Interface.USR;
   Relationship["targetFallback"] = Record.Interface.Name;
-  Relationship["kind"] = getRelationshipString(RelationshipKind::MemberOf);
+  Relationship["kind"] = getRelationshipString(RelationshipKind::ExtensionTo);
   Relationships.emplace_back(std::move(Relationship));
 
   auto ObjCCategory = serializeAPIRecord(Record);
 
   if (!ObjCCategory)
     return;
 
   Symbols.emplace_back(std::move(*ObjCCategory));

clang/test/ExtractAPI/objc_module_category.m

"platform": {
         },
         "name": "macosx"
       },
       "vendor": "apple"
     }
   },
   "relationships": [
     {
-      "kind": "memberOf",
+      "kind": "extensionTo",
       "source": "c:objc(cy)NSString@Category1",
       "target": "c:objc(cs)NSString",
       "targetFallback": "NSString"
     },
     {
       "kind": "memberOf",
       "source": "c:objc(cs)NSString(im)method1",
       "target": "c:objc(cy)NSString@Category1",
       "targetFallback": "Category1"
     },
     {
-      "kind": "memberOf",
+      "kind": "extensionTo",
       "source": "c:objc(cy)NSString@Category2",
       "target": "c:objc(cs)NSString",
       "targetFallback": "NSString"
     },
     {
       "kind": "memberOf",
       "source": "c:objc(cs)NSString(im)method2",
       "target": "c:objc(cy)NSString@Category2",
       "targetFallback": "Category2"
     }
   ],
   "symbols": [
     {
       "accessLevel": "public",
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)NSString"
+        "precise": "c:objc(cy)NSString@Category1"
       },
       "kind": {
         "displayName": "Module Extension",
         "identifier": "objective-c.module.extension"
       }
     },
     {
       "accessLevel": "public",
"symbols": [
               }
             },
             "text": "Doc comment 1"
           }
         ]
       },
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)NSString"
+        "precise": "c:objc(cy)NSString@Category1"
       },
       "kind": {
         "displayName": "Class Extension",
         "identifier": "objective-c.class.extension"
       },
       "location": {
         "position": {
           "character": 12,
"symbols": [
               }
             },
             "text": "Doc comment 2"
           }
         ]
       },
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)NSString"
+        "precise": "c:objc(cy)NSString@Category2"
       },
       "kind": {
         "displayName": "Class Extension",
         "identifier": "objective-c.class.extension"
       },
       "location": {
         "position": {
           "character": 12,

clang/test/ExtractAPI/objc_various_categories.m

 // RUN: rm -rf %t
 // RUN: split-file %s %t
 // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api -x objective-c-header \
 // RUN: -target arm64-apple-macosx \
 // RUN: %t/myclass_1.h \
 // RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
 // RUN: %t/output.json >> %t/output-normalized.json
 // RUN: diff %t/reference.output.json %t/output-normalized.json
 
 // CHECK-NOT: error:
 // CHECK-NOT: warning:
//--- reference.output.json.in
   "relationships": [
     {
       "kind": "memberOf",
       "source": "c:objc(cs)MyClass1(im)SomeMethod",
       "target": "c:objc(cs)MyClass1",
       "targetFallback": "MyClass1"
     },
     {
-      "kind": "memberOf",
+      "kind": "extensionTo",
       "source": "c:objc(cy)MyClass2@MyCategory2",
       "target": "c:objc(cs)MyClass2",
       "targetFallback": "MyClass2"
     },
     {
       "kind": "memberOf",
       "source": "c:objc(cs)MyClass2(im)SomeMethod2",
       "target": "c:objc(cy)MyClass2@MyCategory2",
       "targetFallback": "MyCategory2"
     },
     {
-      "kind": "memberOf",
+      "kind": "extensionTo",
       "source": "c:objc(cy)NSString@Category1",
       "target": "c:objc(cs)NSString",
       "targetFallback": "NSString"
     },
     {
       "kind": "memberOf",
       "source": "c:objc(cs)NSString(im)StringMethod",
       "target": "c:objc(cy)NSString@Category1",
       "targetFallback": "Category1"
     },
     {
-      "kind": "memberOf",
+      "kind": "extensionTo",
       "source": "c:objc(cy)NSString@Category2",
       "target": "c:objc(cs)NSString",
       "targetFallback": "NSString"
     },
     {
       "kind": "memberOf",
       "source": "c:objc(cs)NSString(im)StringMethod2",
       "target": "c:objc(cy)NSString@Category2",
"symbols": [
         "MyClass1",
         "SomeMethod"
       ]
     },
     {
       "accessLevel": "public",
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)MyClass2"
+        "precise": "c:objc(cy)MyClass2@MyCategory2"
       },
       "kind": {
         "displayName": "Module Extension",
         "identifier": "objective-c.module.extension"
       }
     },
     {
       "accessLevel": "public",
"symbols": [
         },
         {
           "kind": "text",
           "spelling": ")"
         }
       ],
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)MyClass2"
+        "precise": "c:objc(cy)MyClass2@MyCategory2"
       },
       "kind": {
         "displayName": "Class Extension",
         "identifier": "objective-c.class.extension"
       },
       "location": {
         "position": {
           "character": 12,
"symbols": [
         "MyCategory2",
         "SomeMethod2"
       ]
     },
     {
       "accessLevel": "public",
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)NSString"
+        "precise": "c:objc(cy)NSString@Category1"
       },
       "kind": {
         "displayName": "Module Extension",
         "identifier": "objective-c.module.extension"
       }
     },
     {
       "accessLevel": "public",
"symbols": [
         },
         {
           "kind": "text",
           "spelling": ")"
         }
       ],
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)NSString"
+        "precise": "c:objc(cy)NSString@Category1"
       },
       "kind": {
         "displayName": "Class Extension",
         "identifier": "objective-c.class.extension"
       },
       "location": {
         "position": {
           "character": 12,
"symbols": [
         },
         {
           "kind": "text",
           "spelling": ")"
         }
       ],
       "identifier": {
         "interfaceLanguage": "objective-c",
-        "precise": "c:objc(cs)NSString"
+        "precise": "c:objc(cy)NSString@Category2"
       },
       "kind": {
         "displayName": "Class Extension",
         "identifier": "objective-c.class.extension"
       },
       "location": {
         "position": {
           "character": 12,