From a067b40365d74ba511c36e496bfc0eeb52ceecd2 Mon Sep 17 00:00:00 2001 From: Dan Wood Date: Mon, 15 Jun 2026 17:33:45 -0700 Subject: [PATCH] Retain public accessibility for witnesses of external protocol requirements A public member declared in a protocol extension can be the witness for a requirement of an external public protocol such as the standard library's Identifiable. Removing the public modifier from such a witness breaks compilation, because the witness must be at least as accessible as the requirement it satisfies. The redundant public accessibility analysis previously marked the enclosing extension as redundantly public when the extended type was itself redundant, without considering that one of the extension's members could be an external protocol witness. Detect this case by inspecting the member's related references: a related reference to a protocol member whose USR does not resolve to a declaration in the source graph indicates the requirement belongs to an external protocol, which is necessarily public. When such a witness is present, the extension's public accessibility is retained. --- ...antExplicitPublicAccessibilityMarker.swift | 20 ++++++++++++++++ .../Sources/MainTarget/main.swift | 2 ++ ...icProtocolWitnessForExternalProtocol.swift | 24 +++++++++++++++++++ .../RedundantPublicAccessibilityTest.swift | 10 ++++++++ 4 files changed, 56 insertions(+) create mode 100644 Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/PublicProtocolWitnessForExternalProtocol.swift diff --git a/Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift b/Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift index 20bb55c76..b92d70cbf 100644 --- a/Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift +++ b/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 } diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift index 48ff0ded2..3e566600f 100644 --- a/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift @@ -33,6 +33,8 @@ _ = NotRedundantPublicTestableImportClass().testableProperty ProtocolIndirectlyReferencedCrossModuleByExtensionMemberImpl().somePublicFunc() +PublicProtocolWitnessForExternalProtocolRetainer().retain() + // Properties _ = PublicTypeUsedAsPublicPropertyTypeRetainer().retain _ = PublicTypeUsedAsPublicPropertyInitializer().retain diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/PublicProtocolWitnessForExternalProtocol.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/PublicProtocolWitnessForExternalProtocol.swift new file mode 100644 index 000000000..b760ea83c --- /dev/null +++ b/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 + } +} diff --git a/Tests/AccessibilityTests/RedundantPublicAccessibilityTest.swift b/Tests/AccessibilityTests/RedundantPublicAccessibilityTest.swift index 2feebf794..600360fb1 100644 --- a/Tests/AccessibilityTests/RedundantPublicAccessibilityTest.swift +++ b/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")) + } + } }