Detect assign-only properties on structs with synthesized initializers

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 - Add experimental unused import analysis.
 - The option `--external-encodable-protocols` is deprecated, use `--external-codable-protocols` instead.
+- Assign-only properties on structs with synthesized initializers are now detected.
 
 ##### Bug Fixes
 
@@ -23,6 +24,7 @@
 - Fix public accessibility analysis false-positive for retained/ignored declarations.
 - Fix public accessibility analysis false-positive for enum case parameter types.
 - Fix public accessibility analysis false-positive for properties initialized with generic specialized types.
+- Types associated with assign-only properties are no longer identified as unused until the property is removed.
 
 ## 2.17.1 (2023-12-04)
 

Sources/PeripheryKit/Formatters/OutputDeclarationFilter.swift

@@ -14,7 +14,7 @@ public final class OutputDeclarationFilter {
 
     public func filter(_ declarations: [ScanResult]) -> [ScanResult] {
         if configuration.reportInclude.isEmpty && configuration.reportExclude.isEmpty {
-            return declarations
+            return declarations.sorted { $0.declaration < $1.declaration }
         }
 
         return declarations

Sources/PeripheryKit/SourceGraph/Mutators/AssignOnlyPropertyReferenceEliminator.swift

@@ -16,59 +16,19 @@ final class AssignOnlyPropertyReferenceEliminator: SourceGraphMutator {
     func mutate() throws {
         guard !configuration.retainAssignOnlyProperties else { return }
 
-        let setters = graph.declarations(ofKind: .functionAccessorSetter)
-        var assignOnlyProperties: [Declaration: [Reference]] = [:]
-
-        for setter in setters {
-            let references = graph.references(to: setter)
-
-            for reference in references {
-                // Ensure the property being assigned is simple.
-                guard
-                    let caller = reference.parent,
-                    let property = setter.parent,
-                    !property.isComplexProperty
-                else { continue }
-
-                // Ensure the property hasn't been been explicitly retained, e.g by a comment command.
-                guard !graph.isRetained(property) else { continue }
-
-                // A protocol property can technically be assigned and never used when the protocol is used as an existential
-                // type, however communicating that succinctly would be very tricky, and most likely just lead to confusion.
-                // Here we filter out protocol properties and thus restrict this analysis only to concrete properties.
-                guard property.parent?.kind != .protocol else { continue }
-
-                // Find all references to the property at the same call site as the setter reference.
-                let propertyReferences = caller.references.filter {
-                    property.usrs.contains($0.usr) && $0.location == reference.location
-                }
-
-                // Determine if the containing method contains references to the property's getter accessor.
-                let propertyGetterUSRs = property.declarations
-                    .filter { $0.kind == .functionAccessorGetter }
-                    .flatMap { $0.usrs }
-                let hasGetterReference = caller.references
-                    .contains { $0.kind == .functionAccessorGetter && propertyGetterUSRs.contains($0.usr) }
-
-                if !hasGetterReference {
-                    propertyReferences.forEach {
-                        assignOnlyProperties[property, default: []].append($0)
-                    }
-                }
-            }
-        }
-
-        for (property, references) in assignOnlyProperties {
-            if let declaredType = property.declaredType,
-               retainAssignOnlyPropertyTypes.contains(declaredType) {
-                continue
-            }
-
-            graph.markPotentialAssignOnlyProperty(property)
-
-            for reference in references {
-                graph.remove(reference)
-            }
+        for property in graph.declarations(ofKinds: Declaration.Kind.variableKinds) {
+            guard let declaredType = property.declaredType,
+                  !retainAssignOnlyPropertyTypes.contains(declaredType),
+                  !graph.isRetained(property),
+                  !property.isComplexProperty,
+                  !graph.references(to: property).contains(where: { $0.parent?.parent?.kind == .protocol }),
+                  let setter = property.declarations.first(where: { $0.kind == .functionAccessorSetter }),
+                  let getter = property.declarations.first(where: { $0.kind == .functionAccessorGetter }),
+                  graph.hasReferences(to: setter),
+                  !graph.hasReferences(to: getter)
+            else { continue }
+
+            graph.markAssignOnlyProperty(property)
         }
     }
 }

Sources/PeripheryKit/SourceGraph/Mutators/CodingKeyEnumReferenceBuilder.swift

@@ -4,11 +4,9 @@ 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() {
@@ -19,13 +17,6 @@ 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 {
-                guard let name = $0.name else { return false }
-                return [.protocol, .typealias].contains($0.kind) && codableTypes.contains(name)
-            }
-
             guard isCodingKey else { continue }
 
             // Retain each enum element.
@@ -34,7 +25,7 @@ final class CodingKeyEnumReferenceBuilder: SourceGraphMutator {
                 graph.markRetained(elem)
             }
 
-            if isParentCodable {
+            if graph.isCodable(parent) {
                 // Build a reference from the Codable type to the CodingKey enum.
                 for usr in enumDeclaration.usrs {
                     let newReference = Reference(kind: .enum, usr: usr, location: enumDeclaration.location)

Sources/PeripheryKit/SourceGraph/Mutators/DefaultConstructorReferenceBuilder.swift

@@ -17,7 +17,9 @@ final class DefaultConstructorReferenceBuilder: SourceGraphMutator {
 
     private func referenceDefaultConstructors() {
         let defaultConstructors = graph.declarations(ofKind: .functionConstructor).filter {
-            $0.name == "init()"
+            // Some initializers are referenced internally, e.g by JSONEncoder/Decoder so we need
+            // to assume they are referenced.
+            $0.name == "init()" ||  $0.isImplicit
         }
 
         defaultConstructors.forEach { constructor in

Sources/PeripheryKit/SourceGraph/Mutators/StructImplicitInitializerReferenceBuilder.swift

@@ -0,0 +1,65 @@
+import Foundation
+import Shared
+
+/// Builds references from struct implicit initializers to the properties it assigns.
+final class StructImplicitInitializerReferenceBuilder: SourceGraphMutator {
+    private let graph: SourceGraph
+
+    init(graph: SourceGraph, configuration: Configuration) {
+        self.graph = graph
+    }
+
+    func mutate() throws {
+        for structDecl in graph.declarations(ofKind: .struct) {
+            let implicitInitDecl = structDecl.declarations.first { $0.kind == .functionConstructor && $0.isImplicit }
+
+            guard let implicitInitDecl, let name = implicitInitDecl.name else { continue }
+
+            let propertyNames = name
+                .dropFirst("init(".count)
+                .dropLast(")".count)
+                .split(separator: ":")
+                .map(String.init)
+
+            let initPropertyDecls = structDecl.declarations.filter {
+                guard $0.kind == .varInstance, let name = $0.name, propertyNames.contains(name)
+                else { return false }
+                return true
+            }
+
+            for propertyDecl in initPropertyDecls {
+                guard let setterDecl = propertyDecl.declarations.first(where: { $0.kind == .functionAccessorSetter })
+                else { continue }
+
+                for decl in [propertyDecl, setterDecl] {
+                    for usr in decl.usrs {
+                        let ref = Reference(kind: decl.kind, usr: usr, location: implicitInitDecl.location)
+                        ref.name = decl.name
+                        ref.parent = implicitInitDecl
+                        graph.add(ref, from: implicitInitDecl)
+                    }
+                }
+            }
+
+            // The index contains references to properties at the call site of the implicit
+            // initializer. This pattern is contrary to explicit initializers, where only the
+            // initializer is referenced. Now that we've built the property references stemming from
+            // the implicit initializer, we can remove the additional property references at the
+            // call site. This enables assign-only property detection for structs using implicit
+            // initializers.
+            for initRef in graph.references(to: implicitInitDecl) {
+                guard let caller = initRef.parent, caller != structDecl else { continue }
+
+                let sortedReferences = caller.references.sorted()
+
+                for initPropertyDecl in initPropertyDecls {
+                    let firstRef = sortedReferences.first { initPropertyDecl.usrs.contains($0.usr) && $0.location > initRef.location }
+
+                    if let firstRef {
+                        graph.remove(firstRef)
+                    }
+                }
+            }
+        }
+    }
+}

Sources/PeripheryKit/SourceGraph/SourceGraph.swift

@@ -19,13 +19,18 @@ public final class SourceGraph {
     private(set) var allReferencesByUsr: [String: Set<Reference>] = [:]
     private(set) var indexedModules: Set<String> = []
     private(set) var unusedModuleImports: Set<Declaration> = []
+    private(set) var assignOnlyProperties: Set<Declaration> = []
 
-    private var potentialAssignOnlyProperties: Set<Declaration> = []
     private var allDeclarationsByKind: [Declaration.Kind: Set<Declaration>] = [:]
     private var allExplicitDeclarationsByUsr: [String: Declaration] = [:]
     private var moduleToExportingModules: [String: Set<String>] = [:]
 
     private let lock = UnfairLock()
+    private let configuration: Configuration
+
+    init(configuration: Configuration = .shared) {
+        self.configuration = configuration
+    }
 
     public func indexingComplete() {
         rootDeclarations = allDeclarations.filter { $0.parent == nil }
@@ -36,10 +41,6 @@ public final class SourceGraph {
         allDeclarations.subtracting(usedDeclarations)
     }
 
-    var assignOnlyProperties: Set<Declaration> {
-        return potentialAssignOnlyProperties.intersection(unusedDeclarations)
-    }
-
     func declarations(ofKind kind: Declaration.Kind) -> Set<Declaration> {
         allDeclarationsByKind[kind] ?? []
     }
@@ -98,9 +99,9 @@ public final class SourceGraph {
         _ = retainedDeclarations.insert(declaration)
     }
 
-    func markPotentialAssignOnlyProperty(_ declaration: Declaration) {
+    func markAssignOnlyProperty(_ declaration: Declaration) {
         withLock {
-            _ = potentialAssignOnlyProperties.insert(declaration)
+            _ = assignOnlyProperties.insert(declaration)
         }
     }
 
@@ -149,7 +150,7 @@ public final class SourceGraph {
         allDeclarationsByKind[declaration.kind]?.remove(declaration)
         rootDeclarations.remove(declaration)
         usedDeclarations.remove(declaration)
-        potentialAssignOnlyProperties.remove(declaration)
+        assignOnlyProperties.remove(declaration)
         declaration.usrs.forEach { allExplicitDeclarationsByUsr.removeValue(forKey: $0) }
     }
 
@@ -341,4 +342,13 @@ public final class SourceGraph {
                 result.formUnion(allOverrideDeclarations(fromBase: decl))
             }
     }
+
+    func isCodable(_ decl: Declaration) -> Bool {
+        let codableTypes = ["Codable", "Decodable", "Encodable"] + configuration.externalEncodableProtocols + configuration.externalCodableProtocols
+
+        return inheritedTypeReferences(of: decl).contains {
+            guard let name = $0.name else { return false }
+            return [.protocol, .typealias].contains($0.kind) && codableTypes.contains(name)
+        }
+    }
 }

Sources/PeripheryKit/SourceGraph/SourceGraphMutatorRunner.swift

@@ -27,10 +27,11 @@ public final class SourceGraphMutatorRunner {
         ProtocolExtensionReferenceBuilder.self,
         ProtocolConformanceReferenceBuilder.self,
         ExternalTypeProtocolConformanceReferenceRemover.self,
-        DefaultConstructorReferenceBuilder.self,
         ComplexPropertyAccessorReferenceBuilder.self,
         EnumCaseReferenceBuilder.self,
         DynamicMemberLookupReferenceBuilder.self,
+        DefaultConstructorReferenceBuilder.self,
+        StructImplicitInitializerReferenceBuilder.self,
 
         UnusedParameterRetainer.self,
         AssetReferenceRetainer.self,

Tests/Fixtures/RetentionFixtures/testCodingKeyEnum.swift

@@ -62,23 +62,3 @@ public struct FixtureClass218: CustomStringConvertible {
         case someVar
     }
 }
-
-public class FixtureClass220: Encodable {
-    public var someUsedVar: String?
-    var someUnusedVar: String?
-
-    enum CodingKeys: CodingKey {
-        case someUsedVar
-        case someUnusedVar
-    }
-}
-
-public struct FixtureStruct220: Encodable {
-    public var someUsedVar: String?
-    var someUnusedVar: String?
-
-    enum CodingKeys: String, CodingKey {
-        case someUsedVar
-        case someUnusedVar
-    }
-}

Tests/Fixtures/RetentionFixtures/testStructImplicitInitializer.swift

@@ -0,0 +1,18 @@
+import Foundation
+
+public struct FixtureStruct13_Codable: Codable {
+    let assignOnly: Int
+}
+
+public struct FixtureStruct13_NotCodable {
+    let assignOnly: Int
+    let used: Int
+}
+
+public struct FixtureStruct13Retainer {
+    public func retain() throws {
+        let data = "".data(using: .utf8)!
+        _ = try JSONDecoder().decode(FixtureStruct13_Codable.self, from: data)
+        _ = FixtureStruct13_NotCodable(assignOnly: 0, used: 0).used
+    }
+}