From 7ea2f4ce11736862262097a98fae543dfbbf6578 Mon Sep 17 00:00:00 2001 From: Ian Leitch Date: Mon, 18 Dec 2023 14:26:05 +0000 Subject: [PATCH] Retain enums conforming to CodingKey when an external Codable protocol is specified. Closes #679 --- CHANGELOG.md | 2 ++ Sources/Frontend/Commands/ScanCommand.swift | 10 +++++++++- .../Mutators/CodingKeyEnumReferenceBuilder.swift | 7 ++++++- .../Mutators/EncodablePropertyRetainer.swift | 4 +++- Sources/Shared/Configuration.swift | 13 +++++++++++++ .../RetentionFixtures/testCodingKeyEnum.swift | 12 ++++++++++++ Tests/PeripheryTests/RetentionTest.swift | 16 ++++++++++++---- 7 files changed, 57 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fd5e8d95..67ca7eaee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/Sources/Frontend/Commands/ScanCommand.swift b/Sources/Frontend/Commands/ScanCommand.swift index 1bc2c121d..25f19ecbb 100644 --- a/Sources/Frontend/Commands/ScanCommand.swift +++ b/Sources/Frontend/Commands/ScanCommand.swift @@ -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 @@ -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) @@ -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) diff --git a/Sources/PeripheryKit/SourceGraph/Mutators/CodingKeyEnumReferenceBuilder.swift b/Sources/PeripheryKit/SourceGraph/Mutators/CodingKeyEnumReferenceBuilder.swift index 6393677a5..f7320dd65 100644 --- a/Sources/PeripheryKit/SourceGraph/Mutators/CodingKeyEnumReferenceBuilder.swift +++ b/Sources/PeripheryKit/SourceGraph/Mutators/CodingKeyEnumReferenceBuilder.swift @@ -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() { @@ -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 { diff --git a/Sources/PeripheryKit/SourceGraph/Mutators/EncodablePropertyRetainer.swift b/Sources/PeripheryKit/SourceGraph/Mutators/EncodablePropertyRetainer.swift index 76404bf2d..c1ead2159 100644 --- a/Sources/PeripheryKit/SourceGraph/Mutators/EncodablePropertyRetainer.swift +++ b/Sources/PeripheryKit/SourceGraph/Mutators/EncodablePropertyRetainer.swift @@ -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() { @@ -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 } } diff --git a/Sources/Shared/Configuration.swift b/Sources/Shared/Configuration.swift index bdae3aac5..a7bf47903 100644 --- a/Sources/Shared/Configuration.swift +++ b/Sources/Shared/Configuration.swift @@ -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] @@ -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 } @@ -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: @@ -323,6 +335,7 @@ public final class Configuration { $disableRedundantPublicAnalysis.reset() $enableUnusedImportsAnalysis.reset() $externalEncodableProtocols.reset() + $externalCodableProtocols.reset() $externalTestCaseClasses.reset() $verbose.reset() $quiet.reset() diff --git a/Tests/Fixtures/RetentionFixtures/testCodingKeyEnum.swift b/Tests/Fixtures/RetentionFixtures/testCodingKeyEnum.swift index 55cb71814..f52c3e352 100644 --- a/Tests/Fixtures/RetentionFixtures/testCodingKeyEnum.swift +++ b/Tests/Fixtures/RetentionFixtures/testCodingKeyEnum.swift @@ -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 + } +} diff --git a/Tests/PeripheryTests/RetentionTest.swift b/Tests/PeripheryTests/RetentionTest.swift index 59447c3bd..8e9c53abd 100644 --- a/Tests/PeripheryTests/RetentionTest.swift +++ b/Tests/PeripheryTests/RetentionTest.swift @@ -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")) @@ -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")) + } } } @@ -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")) {