Skip to content

Commit

Permalink
Fix redundant public accessibility analysis for protocol members
Browse files Browse the repository at this point in the history
  • Loading branch information
ileitch committed Oct 29, 2023
1 parent 0086f4b commit febffbc
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 9 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

##### Bug Fixes

- None.
- Fix redundant public accessibility analysis for protocol members declared in extensions that are referenced cross-module where the protocol itself is not.

## 2.16.0 (2023-09-27)

Expand Down
4 changes: 4 additions & 0 deletions Sources/PeripheryKit/Indexer/Declaration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -314,4 +314,8 @@ public struct DeclarationAccessibility {
public func isExplicitly(_ testValue: Accessibility) -> Bool {
isExplicit && value == testValue
}

var isAccessibleCrossModule: Bool {
value == .public || value == .open
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ final class RedundantExplicitPublicAccessibilityMarker: SourceGraphMutator {
private func validate(_ decl: Declaration) throws {
// Check if the declaration is public, and is referenced cross module.
if decl.accessibility.isExplicitly(.public) {
if try !isReferencedCrossModule(decl) && !isExposedPubliclyByAnotherDeclaration(decl) && !graph.isRetained(decl) {
if !graph.isRetained(decl) &&
!isReferencedCrossModule(decl) &&
!isExposedPubliclyByAnotherDeclaration(decl) &&
!isProtocolIndirectlyReferencedCrossModuleByExtensionMember(decl)
{
// Public accessibility is redundant.
mark(decl)
markExplicitPublicDescendentDeclarations(from: decl)
Expand Down Expand Up @@ -108,17 +112,50 @@ final class RedundantExplicitPublicAccessibilityMarker: SourceGraphMutator {
return nil
}

return referenceDecls.contains { refDecl in
refDecl.accessibility.value == .public || refDecl.accessibility.value == .open
}
return referenceDecls.contains { $0.accessibility.isAccessibleCrossModule }
}

/// A public protocol that is not directly referenced cross-module may still be exposed by a public member declared
/// within an extension that is accessed on a conforming type.
///
/// // TargetA
/// public protocol MyProtocol {}
/// public extension MyProtocol {
/// func someExtensionFunc() {}
/// }
/// public class MyClass: MyProtocol {}
///
/// // TargetB
/// let cls = MyClass()
/// cls.someExtensionFunc()
///
private func isProtocolIndirectlyReferencedCrossModuleByExtensionMember(_ decl: Declaration) -> Bool {
guard decl.kind == .protocol else { return false }

return graph.references(to: decl)
.lazy
.compactMap { ref -> Declaration? in
guard let parent = ref.parent else { return nil }

if parent.kind == .extensionProtocol && parent.name == decl.name {
return parent
}

return nil
}
.contains {
$0.declarations.contains {
$0.accessibility.value == .public && isReferencedCrossModule($0)
}
}
}

private func isReferencedCrossModule(_ decl: Declaration) throws -> Bool {
let referenceModules = try nonTestableModulesReferencing(decl)
private func isReferencedCrossModule(_ decl: Declaration) -> Bool {
let referenceModules = nonTestableModulesReferencing(decl)
return !referenceModules.subtracting(decl.location.file.modules).isEmpty
}

private func nonTestableModulesReferencing(_ decl: Declaration) throws -> Set<String> {
private func nonTestableModulesReferencing(_ decl: Declaration) -> Set<String> {
let referenceFiles = graph.references(to: decl).map { $0.location.file }

let referenceModules = referenceFiles.flatMapSet { file -> Set<String> in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ _ = PublicTypeUsedAsPublicFunctionMetatypeParameterWithGenericReturnTypeRetainer

_ = NotRedundantPublicTestableImportClass().testableProperty

ProtocolIndirectlyReferencedCrossModuleByExtensionMemberImpl().somePublicFunc()

// Typealias
let _: PublicTypealiasWithClosureType? = nil

Expand All @@ -56,4 +58,3 @@ _ = InternalProtocolRefiningPublicProtocolRetainer()

// Closure
let _ = PublicTypeUsedInPublicClosureRetainer().closure

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
public protocol ProtocolIndirectlyReferencedCrossModuleByExtensionMember {}
public extension ProtocolIndirectlyReferencedCrossModuleByExtensionMember {
func somePublicFunc() {}
}
public class ProtocolIndirectlyReferencedCrossModuleByExtensionMemberImpl: ProtocolIndirectlyReferencedCrossModuleByExtensionMember {
public init() {}
}
23 changes: 23 additions & 0 deletions Tests/AccessibilityTests/RedundantPublicAccessibilityTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -229,4 +229,27 @@ class RedundantPublicAccessibilityTest: SourceGraphTestCase {
// Destructured binding control.
assertRedundantPublicAccessibility(.protocol("PublicTypeUsedAsPublicFunctionMetatypeParameterWithGenericReturnType4"))
}

/// A public protocol that is not directly referenced cross-module may still be exposed by a public member declared
/// within an extension that is accessed on a conforming type.
///
/// // TargetA
/// public protocol MyProtocol {}
/// public extension MyProtocol {
/// func someExtensionFunc() {}
/// }
/// public class MyClass: MyProtocol {}
///
/// // TargetB
/// let cls = MyClass()
/// cls.someExtensionFunc()
///
func testPublicProtocolIndirectlyReferencedByExtensionMember() {
Self.index()

assertNotRedundantPublicAccessibility(.protocol("ProtocolIndirectlyReferencedCrossModuleByExtensionMember"))
assertNotRedundantPublicAccessibility(.extensionProtocol("ProtocolIndirectlyReferencedCrossModuleByExtensionMember")) {
self.assertNotRedundantPublicAccessibility(.functionMethodInstance("somePublicFunc()"))
}
}
}

0 comments on commit febffbc

Please sign in to comment.