Retain public accessibility for witnesses of external protocol requirements

Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift

@@ -58,6 +58,11 @@ final class RedundantExplicitPublicAccessibilityMarker: SourceGraphMutator {
 
     private func validateExtension(_ decl: Declaration) throws {
         if decl.accessibility.isExplicitly(.public) {
+            // A public member declared in this extension may be the witness for a requirement of an
+            // external public protocol (e.g. `Identifiable`). Such a witness must remain public,
+            // otherwise compilation fails, so the extension's public accessibility is not redundant.
+            guard !decl.declarations.contains(where: isWitnessForExternalProtocolRequirement) else { return }
+
             // If the extended kind is already marked as having redundant public accessibility, then this extension
             // must also have redundant accessibility.
             if let extendedDecl = try graph.extendedDeclaration(forExtension: decl),
@@ -68,6 +73,21 @@ final class RedundantExplicitPublicAccessibilityMarker: SourceGraphMutator {
         }
     }
 
+    /// A declaration is the witness for an external protocol requirement when it has a related
+    /// reference to a protocol member whose USR does not resolve to a declaration in the source
+    /// graph. The unresolved USR indicates the requirement belongs to an external protocol (such as
+    /// the standard library's `Identifiable`), which is necessarily public. A witness for a public
+    /// protocol requirement must be declared with matching public accessibility, so its explicit
+    /// public modifier is never redundant.
+    private func isWitnessForExternalProtocolRequirement(_ decl: Declaration) -> Bool {
+        decl.related.contains { reference in
+            reference.kind == .related &&
+                reference.declarationKind.isProtocolMemberConformingKind &&
+                reference.name == decl.name &&
+                graph.declaration(withUsr: reference.usr) == nil
+        }
+    }
+
     private func mark(_ decl: Declaration) {
         // This declaration may already be retained by a comment command.
         guard !graph.isRetained(decl) else { return }

Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift

@@ -33,6 +33,8 @@ _ = NotRedundantPublicTestableImportClass().testableProperty
 
 ProtocolIndirectlyReferencedCrossModuleByExtensionMemberImpl().somePublicFunc()
 
+PublicProtocolWitnessForExternalProtocolRetainer().retain()
+
 // Properties
 _ = PublicTypeUsedAsPublicPropertyTypeRetainer().retain
 _ = PublicTypeUsedAsPublicPropertyInitializer().retain

Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/PublicProtocolWitnessForExternalProtocol.swift

@@ -0,0 +1,24 @@
+import Foundation
+
+// The protocol refines the external/stdlib `Identifiable` protocol. The `id` witness is
+// supplied as a default implementation in the extension below. Because `Identifiable` is a
+// public protocol, the witness must remain public, otherwise the compiler emits:
+// "property 'id' must be declared public because it matches a requirement in public protocol
+// 'Identifiable'".
+public protocol PublicProtocolWitnessForExternalProtocol: Identifiable {}
+
+public extension PublicProtocolWitnessForExternalProtocol {
+    var id: String { String(describing: self) }
+}
+
+public enum PublicProtocolWitnessForExternalProtocolEnum: String, PublicProtocolWitnessForExternalProtocol {
+    case first
+    case second
+}
+
+public class PublicProtocolWitnessForExternalProtocolRetainer {
+    public init() {}
+    public func retain() {
+        _ = PublicProtocolWitnessForExternalProtocolEnum.first.id
+    }
+}

Tests/AccessibilityTests/RedundantPublicAccessibilityTest.swift

@@ -349,4 +349,14 @@ final class RedundantPublicAccessibilityTest: SPMSourceGraphTestCase {
 
         assertNotRedundantPublicAccessibility(.enum("PublicTypeUsedAsPublicFunctionThrowType"))
     }
+
+    func testPublicProtocolWitnessForExternalProtocol() {
+        index()
+
+        // The `id` witness is supplied in a protocol extension default implementation and satisfies
+        // the requirement of the external public `Identifiable` protocol, so it must remain public.
+        assertNotRedundantPublicAccessibility(.extensionProtocol("PublicProtocolWitnessForExternalProtocol")) {
+            self.assertNotRedundantPublicAccessibility(.varInstance("id"))
+        }
+    }
 }