diff --git a/CHANGELOG.md b/CHANGELOG.md index 498dd6ed9..0857e83e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - In Swift 5.3 and lower, all optional protocol members are now retained in order to workaround a Swift bug. This bug is resolved in Swift 5.4. - Cases of public enums are now also retained when using `--retain-public`. +- Open method parameters are now also retained when using `--retain-public`. - Empty public protocols that have an implementation in extensions are no longer identified as redundant. - Foreign protocol method parameters are no longer identified as unused. diff --git a/Sources/PeripheryKit/Analyzer/Visitors/UnusedParameterRetainer.swift b/Sources/PeripheryKit/Analyzer/Visitors/UnusedParameterRetainer.swift index 2be76c276..7eca97c98 100644 --- a/Sources/PeripheryKit/Analyzer/Visitors/UnusedParameterRetainer.swift +++ b/Sources/PeripheryKit/Analyzer/Visitors/UnusedParameterRetainer.swift @@ -47,7 +47,7 @@ final class UnusedParameterRetainer: SourceGraphVisitor { if configuration.retainUnusedProtocolFuncParams { functionDecl.unusedParameters.forEach { graph.markRetained($0) } } else { - retain(functionDecl.unusedParameters, usedIn: allFunctionDecls) + retain(params: functionDecl.unusedParameters, usedIn: allFunctionDecls) } } } @@ -64,12 +64,12 @@ final class UnusedParameterRetainer: SourceGraphVisitor { + graph.superclasses(of: classDeclaration) + graph.subclasses(of: classDeclaration) - allMethodDeclarations = allClassDeclarations.compactMap { declaration in + allMethodDeclarations = allClassDeclarations.flatMap { declaration in declaration .declarations .lazy .filter { $0.kind == methodDeclaration.kind } - .first { $0.name == methodDeclaration.name } + .filter { $0.name == methodDeclaration.name } } retainIfNeeded( @@ -110,23 +110,28 @@ final class UnusedParameterRetainer: SourceGraphVisitor { } private func retainIfNeeded(params: Set, inOverridenMethods methodDeclarations: [Declaration]) { - let isSuperclassForeign = methodDeclarations.allSatisfy { declaration in - declaration.modifiers.contains("override") + guard let baseDeclaration = methodDeclarations.first(where: { !$0.modifiers.contains("override") }) else { + // Must be overriding a declaration in a foreign class. + return retainAllUnusedParams(inMethods: methodDeclarations) } - if isSuperclassForeign { - // Must be overriding a declaration in a foreign class. - methodDeclarations - .lazy - .flatMap { $0.unusedParameters } - .forEach { graph.markRetained($0) } - } else { - // Retain all params that are used in any of the functions. - retain(params, usedIn: methodDeclarations) + guard baseDeclaration.accessibility.value != .open || !configuration.retainPublic else { + // Parameters can be used in methods that are overridden from the outside + return retainAllUnusedParams(inMethods: methodDeclarations) } + + // Retain all params that are used in any of the functions. + return retain(params: params, usedIn: methodDeclarations) + } + + private func retainAllUnusedParams(inMethods methodDeclarations: [Declaration]) { + methodDeclarations + .lazy + .flatMap { $0.unusedParameters } + .forEach { graph.markRetained($0) } } - private func retain(_ params: Set, usedIn functionDecls: [Declaration]) { + private func retain(params: Set, usedIn functionDecls: [Declaration]) { for param in params { if isParam(param, usedInAnyOf: functionDecls) { graph.markRetained(param) diff --git a/Tests/PeripheryTests/RetentionTest.swift b/Tests/PeripheryTests/RetentionTest.swift index 0343d458e..d29e087f7 100644 --- a/Tests/PeripheryTests/RetentionTest.swift +++ b/Tests/PeripheryTests/RetentionTest.swift @@ -1012,6 +1012,14 @@ class RetentionTest: SourceGraphTestCase { } } + func testRetainsOpenClassParameters() { + analyze(retainPublic: true) { + XCTAssertReferenced((.varParameter, "value"), + descendentOf: (.functionMethodInstance, "doSomething(with:)"), + (.class, "FixtureClass112")) + } + } + func testIgnoreUnusedParamInUnusedFunction() { analyze() { XCTAssertNotReferenced((.class, "FixtureClass105")) diff --git a/Tests/RetentionFixtures/testRetainsOpenClassParameters.swift b/Tests/RetentionFixtures/testRetainsOpenClassParameters.swift new file mode 100644 index 000000000..fbc47e866 --- /dev/null +++ b/Tests/RetentionFixtures/testRetainsOpenClassParameters.swift @@ -0,0 +1,13 @@ +import Foundation + +open class FixtureClass112 { + open func doSomething(with value: Int) { } +} + +public class FixtureClass112Retainer { + var instance: FixtureClass112? + + public func doSomething() { + instance?.doSomething(with: .random(in: 0...10)) + } +}