Fix decoding empty element as optional (CoreOffice#92)

Fix an issue uncovered by CoreOffice/CoreXLSX#59. Also include slightly unrelated cleanups. Resolve CoreOffice#94 * Fix empty element decoded as optional * Add failing test case to SpacePreserveTest * Disable `nesting` SwiftLint rule * Fix handling of nested empty string values * Add trimValueWhitespaces property on XMLDecoder
salido · Jun 26, 2020 · e0d2bdd · e0d2bdd
1 parent 185e9b6
commit e0d2bdd
Show file tree

Hide file tree

Showing 26 changed files with 151 additions and 87 deletions.
diff --git a/.swiftlint.yml b/.swiftlint.yml
@@ -5,6 +5,7 @@ disabled_rules:
 - operator_whitespace
 - function_parameter_count
 - opening_brace
+- nesting
 
 line_length:
 - 150

diff --git a/Sources/XMLCoder/Auxiliaries/Box/BoolBox.swift b/Sources/XMLCoder/Auxiliaries/Box/BoolBox.swift
@@ -21,10 +21,6 @@ struct BoolBox: Equatable {
         case _: return nil
         }
     }
-
-    func unbox() -> Unboxed {
-        return unboxed
-    }
 }
 
 extension BoolBox: Box {

diff --git a/Sources/XMLCoder/Auxiliaries/Box/DataBox.swift b/Sources/XMLCoder/Auxiliaries/Box/DataBox.swift
@@ -29,10 +29,6 @@ struct DataBox: Equatable {
         self.init(data, format: .base64)
     }
 
-    func unbox() -> Unboxed {
-        return unboxed
-    }
-
     func xmlString(format: Format) -> String {
         switch format {
         case .base64:

diff --git a/Sources/XMLCoder/Auxiliaries/Box/DateBox.swift b/Sources/XMLCoder/Auxiliaries/Box/DateBox.swift
@@ -59,10 +59,6 @@ struct DateBox: Equatable {
         self.init(date, format: .formatter(formatter))
     }
 
-    func unbox() -> Unboxed {
-        return unboxed
-    }
-
     func xmlString(format: Format) -> String {
         switch format {
         case .secondsSince1970:

diff --git a/Sources/XMLCoder/Auxiliaries/Box/DecimalBox.swift b/Sources/XMLCoder/Auxiliaries/Box/DecimalBox.swift
@@ -22,10 +22,6 @@ struct DecimalBox: Equatable {
         }
         self.init(unboxed)
     }
-
-    func unbox() -> Unboxed {
-        return unboxed
-    }
 }
 
 extension DecimalBox: Box {

diff --git a/Sources/XMLCoder/Auxiliaries/Box/KeyedBox.swift b/Sources/XMLCoder/Auxiliaries/Box/KeyedBox.swift
@@ -16,7 +16,7 @@ struct KeyedBox {
     var elements = Elements()
     var attributes = Attributes()
 
-    func unbox() -> (elements: Elements, attributes: Attributes) {
+    var unboxed: (elements: Elements, attributes: Attributes) {
         return (
             elements: elements,
             attributes: attributes

diff --git a/Sources/XMLCoder/Auxiliaries/Box/SharedBox.swift b/Sources/XMLCoder/Auxiliaries/Box/SharedBox.swift
@@ -6,7 +6,7 @@
 //
 
 class SharedBox<Unboxed: Box> {
-    fileprivate var unboxed: Unboxed
+    private(set) var unboxed: Unboxed
 
     init(_ wrapped: Unboxed) {
         unboxed = wrapped

diff --git a/Sources/XMLCoder/Auxiliaries/Box/StringBox.swift b/Sources/XMLCoder/Auxiliaries/Box/StringBox.swift
@@ -17,10 +17,6 @@ struct StringBox: Equatable {
     init(xmlString: Unboxed) {
         self.init(xmlString)
     }
-
-    func unbox() -> Unboxed {
-        return unboxed
-    }
 }
 
 extension StringBox: Box {

diff --git a/Sources/XMLCoder/Auxiliaries/Box/URLBox.swift b/Sources/XMLCoder/Auxiliaries/Box/URLBox.swift
@@ -22,10 +22,6 @@ struct URLBox: Equatable {
         }
         self.init(unboxed)
     }
-
-    func unbox() -> Unboxed {
-        return unboxed
-    }
 }
 
 extension URLBox: Box {

diff --git a/Sources/XMLCoder/Auxiliaries/Box/UnkeyedBox.swift b/Sources/XMLCoder/Auxiliaries/Box/UnkeyedBox.swift
@@ -29,10 +29,6 @@ struct UnkeyedBox {
         self.unboxed = unboxed
     }
 
-    func unbox() -> Unboxed {
-        return unboxed
-    }
-
     mutating func append(_ newElement: Element) {
         unboxed.append(newElement)
     }

diff --git a/Sources/XMLCoder/Auxiliaries/XMLCoderElement.swift b/Sources/XMLCoder/Auxiliaries/XMLCoderElement.swift
@@ -35,9 +35,11 @@ struct XMLCoderElement: Equatable {
     }
 
     mutating func append(value string: String) {
-        var value = self.value ?? ""
-        value += string.trimmingCharacters(in: .whitespacesAndNewlines)
-        self.value = value
+        guard value != nil else {
+            value = string
+            return
+        }
+        value?.append(string)
     }
 
     mutating func append(element: XMLCoderElement, forKey key: String) {
@@ -255,7 +257,7 @@ extension XMLCoderElement {
 
             switch box {
             case let sharedUnkeyedBox as SharedBox<UnkeyedBox>:
-                let box = sharedUnkeyedBox.unbox()
+                let box = sharedUnkeyedBox.unboxed
                 elements.append(contentsOf: box.map {
                     XMLCoderElement(key: key, box: $0)
                 })
@@ -265,7 +267,7 @@ extension XMLCoderElement {
                     XMLCoderElement(key: key, box: $0)
                 })
             case let sharedKeyedBox as SharedBox<KeyedBox>:
-                let box = sharedKeyedBox.unbox()
+                let box = sharedKeyedBox.unboxed
                 elements.append(XMLCoderElement(key: key, box: box))
             case let keyedBox as KeyedBox:
                 elements.append(XMLCoderElement(key: key, box: keyedBox))
@@ -296,9 +298,9 @@ extension XMLCoderElement {
     init(key: String, box: Box) {
         switch box {
         case let sharedUnkeyedBox as SharedBox<UnkeyedBox>:
-            self.init(key: key, box: sharedUnkeyedBox.unbox())
+            self.init(key: key, box: sharedUnkeyedBox.unboxed)
         case let sharedKeyedBox as SharedBox<KeyedBox>:
-            self.init(key: key, box: sharedKeyedBox.unbox())
+            self.init(key: key, box: sharedKeyedBox.unboxed)
         case let unkeyedBox as UnkeyedBox:
             self.init(key: key, box: unkeyedBox)
         case let keyedBox as KeyedBox:

diff --git a/Sources/XMLCoder/Auxiliaries/XMLStackParser.swift b/Sources/XMLCoder/Auxiliaries/XMLStackParser.swift
@@ -11,13 +11,20 @@ import Foundation
 class XMLStackParser: NSObject {
     var root: XMLCoderElement?
     private var stack: [XMLCoderElement] = []
+    private let trimValueWhitespaces: Bool
+
+    init(trimValueWhitespaces: Bool = true) {
+        self.trimValueWhitespaces = trimValueWhitespaces
+        super.init()
+    }
 
     static func parse(
         with data: Data,
         errorContextLength length: UInt,
-        shouldProcessNamespaces: Bool
+        shouldProcessNamespaces: Bool,
+        trimValueWhitespaces: Bool
     ) throws -> KeyedBox {
-        let parser = XMLStackParser()
+        let parser = XMLStackParser(trimValueWhitespaces: trimValueWhitespaces)
 
         let node = try parser.parse(
             with: data,
@@ -99,6 +106,13 @@ class XMLStackParser: NSObject {
         }
         try body(&stack[stack.count - 1])
     }
+
+    /// Trim whitespaces for a given string if needed.
+    func process(string: String) -> String {
+        return trimValueWhitespaces
+            ? string.trimmingCharacters(in: .whitespacesAndNewlines)
+            : string
+    }
 }
 
 extension XMLStackParser: XMLParserDelegate {
@@ -120,14 +134,10 @@ extension XMLStackParser: XMLParserDelegate {
                 didEndElement _: String,
                 namespaceURI _: String?,
                 qualifiedName _: String?) {
-        guard var element = stack.popLast() else {
+        guard let element = stack.popLast() else {
             return
         }
 
-        if let value = element.value {
-            element.value = value.isEmpty ? nil : value
-        }
-
         withCurrentElement { currentElement in
             currentElement.append(element: element, forKey: element.key)
         }
@@ -139,16 +149,17 @@ extension XMLStackParser: XMLParserDelegate {
 
     func parser(_: XMLParser, foundCharacters string: String) {
         withCurrentElement { currentElement in
-            currentElement.append(value: string)
+            currentElement.append(value: process(string: string))
         }
     }
 
     func parser(_: XMLParser, foundCDATA CDATABlock: Data) {
         guard let string = String(data: CDATABlock, encoding: .utf8) else {
             return
         }
+
         withCurrentElement { currentElement in
-            currentElement.append(value: string)
+            currentElement.append(value: process(string: string))
         }
     }
 }
diff --git a/Sources/XMLCoder/Decoder/XMLDecoder.swift b/Sources/XMLCoder/Decoder/XMLDecoder.swift
@@ -284,6 +284,11 @@ open class XMLDecoder {
      */
     open var shouldProcessNamespaces: Bool = false
 
+    /** A boolean value that determines whether the parser trims whitespaces
+     and newlines from the end and the beginning of string values.
+     */
+    open var trimValueWhitespaces: Bool
+
     /// Options set on the top-level encoder to pass down the decoding hierarchy.
     struct Options {
         let dateDecodingStrategy: DateDecodingStrategy
@@ -309,7 +314,9 @@ open class XMLDecoder {
     // MARK: - Constructing a XML Decoder
 
     /// Initializes `self` with default strategies.
-    public init() {}
+    public init(trimValueWhitespaces: Bool = true) {
+        self.trimValueWhitespaces = trimValueWhitespaces
+    }
 
     // MARK: - Decoding Values
 
@@ -327,7 +334,8 @@ open class XMLDecoder {
         let topLevel: Box = try XMLStackParser.parse(
             with: data,
             errorContextLength: errorContextLength,
-            shouldProcessNamespaces: shouldProcessNamespaces
+            shouldProcessNamespaces: shouldProcessNamespaces,
+            trimValueWhitespaces: trimValueWhitespaces
         )
 
         let decoder = XMLDecoderImplementation(
@@ -346,13 +354,6 @@ open class XMLDecoder {
             _ = decoder.nodeDecodings.removeLast()
         }
 
-        guard let box: T = try decoder.unbox(topLevel) else {
-            throw DecodingError.valueNotFound(type, DecodingError.Context(
-                codingPath: [],
-                debugDescription: "The given data did not contain a top-level box."
-            ))
-        }
-
-        return box
+        return try decoder.unbox(topLevel)
     }
 }