Foundation: more idiomatic Swift in NSKeyedArchiver (#3160)

lhoward · web-flow · commit 7f085950b0a7 · 2022-12-12T10:55:52.000-08:00
* Foundation: use class cluster in NSValue implementation

* Foundation: use first(where:) in NSSpecialValue

Use first(where:) in NSSpecialValue instead of iterating loop, as it is more
idiomatic.

* Foundation: collapse guard conditional in NSSpecialValue

* Foundation: more idiomatic swift in NSKeyedArchiver

Eliminate some force-unwrap/force-casts in NSKeyedArchiver and
NSKeyedUnarchiver. Some notes about whether unarchiver delegate is called when
decoding nil objects.

* Foundation: more idiomatic Swift in NSKeyedUnarchiver

Unwrapped variable can have same name as optional.
diff --git a/Docs/Archiving.md b/Docs/Archiving.md
@@ -37,7 +37,7 @@ There is a preliminary implementation of NSKeyedArchiver and NSKeyedUnarchiver w
 * NSTimeZone
 * NSURL
 * NSUUID
-* NSValue (via class cluster hack)
+* NSValue
 
 ## TODO
 
diff --git a/Sources/Foundation/NSGeometry.swift b/Sources/Foundation/NSGeometry.swift
@@ -1085,42 +1085,38 @@ public func NSRectFromString(_ aString: String) -> NSRect {
 
 extension NSValue {
     public convenience init(point: NSPoint) {
-        self.init()
-        self._concreteValue = NSSpecialValue(point)
+        self.init { NSSpecialValue(point) as! Self }
     }
     
     public convenience init(size: NSSize) {
-        self.init()
-        self._concreteValue = NSSpecialValue(size)
+        self.init { NSSpecialValue(size) as! Self }
     }
     
     public convenience init(rect: NSRect) {
-        self.init()
-        self._concreteValue = NSSpecialValue(rect)
+        self.init { NSSpecialValue(rect) as! Self }
     }
     
     public convenience init(edgeInsets insets: NSEdgeInsets) {
-        self.init()
-        self._concreteValue = NSSpecialValue(insets)
+        self.init { NSSpecialValue(insets) as! Self }
     }
     
     public var pointValue: NSPoint {
-        let specialValue = self._concreteValue as! NSSpecialValue
+        let specialValue = self as! NSSpecialValue
         return specialValue._value as! NSPoint
     }
     
     public var sizeValue: NSSize {
-        let specialValue = self._concreteValue as! NSSpecialValue
+        let specialValue = self as! NSSpecialValue
         return specialValue._value as! NSSize
     }
     
     public var rectValue: NSRect {
-        let specialValue = self._concreteValue as! NSSpecialValue
+        let specialValue = self as! NSSpecialValue
         return specialValue._value as! NSRect
     }
     
     public var edgeInsetsValue: NSEdgeInsets {
-        let specialValue = self._concreteValue as! NSSpecialValue
+        let specialValue = self as! NSSpecialValue
         return specialValue._value as! NSEdgeInsets
     }
 }
diff --git a/Sources/Foundation/NSKeyedArchiver.swift b/Sources/Foundation/NSKeyedArchiver.swift
@@ -405,11 +405,12 @@ open class NSKeyedArchiver : NSCoder {
         Returns true if the object has already been encoded.
      */ 
     private func _haveVisited(_ objv: Any?) -> Bool {
-        if objv == nil {
-            return true // always have a null reference
-        } else {
-            return self._objRefMap[__SwiftValue.store(objv!)] != nil
+        guard let objv = objv else {
+            // always have a null reference
+            return true
         }
+        
+        return self._objRefMap[__SwiftValue.store(objv)] != nil
     }
     
     /**
diff --git a/Sources/Foundation/NSKeyedUnarchiver.swift b/Sources/Foundation/NSKeyedUnarchiver.swift
@@ -201,15 +201,15 @@ open class NSKeyedUnarchiver : NSCoder {
     }
     
     private func _objectInCurrentDecodingContext<T>(forKey key: String?) -> T? {
-        var unwrappedKey = key
+        let unwrappedKey: String
         
-        if key != nil {
-            unwrappedKey = escapeArchiverKey(key!)
+        if let key = key {
+            unwrappedKey = escapeArchiverKey(key)
         } else {
             unwrappedKey = _nextGenericKey()
         }
 
-        if let v = _currentDecodingContext.dict[unwrappedKey!] {
+        if let v = _currentDecodingContext.dict[unwrappedKey] {
             return v as? T
         }
         return nil
@@ -272,18 +272,18 @@ open class NSKeyedUnarchiver : NSCoder {
     }
     
     private func _isClassAllowed(_ assertedClass: AnyClass?, allowedClasses: [AnyClass]?) -> Bool {
-        if assertedClass == nil {
+        guard let assertedClass = assertedClass else {
             return false
         }
         
         if _flags.contains(.requiresSecureCoding) {
             if let unwrappedAllowedClasses = allowedClasses {
-                if unwrappedAllowedClasses.contains(where: {NSKeyedUnarchiver._classIsKindOfClass(assertedClass!, $0)}) {
+                if unwrappedAllowedClasses.contains(where: {NSKeyedUnarchiver._classIsKindOfClass(assertedClass, $0)}) {
                     return true
                 }
             }
             
-            fatalError("Value was of unexpected class \(assertedClass!)")
+            fatalError("Value was of unexpected class \(assertedClass)")
         } else {
             return true
         }
@@ -311,25 +311,25 @@ open class NSKeyedUnarchiver : NSCoder {
             return aClass
         }
         
-        guard let unwrappedClassDict = classDict else {
+        guard let classDict = classDict else {
             return false
         }
         
         // TODO is it required to validate the superclass hierarchy?
-        let assertedClassName = unwrappedClassDict["$classname"] as? String
-        let assertedClassHints = unwrappedClassDict["$classhints"] as? [String]
-        let assertedClasses = unwrappedClassDict["$classes"] as? [String]
+        let assertedClassName = classDict["$classname"] as? String
+        let assertedClassHints = classDict["$classhints"] as? [String]
+        let assertedClasses = classDict["$classes"] as? [String]
         
-        if assertedClassName != nil {
-            let assertedClass : AnyClass? = _classForClassName(assertedClassName!)
+        if let assertedClassName = assertedClassName {
+            let assertedClass : AnyClass? = _classForClassName(assertedClassName)
             if _isClassAllowed(assertedClass, allowedClasses: allowedClasses) {
                 classToConstruct = assertedClass
                 return true
             }
         }
         
-        if assertedClassHints != nil {
-            for assertedClassHint in assertedClassHints! {
+        if let assertedClassHints = assertedClassHints {
+            for assertedClassHint in assertedClassHints {
                 // FIXME check whether class hints should be subject to mapping or not
                 let assertedClass : AnyClass? = NSClassFromString(assertedClassHint)
                 if _isClassAllowed(assertedClass, allowedClasses: allowedClasses) {
@@ -339,11 +339,11 @@ open class NSKeyedUnarchiver : NSCoder {
             }
         }
         
-        if assertedClassName != nil {
+        if let assertedClassName = assertedClassName {
             if let unwrappedDelegate = self.delegate {
                 classToConstruct = unwrappedDelegate.unarchiver(self,
-                                                                cannotDecodeObjectOfClassName: assertedClassName!,
-                                                                originalClasses: assertedClasses != nil ? assertedClasses! : [])
+                                                                cannotDecodeObjectOfClassName: assertedClassName,
+                                                                originalClasses: assertedClasses ?? [])
                 if classToConstruct != nil {
                     return true
                 }
@@ -417,28 +417,26 @@ open class NSKeyedUnarchiver : NSCoder {
     }
     
     private func _replacementObject(_ decodedObject: Any?) -> Any? {
-        var object : Any? = nil // object to encode after substitution
-        
-        // nil cannot be mapped
-        if decodedObject == nil {
+        // nil cannot be mapped (this appears to differ from Darwin?)
+        guard let decodedObject = decodedObject else {
             return nil
         }
         
         // check replacement cache
-        object = self._replacementMap[__SwiftValue.store(decodedObject!)]
-        if object != nil {
+        if let object = self._replacementMap[__SwiftValue.store(decodedObject)] {
             return object
         }
-        
-        // object replaced by delegate. If the delegate returns nil, nil is encoded
+
+        // object replaced by delegate. If using ARC, the delegate should only return
+        // nil if the object itself is nil.
         if let unwrappedDelegate = self.delegate {
-            object = unwrappedDelegate.unarchiver(self, didDecode: decodedObject!)
-            if object != nil {
-                replaceObject(decodedObject!, withObject: object!)
-                return object
-            }
+             let object = unwrappedDelegate.unarchiver(self, didDecode: decodedObject)
+             if object != nil {
+                 replaceObject(decodedObject, withObject: object!)
+                 return object
+             }
         }
-        
+
         return decodedObject
     }
     
@@ -470,12 +468,12 @@ open class NSKeyedUnarchiver : NSCoder {
             throw InternalError.decodingHasAlreadyFailed
         }
 
-        if !(objectRef is _NSKeyedArchiverUID) {
+        guard let objectRef = objectRef as? _NSKeyedArchiverUID else {
             throw _decodingError(.coderReadCorrupt,
                                  withDescription: "Object \(objectRef) is not a reference. The data may be corrupt.")
         }
-
-        guard let dereferencedObject = _dereferenceObjectReference(objectRef as! _NSKeyedArchiverUID) else {
+        
+        guard let dereferencedObject = _dereferenceObjectReference(objectRef) else {
             throw _decodingError(.coderReadCorrupt,
                                  withDescription: "Invalid object reference \(objectRef). The data may be corrupt.")
         }
@@ -486,7 +484,7 @@ open class NSKeyedUnarchiver : NSCoder {
 
         if _isContainer(dereferencedObject) {
             // check cached of decoded objects
-            object = _cachedObjectForReference(objectRef as! _NSKeyedArchiverUID)
+            object = _cachedObjectForReference(objectRef)
             if object == nil {
                 guard let dict = dereferencedObject as? Dictionary<String, Any> else {
                     throw _decodingError(.coderReadCorrupt,
@@ -523,7 +521,7 @@ open class NSKeyedUnarchiver : NSCoder {
                                          withDescription: "Class \(classToConstruct!) failed to decode. The data may be corrupt.")
                 }
 
-                _cacheObject(object!, forReference: objectRef as! _NSKeyedArchiverUID)
+                _cacheObject(object!, forReference: objectRef)
             }
         } else {
             object = __SwiftValue.store(dereferencedObject)
diff --git a/Sources/Foundation/NSRange.swift b/Sources/Foundation/NSRange.swift
@@ -403,12 +403,11 @@ extension NSRange: NSSpecialValueCoding {
     
 extension NSValue {
     public convenience init(range: NSRange) {
-        self.init()
-        self._concreteValue = NSSpecialValue(range)
+        self.init { NSSpecialValue(range) as! Self }
     }
     
     public var rangeValue: NSRange {
-        let specialValue = self._concreteValue as! NSSpecialValue
+        let specialValue = self as! NSSpecialValue
         return specialValue._value as! NSRange
     }
 }
diff --git a/Sources/Foundation/NSSpecialValue.swift b/Sources/Foundation/NSSpecialValue.swift
@@ -46,33 +46,17 @@ internal class NSSpecialValue : NSValue {
     }
     
     private static func _flagsFromType(_ type: NSSpecialValueCoding.Type) -> Int {
-        for (F, T) in _specialTypes {
-            if T == type {
-                return F
-            }
-        }
-        return 0
+        return _specialTypes.first(where: { $1 == type })?.0 ?? 0
     }
     
     private static func _objCTypeFromType(_ type: NSSpecialValueCoding.Type) -> String? {
-        for (_, T) in _specialTypes {
-            if T == type {
-                return T.objCType()
-            }
-        }
-        return nil
+        return _specialTypes.first(where: { $1 == type })?.1.objCType()
     }
     
     internal static func _typeFromObjCType(_ type: UnsafePointer<Int8>) -> NSSpecialValueCoding.Type? {
         let objCType = String(cString: type)
         
-        for (_, T) in _specialTypes {
-            if T.objCType() == objCType {
-                return T
-            }
-        }
-        
-        return nil
+        return _specialTypes.first(where: { $1.objCType() == objCType })?.1
     }
     
     internal var _value : NSSpecialValueCoding
@@ -98,10 +82,8 @@ internal class NSSpecialValue : NSValue {
             preconditionFailure("Unkeyed coding is unsupported.")
         }
         let specialFlags = aDecoder.decodeInteger(forKey: "NS.special")
-        guard let specialType = NSSpecialValue._typeFromFlags(specialFlags) else {
-            return nil
-        }
-        guard let specialValue = specialType.init(coder: aDecoder) else {
+        guard let specialType = NSSpecialValue._typeFromFlags(specialFlags),
+              let specialValue = specialType.init(coder: aDecoder) else {
             return nil
         }
         self.init(specialValue)
@@ -121,7 +103,6 @@ internal class NSSpecialValue : NSValue {
     }
     
     override var classForCoder: AnyClass {
-        // for some day when we support class clusters
         return NSValue.self
     }
     
diff --git a/Sources/Foundation/NSValue.swift b/Sources/Foundation/NSValue.swift
diff --git a/Tests/Foundation/Tests/TestNSKeyedArchiver.swift b/Tests/Foundation/Tests/TestNSKeyedArchiver.swift
diff --git a/Tests/Foundation/Tests/TestNSKeyedUnarchiver.swift b/Tests/Foundation/Tests/TestNSKeyedUnarchiver.swift

Original file line number	Diff line number	Diff line change
`@@ -1085,42 +1085,38 @@ public func NSRectFromString(_ aString: String) -> NSRect {`
`1085`	`1085`
`1086`	`1086`	`extension NSValue {`
`1087`	`1087`	`public convenience init(point: NSPoint) {`
`1088`		`- self.init()`
`1089`		`- self._concreteValue = NSSpecialValue(point)`
	`1088`	`+ self.init { NSSpecialValue(point) as! Self }`
`1090`	`1089`	`}`
`1091`	`1090`
`1092`	`1091`	`public convenience init(size: NSSize) {`
`1093`		`- self.init()`
`1094`		`- self._concreteValue = NSSpecialValue(size)`
	`1092`	`+ self.init { NSSpecialValue(size) as! Self }`
`1095`	`1093`	`}`
`1096`	`1094`
`1097`	`1095`	`public convenience init(rect: NSRect) {`
`1098`		`- self.init()`
`1099`		`- self._concreteValue = NSSpecialValue(rect)`
	`1096`	`+ self.init { NSSpecialValue(rect) as! Self }`
`1100`	`1097`	`}`
`1101`	`1098`
`1102`	`1099`	`public convenience init(edgeInsets insets: NSEdgeInsets) {`
`1103`		`- self.init()`
`1104`		`- self._concreteValue = NSSpecialValue(insets)`
	`1100`	`+ self.init { NSSpecialValue(insets) as! Self }`
`1105`	`1101`	`}`
`1106`	`1102`
`1107`	`1103`	`public var pointValue: NSPoint {`
`1108`		`- let specialValue = self._concreteValue as! NSSpecialValue`
	`1104`	`+ let specialValue = self as! NSSpecialValue`
`1109`	`1105`	`return specialValue._value as! NSPoint`
`1110`	`1106`	`}`
`1111`	`1107`
`1112`	`1108`	`public var sizeValue: NSSize {`
`1113`		`- let specialValue = self._concreteValue as! NSSpecialValue`
	`1109`	`+ let specialValue = self as! NSSpecialValue`
`1114`	`1110`	`return specialValue._value as! NSSize`
`1115`	`1111`	`}`
`1116`	`1112`
`1117`	`1113`	`public var rectValue: NSRect {`
`1118`		`- let specialValue = self._concreteValue as! NSSpecialValue`
	`1114`	`+ let specialValue = self as! NSSpecialValue`
`1119`	`1115`	`return specialValue._value as! NSRect`
`1120`	`1116`	`}`
`1121`	`1117`
`1122`	`1118`	`public var edgeInsetsValue: NSEdgeInsets {`
`1123`		`- let specialValue = self._concreteValue as! NSSpecialValue`
	`1119`	`+ let specialValue = self as! NSSpecialValue`
`1124`	`1120`	`return specialValue._value as! NSEdgeInsets`
`1125`	`1121`	`}`
`1126`	`1122`	`}`
Original file line number	Diff line number	Diff line change
`@@ -403,12 +403,11 @@ extension NSRange: NSSpecialValueCoding {`
`403`	`403`
`404`	`404`	`extension NSValue {`
`405`	`405`	`public convenience init(range: NSRange) {`
`406`		`- self.init()`
`407`		`- self._concreteValue = NSSpecialValue(range)`
	`406`	`+ self.init { NSSpecialValue(range) as! Self }`
`408`	`407`	`}`
`409`	`408`
`410`	`409`	`public var rangeValue: NSRange {`
`411`		`- let specialValue = self._concreteValue as! NSSpecialValue`
	`410`	`+ let specialValue = self as! NSSpecialValue`
`412`	`411`	`return specialValue._value as! NSRange`
`413`	`412`	`}`
`414`	`413`	`}`