Fix comments

Author: Pouya Yarandi

Sources/SwiftProtobuf/GetPathDecoder.swift

@@ -14,21 +14,21 @@
 
 import Foundation
 
+// Decoder that captures value of a message field from the given path
 struct GetPathDecoder<T: Message>: Decoder {
 
+  // The path only including sub-paths
   private let nextPath: [String]
-  private var number: Int?
 
-  private var _value: Any?
-  private var _hasPath: Bool = false
+  // Field number should be captured by decoder
+  private var number: Int?
 
-  internal var value: Any? {
-    _value
-  }
+  // Captured value after decoding will be stored in this property
+  private(set) var value: Any?
 
-  internal var hasPath: Bool {
-    _hasPath
-  }
+  // While decoding this property becomes true if the path exists
+  // Note that a property value could be nil while its path is found
+  private(set) var hasPath: Bool = false
 
   internal init(path: [String]) throws {
     guard let firstComponent = path.first,
@@ -50,8 +50,8 @@ struct GetPathDecoder<T: Message>: Decoder {
     guard nextPath.isEmpty else {
       throw PathDecodingError.pathNotFound
     }
-    self._value = value
-    self._hasPath = true
+    self.value = value
+    self.hasPath = true
   }
 
   mutating func decodeSingularFloatField(value: inout Float) throws {
@@ -260,8 +260,8 @@ struct GetPathDecoder<T: Message>: Decoder {
       var tmp = M()
       try tmp.decodeMessage(decoder: &decoder)
     }
-    self._value = decoder.value
-    self._hasPath = decoder.hasPath
+    self.value = decoder.value
+    self.hasPath = decoder.hasPath
   }
 
   mutating func decodeRepeatedMessageField<M>(value: inout [M]) throws {
@@ -301,7 +301,11 @@ struct GetPathDecoder<T: Message>: Decoder {
     values: inout ExtensionFieldValueSet,
     messageType: Message.Type,
     fieldNumber: Int
-  ) throws {}
+  ) throws {
+    preconditionFailure(
+      "Path decoder should never decode an extension field"
+    )
+  }
 
 }
 

Sources/SwiftProtobuf/Google_Protobuf_FieldMask+Extensions.swift

@@ -283,8 +283,7 @@ extension Google_Protobuf_FieldMask {
     let set = mask.pathsSet
     var _paths: [String] = []
     var _buffer = Set<String>()
-    for path in paths where set.contains(path)
-    && !_buffer.contains(path) {
+    for path in paths where set.contains(path) && !_buffer.contains(path) {
       _buffer.insert(path)
       _paths.append(path)
     }
@@ -304,8 +303,7 @@ extension Google_Protobuf_FieldMask {
     let set = mask.pathsSet
     var _paths: [String] = []
     var _buffer = Set<String>()
-    for path in paths where !set.contains(path)
-    && !_buffer.contains(path) {
+    for path in paths where !set.contains(path) && !_buffer.contains(path) {
       _buffer.insert(path)
       _paths.append(path)
     }
@@ -334,13 +332,6 @@ extension Google_Protobuf_FieldMask {
   private var pathsSet: Set<String> {
     .init(paths)
   }
-
-  private func levels(path: String) -> [String] {
-    let comps = path.components(separatedBy: ".")
-    return (0..<comps.count).map {
-      comps[0...$0].joined(separator: ".")
-    }
-  }
 }
 
 extension Google_Protobuf_FieldMask {

Sources/SwiftProtobuf/Message+FieldMask.swift

@@ -35,13 +35,17 @@ extension Message {
 }
 
 /// Defines available options for merging two messages.
-public enum MergeOption: Equatable {
+public struct MergeOption {
+
+  public init(replaceRepeatedFields: Bool = false) {
+    self.replaceRepeatedFields = replaceRepeatedFields
+  }
 
   /// The default merging behavior will append entries from the source
   /// repeated field to the destination repeated field. If you only want
   /// to keep the entries from the source repeated field, set this flag
   /// to true.
-  case replaceRepeatedFields
+  public var replaceRepeatedFields = false
 }
 
 extension Message {
@@ -54,29 +58,27 @@ extension Message {
   public mutating func merge(
     to source: Self,
     fieldMask: Google_Protobuf_FieldMask,
-    mergeOptions: [MergeOption] = []
+    mergeOption: MergeOption = .init()
   ) throws {
     var source = source
-    var copy = self
     var pathToValueMap: [String: Any?] = [:]
-    let replaceRepeatedFields = mergeOptions
-      .contains(.replaceRepeatedFields)
     for path in fieldMask.paths {
       pathToValueMap[path] = try source.get(path: path)
     }
     for (path, value) in pathToValueMap {
-      try copy.set(
+      try? set(
         path: path,
         value: value,
-        replaceRepeatedFields: replaceRepeatedFields
+        mergeOption: mergeOption
       )
     }
-    self = copy
   }
 }
 
 extension Message where Self: Equatable, Self: _ProtoNameProviding {
 
+  // TODO: Re-implement using clear fields instead of copying message
+
   @discardableResult
   /// Removes from 'message' any field that is not represented in the given
   /// FieldMask. If the FieldMask is empty, does nothing.
@@ -92,7 +94,7 @@ extension Message where Self: Equatable, Self: _ProtoNameProviding {
     if fieldMask.paths.isEmpty {
       return false
     }
-    var tmp = Self.init()
+    var tmp = Self(removingAllFieldsOf: self)
     do {
       try tmp.merge(to: self, fieldMask: fieldMask)
       let changed = tmp != self
@@ -103,3 +105,22 @@ extension Message where Self: Equatable, Self: _ProtoNameProviding {
     }
   }
 }
+
+private extension Message {
+  init(removingAllFieldsOf message: Self) {
+    if let type = Self.self as? ExtensibleMessage.Type,
+       let extensible = message as? ExtensibleMessage {
+      self = type.init(extensionsOf: extensible) as? Self ?? .init()
+    } else {
+      self = .init()
+    }
+    self.unknownFields = message.unknownFields
+  }
+}
+
+private extension Message where Self: ExtensibleMessage {
+  init(extensionsOf message: ExtensibleMessage) {
+    self.init()
+    _protobuf_extensionFieldValues = message._protobuf_extensionFieldValues
+  }
+}

Sources/SwiftProtobuf/SetPathDecoder.swift

@@ -39,24 +39,37 @@ extension Message {
   }
 }
 
+// Decoder that set value of a message field by the given path
 struct SetPathDecoder<T: Message>: Decoder {
 
+  // The value should be set to the path
   private let value: Any?
+
+  // Field number should be overriden by decoder
   private var number: Int?
+
+  // The path only including sub-paths
   private let nextPath: [String]
-  private let replaceRepeatedFields: Bool
+
+  // Merge options to be concidered while setting value
+  private let mergeOption: MergeOption
+
+  private var replaceRepeatedFields: Bool {
+    mergeOption.replaceRepeatedFields
+  }
 
   init(
     path: [String],
     value: Any?,
-    replaceRepeatedFields: Bool
+    mergeOption: MergeOption
   ) {
-    if let firstComponent = path.first {
-      self.number = T.number(for: firstComponent)
+    if let firstComponent = path.first,
+       let number = T.number(for: firstComponent) {
+      self.number = number
     }
     self.nextPath = .init(path.dropFirst())
     self.value = value
-    self.replaceRepeatedFields = replaceRepeatedFields
+    self.mergeOption = mergeOption
   }
 
   private func setValue<V>(_ value: inout V) throws {
@@ -77,6 +90,21 @@ struct SetPathDecoder<T: Message>: Decoder {
     }
   }
 
+  private func setMapValue<K, V>(
+    _ value: inout Dictionary<K, V>
+  ) throws {
+    guard let __value = self.value as? Dictionary<K, V> else {
+      throw PathDecodingError.typeMismatch
+    }
+    if replaceRepeatedFields {
+      value = __value
+    } else {
+      value.merge(__value) { _, new in
+        new
+      }
+    }
+  }
+
   mutating func handleConflictingOneOf() throws {}
 
   mutating func nextFieldNumber() throws -> Int? {
@@ -292,7 +320,7 @@ struct SetPathDecoder<T: Message>: Decoder {
     var decoder = SetPathDecoder<M>(
         path: nextPath, 
         value: self.value,
-        replaceRepeatedFields: replaceRepeatedFields
+        mergeOption: mergeOption
     )
     if value == nil {
       value = .init()
@@ -322,42 +350,46 @@ struct SetPathDecoder<T: Message>: Decoder {
     fieldType: _ProtobufMap<KeyType, ValueType>.Type,
     value: inout _ProtobufMap<KeyType, ValueType>.BaseType
   ) throws where KeyType : MapKeyType, ValueType : MapValueType {
-    try setValue(&value)
+    try setMapValue(&value)
   }
 
   mutating func decodeMapField<KeyType, ValueType>(
     fieldType: _ProtobufEnumMap<KeyType, ValueType>.Type,
     value: inout _ProtobufEnumMap<KeyType, ValueType>.BaseType
   ) throws where KeyType : MapKeyType, ValueType : Enum, ValueType.RawValue == Int {
-    try setValue(&value)
+    try setMapValue(&value)
   }
 
   mutating func decodeMapField<KeyType, ValueType>(
     fieldType: _ProtobufMessageMap<KeyType, ValueType>.Type,
     value: inout _ProtobufMessageMap<KeyType, ValueType>.BaseType
   ) throws where KeyType : MapKeyType, ValueType : Hashable, ValueType : Message {
-    try setValue(&value)
+    try setMapValue(&value)
   }
 
   mutating func decodeExtensionField(
     values: inout ExtensionFieldValueSet,
     messageType: Message.Type,
     fieldNumber: Int
-  ) throws {}
+  ) throws {
+    preconditionFailure(
+      "Path decoder should never decode an extension field"
+    )
+  }
 
 }
 
 extension Message {
   mutating func `set`(
     path: String,
     value: Any?,
-    replaceRepeatedFields: Bool
+    mergeOption: MergeOption
   ) throws {
     let _path = path.components(separatedBy: ".")
     var decoder = SetPathDecoder<Self>(
       path: _path,
       value: value,
-      replaceRepeatedFields: replaceRepeatedFields
+      mergeOption: mergeOption
     )
     try decodeMessage(decoder: &decoder)
   }

Tests/SwiftProtobufTests/Test_FieldMask.swift

@@ -151,10 +151,31 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
         XCTAssertEqual(message.repeatedInt32, [1, 2, 3, 4])
 
         // Checks with replacing repeated fields
-        try message.merge(to: secondMessage, fieldMask: fieldMask, mergeOptions: [.replaceRepeatedFields])
+        try message.merge(to: secondMessage, fieldMask: fieldMask, mergeOption: .init(replaceRepeatedFields: true))
         XCTAssertEqual(message.repeatedInt32, [3, 4])
     }
 
+    // Checks merge functionality for map field masks.
+    func testMergeMapFieldsOfMessage() throws {
+        var message = SwiftProtoTesting_Fuzz_Message.with { model in
+            model.mapInt32String = [1: "a", 2: "c"]
+        }
+
+        let secondMessage = SwiftProtoTesting_Fuzz_Message.with { model in
+            model.mapInt32String = [2: "b"]
+        }
+
+        let fieldMask = Google_Protobuf_FieldMask(protoPaths: ["map_int32_string"])
+
+        // Checks without replacing repeated fields
+        try message.merge(to: secondMessage, fieldMask: fieldMask)
+        XCTAssertEqual(message.mapInt32String, [1: "a", 2: "b"])
+
+        // Checks with replacing repeated fields
+        try message.merge(to: secondMessage, fieldMask: fieldMask, mergeOption: .init(replaceRepeatedFields: true))
+        XCTAssertEqual(message.mapInt32String, [2: "b"])
+    }
+
     // Checks trim functionality for field masks.
     func testTrimFieldsOfMessage() throws {
         var message = SwiftProtoTesting_TestAllTypes.with { model in