Merge branch 'master' into xcode_plugin_support

* master: Follow-up changes to #699 Added schemes_arguments to configuration (#699) Add `--retain-encodable-properties` option. Closes #736 # Conflicts: # Sources/XcodeSupport/XcodeProjectDriver.swift
peripheryapp · May 19, 2024 · 6d35eca · 6d35eca
2 parents 1e1db48 + 115df72
commit 6d35eca
Show file tree

Hide file tree

Showing 14 changed files with 128 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@
 ##### Enhancements
 
 - Unused import detection is now enabled by default.
+- Added the `--retain-encodable-properties` option to retain all properties on `Encodable` types only.
+- Added the `--xcode-list-arguments` option to pass additional arguments to `xcodebuild -list`.
 
 ##### Bug Fixes
 

diff --git a/Sources/Frontend/Commands/ScanCommand.swift b/Sources/Frontend/Commands/ScanCommand.swift
@@ -87,9 +87,12 @@ struct ScanCommand: FrontendCommand {
     @Flag(help: "Retain SwiftUI previews")
     var retainSwiftUIPreviews: Bool = defaultConfiguration.$retainSwiftUIPreviews.defaultValue
 
-    @Flag(help: "Retain properties on Codable types")
+    @Flag(help: "Retain properties on Codable types (including Encodable and Decodable)")
     var retainCodableProperties: Bool = defaultConfiguration.$retainCodableProperties.defaultValue
 
+    @Flag(help: "Retain properties on Encodable types only")
+    var retainEncodableProperties: Bool = defaultConfiguration.$retainEncodableProperties.defaultValue
+
     @Flag(help: "Automatically remove code that can be done so safely without introducing build errors (experimental)")
     var autoRemove: Bool = defaultConfiguration.$autoRemove.defaultValue
 
@@ -169,6 +172,7 @@ struct ScanCommand: FrontendCommand {
         configuration.apply(\.$buildArguments, buildArguments)
         configuration.apply(\.$relativeResults, relativeResults)
         configuration.apply(\.$retainCodableProperties, retainCodableProperties)
+        configuration.apply(\.$retainEncodableProperties, retainEncodableProperties)
         configuration.apply(\.$jsonPackageManifestPath, jsonPackageManifestPath)
 
         try scanBehavior.main { project in

diff --git a/Sources/PeripheryKit/SourceGraph/Mutators/CodablePropertyRetainer.swift b/Sources/PeripheryKit/SourceGraph/Mutators/CodablePropertyRetainer.swift
@@ -11,14 +11,23 @@ final class CodablePropertyRetainer: SourceGraphMutator {
     }
 
     func mutate() {
-        guard configuration.retainCodableProperties else { return }
+        if configuration.retainCodableProperties {
+            for decl in graph.declarations(ofKinds: Declaration.Kind.discreteConformableKinds) {
+                guard graph.isCodable(decl) else { continue }
 
-        for decl in graph.declarations(ofKinds: Declaration.Kind.discreteConformableKinds) {
-            guard graph.isCodable(decl) else { continue }
+                for decl in decl.declarations {
+                    guard decl.kind == .varInstance else { continue }
+                    graph.markRetained(decl)
+                }
+            }
+        } else if configuration.retainEncodableProperties {
+            for decl in graph.declarations(ofKinds: Declaration.Kind.discreteConformableKinds) {
+                guard graph.isEncodable(decl) else { continue }
 
-            for decl in decl.declarations {
-                guard decl.kind == .varInstance else { continue }
-                graph.markRetained(decl)
+                for decl in decl.declarations {
+                    guard decl.kind == .varInstance else { continue }
+                    graph.markRetained(decl)
+                }
             }
         }
     }

diff --git a/Sources/PeripheryKit/SourceGraph/SourceGraph.swift b/Sources/PeripheryKit/SourceGraph/SourceGraph.swift
@@ -362,4 +362,13 @@ public final class SourceGraph {
             return [.protocol, .typealias].contains($0.kind) && codableTypes.contains(name)
         }
     }
+
+    func isEncodable(_ decl: Declaration) -> Bool {
+        let encodableTypes = ["Encodable"] + configuration.externalEncodableProtocols + configuration.externalCodableProtocols
+
+        return inheritedTypeReferences(of: decl).contains {
+            guard let name = $0.name else { return false }
+            return [.protocol, .typealias].contains($0.kind) && encodableTypes.contains(name)
+        }
+    }
 }
diff --git a/Sources/Shared/Configuration.swift b/Sources/Shared/Configuration.swift
@@ -41,6 +41,9 @@ public final class Configuration {
     @Setting(key: "build_arguments", defaultValue: [])
     public var buildArguments: [String]
 
+    @Setting(key: "xcode_list_arguments", defaultValue: [])
+    public var xcodeListArguments: [String]
+
     @Setting(key: "retain_assign_only_property_types", defaultValue: [], valueSanitizer: PropertyTypeSanitizer.sanitize)
     public var retainAssignOnlyPropertyTypes: [String]
 
@@ -83,6 +86,9 @@ public final class Configuration {
     @Setting(key: "retain_codable_properties", defaultValue: false)
     public var retainCodableProperties: Bool
 
+    @Setting(key: "retain_encodable_properties", defaultValue: false)
+    public var retainEncodableProperties: Bool
+
     @Setting(key: "auto_remove", defaultValue: false)
     public var autoRemove: Bool
 
@@ -254,13 +260,21 @@ public final class Configuration {
             config[$buildArguments.key] = buildArguments
         }
 
+        if $xcodeListArguments.hasNonDefaultValue {
+            config[$xcodeListArguments.key] = xcodeListArguments
+        }
+
         if $relativeResults.hasNonDefaultValue {
             config[$relativeResults.key] = relativeResults
         }
 
         if $retainCodableProperties.hasNonDefaultValue {
             config[$retainCodableProperties.key] = retainCodableProperties
         }
+
+        if $retainEncodableProperties.hasNonDefaultValue {
+            config[$retainEncodableProperties.key] = retainEncodableProperties
+        }
 
         if $jsonPackageManifestPath.hasNonDefaultValue {
             config[$jsonPackageManifestPath.key] = jsonPackageManifestPath
@@ -349,10 +363,14 @@ public final class Configuration {
                 $cleanBuild.assign(value)
             case $buildArguments.key:
                 $buildArguments.assign(value)
+            case $xcodeListArguments.key:
+                $xcodeListArguments.assign(value)
             case $relativeResults.key:
                 $relativeResults.assign(value)
             case $retainCodableProperties.key:
                 $retainCodableProperties.assign(value)
+            case $retainEncodableProperties.key:
+                $retainEncodableProperties.assign(value)
             case $jsonPackageManifestPath.key:
                 $jsonPackageManifestPath.assign(value)
             default:
@@ -394,8 +412,10 @@ public final class Configuration {
         $skipSchemesValidation.reset()
         $cleanBuild.reset()
         $buildArguments.reset()
+        $xcodeListArguments.reset()
         $relativeResults.reset()
         $retainCodableProperties.reset()
+        $retainEncodableProperties.reset()
         $jsonPackageManifestPath.reset()
     }
 

diff --git a/Sources/XcodeSupport/XcodeProject.swift b/Sources/XcodeSupport/XcodeProject.swift
@@ -105,8 +105,8 @@ final class XcodeProject: XcodeProjectlike {
         }
     }
 
-    func schemes() throws -> Set<String> {
-        try xcodebuild.schemes(project: self)
+    func schemes(additionalArguments: [String]) throws -> Set<String> {
+        try xcodebuild.schemes(project: self, additionalArguments: additionalArguments)
     }
 }
 

diff --git a/Sources/XcodeSupport/XcodeProjectDriver.swift b/Sources/XcodeSupport/XcodeProjectDriver.swift
@@ -52,7 +52,9 @@ public final class XcodeProjectDriver {
             schemes = Set(configuration.schemes)
         } else {
             // Ensure schemes exist within the project
-            schemes = try project.schemes().filter { configuration.schemes.contains($0) }
+            schemes = try project.schemes(
+                additionalArguments: configuration.xcodeListArguments
+            ).filter { configuration.schemes.contains($0) }
             let validSchemeNames = schemes.mapSet { $0 }
 
             if let scheme = Set(configuration.schemes).subtracting(validSchemeNames).first {

diff --git a/Sources/XcodeSupport/XcodeProjectSetupGuide.swift b/Sources/XcodeSupport/XcodeProjectSetupGuide.swift
@@ -43,7 +43,10 @@ public final class XcodeProjectSetupGuide: SetupGuideHelpers, ProjectSetupGuide
             print(colorize("Select build targets to analyze:", .bold))
             configuration.targets = select(multiple: targets, allowAll: true).selectedValues
 
-            let schemes = try filter(project.schemes(), project).map { $0 }.sorted()
+            let schemes = try filter(
+                project.schemes(additionalArguments: configuration.xcodeListArguments),
+                project
+            ).map { $0 }.sorted()
 
             print(colorize("\nSelect the schemes necessary to build your chosen targets:", .bold))
             configuration.schemes = select(multiple: schemes, allowAll: false).selectedValues
@@ -82,7 +85,11 @@ public final class XcodeProjectSetupGuide: SetupGuideHelpers, ProjectSetupGuide
     private func getPodSchemes(in project: XcodeProjectlike) throws -> Set<String> {
         let path = project.sourceRoot.appending("Pods/Pods.xcodeproj")
         guard path.exists else { return [] }
-        return try xcodebuild.schemes(type: "project", path: path.lexicallyNormalized().string)
+        return try xcodebuild.schemes(
+            type: "project",
+            path: path.lexicallyNormalized().string,
+            additionalArguments: configuration.xcodeListArguments
+        )
     }
 
     private func filter(_ schemes: Set<String>, _ project: XcodeProjectlike) throws -> [String] {

diff --git a/Sources/XcodeSupport/XcodeProjectlike.swift b/Sources/XcodeSupport/XcodeProjectlike.swift
@@ -10,7 +10,7 @@ protocol XcodeProjectlike: AnyObject {
     var name: String { get }
     var sourceRoot: FilePath { get }
 
-    func schemes() throws -> Set<String>
+    func schemes(additionalArguments: [String]) throws -> Set<String>
 }
 
 extension XcodeProjectlike {

diff --git a/Sources/XcodeSupport/XcodeWorkspace.swift b/Sources/XcodeSupport/XcodeWorkspace.swift
@@ -42,8 +42,8 @@ final class XcodeWorkspace: XcodeProjectlike {
         }
     }
 
-    func schemes() throws -> Set<String> {
-        try xcodebuild.schemes(project: self)
+    func schemes(additionalArguments: [String]) throws -> Set<String> {
+        try xcodebuild.schemes(project: self, additionalArguments: additionalArguments)
     }
 
     // MARK: - Private

diff --git a/Sources/XcodeSupport/Xcodebuild.swift b/Sources/XcodeSupport/Xcodebuild.swift
@@ -39,20 +39,7 @@ public final class Xcodebuild {
             "INDEX_ENABLE_DATA_STORE=\"YES\""
         ]
 
-        // Apply quotes to additional option values is needed.
-        var quotedArguments = additionalArguments
-
-        for (i, arg) in additionalArguments.enumerated() {
-            if arg.hasPrefix("-"),
-               let value = additionalArguments[safe: i + 1],
-               !value.hasPrefix("-"),
-               !value.hasPrefix("\""),
-               !value.hasPrefix("\'")
-            {
-                quotedArguments[i + 1] = "\"\(value)\""
-            }
-        }
-
+        let quotedArguments = quote(arguments: additionalArguments)
         let xcodebuild = "xcodebuild \((args + [cmd] + envs + quotedArguments).joined(separator: " "))"
         return try shell.exec(["/bin/sh", "-c", xcodebuild])
     }
@@ -71,18 +58,24 @@ public final class Xcodebuild {
         return path
     }
 
-    func schemes(project: XcodeProjectlike) throws -> Set<String> {
-        try schemes(type: project.type, path: project.path.lexicallyNormalized().string)
+    func schemes(project: XcodeProjectlike, additionalArguments: [String]) throws -> Set<String> {
+        try schemes(
+            type: project.type,
+            path: project.path.lexicallyNormalized().string,
+            additionalArguments: additionalArguments
+        )
     }
 
-    func schemes(type: String, path: String) throws -> Set<String> {
+    func schemes(type: String, path: String, additionalArguments: [String]) throws -> Set<String> {
         let args = [
             "-\(type)", path,
             "-list",
             "-json"
         ]
 
-        let lines = try shell.exec(["xcodebuild"] + args, stderr: false).split(separator: "\n").map { String($0).trimmed }
+        let quotedArguments = quote(arguments: additionalArguments)
+        let xcodebuild = "xcodebuild \((args + quotedArguments).joined(separator: " "))"
+        let lines = try shell.exec(["/bin/sh", "-c", xcodebuild], stderr: false).split(separator: "\n").map { String($0).trimmed }
 
         // xcodebuild may output unrelated warnings, we need to strip them out otherwise
         // JSON parsing will fail.
@@ -125,4 +118,21 @@ public final class Xcodebuild {
 
         return try Constants.cachePath().appending("DerivedData-\(xcodeVersionHash)-\(projectHash)-\(schemesHash)")
     }
+
+    private func quote(arguments: [String]) -> [String] {
+        var quotedArguments = arguments
+
+        for (i, arg) in arguments.enumerated() {
+            if arg.hasPrefix("-"),
+               let value = arguments[safe: i + 1],
+               !value.hasPrefix("-"),
+               !value.hasPrefix("\""),
+               !value.hasPrefix("\'")
+            {
+                quotedArguments[i + 1] = "\"\(value)\""
+            }
+        }
+
+        return quotedArguments
+    }
 }
diff --git a/Tests/Fixtures/RetentionFixtures/testRetainsEncodableProperties.swift b/Tests/Fixtures/RetentionFixtures/testRetainsEncodableProperties.swift
@@ -0,0 +1,9 @@
+import Foundation
+
+public struct FixtureStruct15: Encodable {
+    let unused: Int
+
+    init(unused: Int) {
+        self.unused = unused
+    }
+}
diff --git a/Tests/PeripheryTests/RetentionTest.swift b/Tests/PeripheryTests/RetentionTest.swift
@@ -1034,6 +1034,28 @@ final class RetentionTest: FixtureSourceGraphTestCase {
         }
     }
 
+    func testRetainsEncodableProperties() {
+        configuration.retainEncodableProperties = false
+        configuration.retainAssignOnlyProperties = false
+
+        analyze(retainPublic: true) {
+            assertReferenced(.struct("FixtureStruct15")) {
+                self.assertNotReferenced(.functionConstructor("init(unused:)"))
+                self.assertAssignOnlyProperty(.varInstance("unused"))
+            }
+        }
+
+        configuration.retainEncodableProperties = true
+
+        analyze(retainPublic: true) {
+            assertReferenced(.struct("FixtureStruct15")) {
+                self.assertNotReferenced(.functionConstructor("init(unused:)"))
+                self.assertReferenced(.varInstance("unused"))
+                self.assertNotAssignOnlyProperty(.varInstance("unused"))
+            }
+        }
+    }
+
     func testRetainsFilesOption() {
         configuration.retainFiles = [testFixturePath.string]
 

diff --git a/Tests/XcodeTests/XcodebuildTest.swift b/Tests/XcodeTests/XcodebuildTest.swift
@@ -36,7 +36,7 @@ class XcodebuildSchemesTest: XCTestCase {
     func testParseSchemes() {
         for output in XcodebuildListOutputs {
             shell.output = output
-            let schemes = try! xcodebuild.schemes(project: project)
+            let schemes = try! xcodebuild.schemes(project: project, additionalArguments: [])
             XCTAssertEqual(schemes, ["SchemeA", "SchemeB"])
         }
     }