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")) + } + } }