[SymbolGraphGen] move "protocol implementations" check into isImplicitlyPrivate (#64867) (#64940)

rdar://107432084

Author: QuietMisdreavus

lib/SymbolGraphGen/SymbolGraph.cpp

@@ -191,15 +191,6 @@ SymbolGraph::isRequirementOrDefaultImplementation(const ValueDecl *VD) const {
 // MARK: - Symbols (Nodes)
 
 void SymbolGraph::recordNode(Symbol S) {
-  if (Walker.Options.SkipProtocolImplementations && S.getInheritedDecl()) {
-    const auto *DocCommentProvidingDecl =
-      getDocCommentProvidingDecl(S.getLocalSymbolDecl(), /*AllowSerialized=*/true);
-
-    // allow implementation symbols to remain if they have their own comment
-    if (DocCommentProvidingDecl != S.getLocalSymbolDecl())
-      return;
-  }
-
   Nodes.insert(S);
 
   // Record all of the possible relationships (edges) originating
@@ -637,6 +628,19 @@ SymbolGraph::serializeDeclarationFragments(StringRef Key, Type T,
   T->print(Printer, Options);
 }
 
+namespace {
+
+const ValueDecl *getProtocolRequirement(const ValueDecl *VD) {
+  auto reqs = VD->getSatisfiedProtocolRequirements();
+
+  if (!reqs.empty())
+    return reqs.front();
+  else
+    return nullptr;
+}
+
+}
+
 bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
                                       bool IgnoreContext) const {
   // Don't record unconditionally private declarations
@@ -698,6 +702,15 @@ bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
 
     // Special cases below.
 
+    // If we've been asked to skip protocol implementations, filter them out here.
+    if (Walker.Options.SkipProtocolImplementations && getProtocolRequirement(VD)) {
+      // Allow them to stay if they have their own doc comment
+      const auto *DocCommentProvidingDecl =
+        getDocCommentProvidingDecl(VD, /*AllowSerialized=*/true);
+      if (DocCommentProvidingDecl != VD)
+        return true;
+    }
+
     // Symbols from exported-imported modules should only be included if they
     // were originally public.
     if (Walker.isFromExportedImportedModule(D) &&

test/SymbolGraph/Symbols/SkipProtocolImplementations.swift

@@ -14,18 +14,27 @@
 // CHECK-NOT: s:27SkipProtocolImplementations04SomeB0PAAE9bonusFuncyyF::SYNTHESIZED::s:27SkipProtocolImplementations10SomeStructV
 // CHECK-NOT: s:27SkipProtocolImplementations10SomeStructV8someFuncyyF
 
+// ...as well as the inner type from `OtherProtocol` on `OtherStruct`
+// CHECK-NOT: "s:27SkipProtocolImplementations11OtherStructV5InnerV"
+
 // CHECK-LABEL: "symbols": [
 
 // SomeStruct.otherFunc() should be present because it has its own doc comment
-// CHECK: s:27SkipProtocolImplementations10SomeStructV9otherFuncyyF
+// CHECK-DAG: s:27SkipProtocolImplementations10SomeStructV9otherFuncyyF
+
+// Same for ExtraStruct.Inner
+// CHECK-DAG: s:27SkipProtocolImplementations11ExtraStructV5InnerV
 
 // CHECK-LABEL: "relationships": [
 
 // we want to make sure that the conformance relationship itself stays
 // CHECK-DAG: conformsTo
 
-// SomeStruct.otherFunc() should be the only one with sourceOrigin information
-// COUNT-COUNT-1: sourceOrigin
+// SomeStruct.otherFunc() and ExtraStruct.Inner should be the only ones with sourceOrigin information
+// (ExtraStruct.Inner will have two sourceOrigins because it has two relationships: a memberOf and a
+// conformsTo)
+// COUNT-COUNT-3: sourceOrigin
+// COUNT-NOT: sourceOrigin
 
 public protocol SomeProtocol {
     /// Base docs
@@ -45,3 +54,22 @@ public struct SomeStruct: SomeProtocol {
     /// Local docs
     public func otherFunc() {}
 }
+
+// Make sure that protocol conformances added in extensions don't create bogus symbol relationships (rdar://107432084)
+
+public protocol OtherProtocol {
+    associatedtype Inner
+}
+
+public struct OtherStruct: OtherProtocol {
+    public struct Inner {}
+}
+
+extension OtherStruct.Inner: Sendable {}
+
+public struct ExtraStruct: OtherProtocol {
+    /// This time with a doc comment!
+    public struct Inner {}
+}
+
+extension ExtraStruct.Inner: Sendable {}