Skip to content

Commit

Permalink
Retain enums conforming to CodingKey when an external Codable protoco…
Browse files Browse the repository at this point in the history
…l is specified. Closes #679
  • Loading branch information
ileitch committed Dec 18, 2023
1 parent 61b86ae commit 7ea2f4c
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
##### Enhancements

- Add experimental unused import analysis.
- The option `--external-encodable-protocols` is deprecated, use `--external-codable-protocols` instead.

##### Bug Fixes

- Subscript functions required by `@dynamicMemberLookup` are now retained.
- A newline is no longer printed before non-Xcode formatted results.
- `--external-codable-protocols` now retains enums that conform to `CodingKey`.

## 2.17.1 (2023-12-04)

Expand Down
10 changes: 9 additions & 1 deletion Sources/Frontend/Commands/ScanCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ struct ScanCommand: FrontendCommand {
@Option(help: "Comma-separated list of property types to retain if the property is assigned, but never read", transform: split(by: ","))
var retainAssignOnlyPropertyTypes: [String] = defaultConfiguration.$retainAssignOnlyPropertyTypes.defaultValue

@Option(help: "Comma-separated list of external protocols that inherit Encodable. Properties of types conforming to these protocols will be retained", transform: split(by: ","))
@Option(help: .private, transform: split(by: ","))
var externalEncodableProtocols: [String] = defaultConfiguration.$externalEncodableProtocols.defaultValue

@Option(parsing: .upToNextOption, help: "Names of external protocols that inherit Codable. Properties and CodingKey enums of types conforming to these protocols will be retained")
var externalCodableProtocols: [String] = defaultConfiguration.$externalCodableProtocols.defaultValue

@Option(parsing: .upToNextOption, help: "Names of XCTestCase subclasses that reside in external targets")
var externalTestCaseClasses: [String] = defaultConfiguration.$externalTestCaseClasses.defaultValue

Expand Down Expand Up @@ -111,6 +114,10 @@ struct ScanCommand: FrontendCommand {
try scanBehavior.setup(config).get()
}

if !externalEncodableProtocols.isEmpty {
Logger().warn("The option '--external-encodable-protocols' is deprecated, use '--external-codable-protocols' instead.")
}

let configuration = Configuration.shared
configuration.guidedSetup = setup
configuration.apply(\.$workspace, workspace)
Expand All @@ -132,6 +139,7 @@ struct ScanCommand: FrontendCommand {
configuration.apply(\.$disableRedundantPublicAnalysis, disableRedundantPublicAnalysis)
configuration.apply(\.$enableUnusedImportsAnalysis, enableUnusedImportAnalysis)
configuration.apply(\.$externalEncodableProtocols, externalEncodableProtocols)
configuration.apply(\.$externalCodableProtocols, externalCodableProtocols)
configuration.apply(\.$externalTestCaseClasses, externalTestCaseClasses)
configuration.apply(\.$verbose, verbose)
configuration.apply(\.$quiet, quiet)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import Shared
/// Builds a reference from a `Codable` conforming type to any child enum that conforms to `CodingKey`.
final class CodingKeyEnumReferenceBuilder: SourceGraphMutator {
private let graph: SourceGraph
private let configuration: Configuration

required init(graph: SourceGraph, configuration: Configuration) {
self.graph = graph
self.configuration = configuration
}

func mutate() {
Expand All @@ -17,8 +19,11 @@ final class CodingKeyEnumReferenceBuilder: SourceGraphMutator {
$0.kind == .protocol && $0.name == "CodingKey"
}

let codableTypes = ["Codable", "Decodable", "Encodable"] + configuration.externalEncodableProtocols + configuration.externalCodableProtocols

let isParentCodable = graph.inheritedTypeReferences(of: parent).contains {
[.protocol, .typealias].contains($0.kind) && ["Codable", "Decodable", "Encodable"].contains($0.name)
guard let name = $0.name else { return false }
return [.protocol, .typealias].contains($0.kind) && codableTypes.contains(name)
}

if isCodingKey && isParentCodable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import Shared
final class EncodablePropertyRetainer: SourceGraphMutator {
private let graph: SourceGraph
private let configuration: Configuration
private let externalCodableProtocols: [String]

required init(graph: SourceGraph, configuration: Configuration) {
self.graph = graph
self.configuration = configuration
self.externalCodableProtocols = configuration.externalEncodableProtocols + configuration.externalCodableProtocols
}

func mutate() {
Expand Down Expand Up @@ -41,7 +43,7 @@ final class EncodablePropertyRetainer: SourceGraphMutator {
}

if let name = ref.name {
if graph.isExternal(ref), configuration.externalEncodableProtocols.contains(name) {
if graph.isExternal(ref), externalCodableProtocols.contains(name) {
return true
}
}
Expand Down
13 changes: 13 additions & 0 deletions Sources/Shared/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ public final class Configuration {
@Setting(key: "external_encodable_protocols", defaultValue: [])
public var externalEncodableProtocols: [String]

@Setting(key: "external_codable_protocols", defaultValue: [])
public var externalCodableProtocols: [String]

@Setting(key: "external_test_case_classes", defaultValue: [])
public var externalTestCaseClasses: [String]

Expand Down Expand Up @@ -167,6 +170,10 @@ public final class Configuration {
config[$externalEncodableProtocols.key] = externalEncodableProtocols
}

if $externalCodableProtocols.hasNonDefaultValue {
config[$externalCodableProtocols.key] = externalCodableProtocols
}

if $externalTestCaseClasses.hasNonDefaultValue {
config[$externalTestCaseClasses.key] = externalTestCaseClasses
}
Expand Down Expand Up @@ -264,7 +271,12 @@ public final class Configuration {
case $retainAssignOnlyPropertyTypes.key:
$retainAssignOnlyPropertyTypes.assign(value)
case $externalEncodableProtocols.key:
if !externalEncodableProtocols.isEmpty {
logger.warn("The option '--external-encodable-protocols' is deprecated, use '--external-codable-protocols' instead.")
}
$externalEncodableProtocols.assign(value)
case $externalCodableProtocols.key:
$externalCodableProtocols.assign(value)
case $externalTestCaseClasses.key:
$externalTestCaseClasses.assign(value)
case $retainObjcAccessible.key:
Expand Down Expand Up @@ -323,6 +335,7 @@ public final class Configuration {
$disableRedundantPublicAnalysis.reset()
$enableUnusedImportsAnalysis.reset()
$externalEncodableProtocols.reset()
$externalCodableProtocols.reset()
$externalTestCaseClasses.reset()
$verbose.reset()
$quiet.reset()
Expand Down
12 changes: 12 additions & 0 deletions Tests/Fixtures/RetentionFixtures/testCodingKeyEnum.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,15 @@ public struct FixtureClass120 {
}

extension FixtureClass120: Decodable {}

// CustomStringConvertible doesn't actually inherit Codable, we're just using it because we don't have an external
// module in which to declare our own type.
public struct FixtureClass218: CustomStringConvertible {
public var description: String = ""

let someVar: String

enum CodingKeys: CodingKey {
case someVar
}
}
16 changes: 12 additions & 4 deletions Tests/PeripheryTests/RetentionTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,11 @@ final class RetentionTest: FixtureSourceGraphTestCase {
}

func testCodingKeyEnum() {
let configuration = Configuration.shared
// CustomStringConvertible doesn't actually inherit Codable, we're just using it because we don't have an
// external module in which to declare our own type.
configuration.externalCodableProtocols = ["CustomStringConvertible"]

analyze(retainPublic: true) {
assertReferenced(.class("FixtureClass74")) {
self.assertReferenced(.enum("CodingKeys"))
Expand All @@ -573,9 +578,12 @@ final class RetentionTest: FixtureSourceGraphTestCase {
// Not referenced because the enclosing class does not conform to Codable.
self.assertNotReferenced(.enum("CodingKeys"))
}
assertReferenced(.struct("FixtureClass120")) {
self.assertReferenced(.enum("CodingKeys"))
}
assertReferenced(.struct("FixtureClass120")) {
self.assertReferenced(.enum("CodingKeys"))
}
assertReferenced(.struct("FixtureClass218")) {
self.assertReferenced(.enum("CodingKeys"))
}
}
}

Expand Down Expand Up @@ -960,7 +968,7 @@ final class RetentionTest: FixtureSourceGraphTestCase {
let configuration = Configuration.shared
// CustomStringConvertible doesn't actually inherit Encodable, we're just using it because we don't have an
// external module in which to declare our own type.
configuration.externalEncodableProtocols = ["CustomStringConvertible"]
configuration.externalCodableProtocols = ["CustomStringConvertible"]

analyze(retainPublic: true) {
self.assertReferenced(.class("FixtureClass204")) {
Expand Down

0 comments on commit 7ea2f4c

Please sign in to comment.