Stop removing underscores from CodingKey names in InputKey (#548)

When a property wrapper is applied to a property, the property's
storage is given a name with a prefixed underscore. That is,
for a property named `x`, the actual storage is named `_x`.

That prefixed storage is what is visible through reflection, so
when building an ArgumentSet from a command type's Mirror, we
need to remove the leading underscore. This is done when creating
an InputKey for each property.

However, InputKeys are also created from CodingKeys during
decoding of a ParsableCommand. These CodingKeys _do not_ have
the leading underscore that is visible, so any underscores
that appear are actually from the declaration of the property
with an underscored name. Removing leading underscores from
CodingKey names results in a mismatch when trying to find
the decoded value.

This change simplifies the InputKey type to use an array
path instead of an indirect enum and removes the leading
underscore dropping when creating an InputKey from a CodingKey.

rdar://104928743

Author: natecook1000

Sources/ArgumentParser/Completions/BashCompletionsGenerator.swift

@@ -132,7 +132,7 @@ struct BashCompletionsGenerator {
   ///
   /// These consist of completions that are defined as `.list` or `.custom`.
   fileprivate static func generateArgumentCompletions(_ commands: [ParsableCommand.Type]) -> [String] {
-    ArgumentSet(commands.last!, visibility: .default, parent: .root)
+    ArgumentSet(commands.last!, visibility: .default, parent: nil)
       .compactMap { arg -> String? in
         guard arg.isPositional else { return nil }
 
@@ -159,7 +159,7 @@ struct BashCompletionsGenerator {
 
   /// Returns the case-matching statements for supplying completions after an option or flag.
   fileprivate static func generateOptionHandlers(_ commands: [ParsableCommand.Type]) -> String {
-    ArgumentSet(commands.last!, visibility: .default, parent: .root)
+    ArgumentSet(commands.last!, visibility: .default, parent: nil)
       .compactMap { arg -> String? in
         let words = arg.bashCompletionWords()
         if words.isEmpty { return nil }

Sources/ArgumentParser/Parsable Properties/Flag.swift

@@ -396,7 +396,7 @@ extension Flag where Value: EnumerableFlag {
       // flag, the default value to show to the user is the `--value-name`
       // flag that a user would provide on the command line, not a Swift value.
       let defaultValueFlag = initial.flatMap { value -> String? in
-        let defaultKey = InputKey(name: String(describing: value), parent: .key(key))
+        let defaultKey = InputKey(name: String(describing: value), parent: key)
         let defaultNames = Value.name(for: value).makeNames(defaultKey)
         return defaultNames.first?.synopsisString
       }
@@ -405,7 +405,7 @@ extension Flag where Value: EnumerableFlag {
       let hasCustomCaseHelp = caseHelps.contains(where: { $0 != nil })
 
       let args = Value.allCases.enumerated().map { (i, value) -> ArgumentDefinition in
-        let caseKey = InputKey(name: String(describing: value), parent: .key(key))
+        let caseKey = InputKey(name: String(describing: value), parent: key)
         let name = Value.name(for: value)
 
         let helpForCase = caseHelps[i] ?? help
@@ -519,7 +519,7 @@ extension Flag {
       let hasCustomCaseHelp = caseHelps.contains(where: { $0 != nil })
 
       let args = Element.allCases.enumerated().map { (i, value) -> ArgumentDefinition in
-        let caseKey = InputKey(name: String(describing: value), parent: .key(parentKey))
+        let caseKey = InputKey(name: String(describing: value), parent: parentKey)
         let name = Element.name(for: value)
         let helpForCase = hasCustomCaseHelp ? (caseHelps[i] ?? help) : help
 
@@ -552,7 +552,7 @@ extension Flag {
       let hasCustomCaseHelp = caseHelps.contains(where: { $0 != nil })
 
       let args = Element.allCases.enumerated().map { (i, value) -> ArgumentDefinition in
-        let caseKey = InputKey(name: String(describing: value), parent: .key(parentKey))
+        let caseKey = InputKey(name: String(describing: value), parent: parentKey)
         let name = Element.name(for: value)
         let helpForCase = hasCustomCaseHelp ? (caseHelps[i] ?? help) : help
         let help = ArgumentDefinition.Help(

Sources/ArgumentParser/Parsable Properties/NameSpecification.swift

@@ -167,7 +167,7 @@ extension FlagInversion {
         case .short, .customShort:
           return includingShort ? element.name(for: key) : nil
         case .long:
-          let modifiedKey = key.with(newName: key.name.addingIntercappedPrefix(prefix))
+          let modifiedKey = InputKey(name: key.name.addingIntercappedPrefix(prefix), parent: key)
           return element.name(for: modifiedKey)
         case .customLong(let name, let withSingleDash):
           let modifiedName = name.addingPrefixWithAutodetectedStyle(prefix)

Sources/ArgumentParser/Parsable Properties/OptionGroup.swift

@@ -78,7 +78,7 @@ public struct OptionGroup<Value: ParsableArguments>: Decodable, ParsedWrapper {
     visibility: ArgumentVisibility = .default
   ) {
     self.init(_parsedValue: .init { parentKey in
-      var args = ArgumentSet(Value.self, visibility: .private, parent: .key(parentKey))
+      var args = ArgumentSet(Value.self, visibility: .private, parent: parentKey)
       args.content.withEach {
         $0.help.parentTitle = title
       }

Sources/ArgumentParser/Parsable Types/ParsableArguments.swift

@@ -248,7 +248,7 @@ extension ArgumentSetProvider {
 }
 
 extension ArgumentSet {
-  init(_ type: ParsableArguments.Type, visibility: ArgumentVisibility, parent: InputKey.Parent) {
+  init(_ type: ParsableArguments.Type, visibility: ArgumentVisibility, parent: InputKey?) {
     #if DEBUG
     do {
       try type._validate(parent: parent)

Sources/ArgumentParser/Parsable Types/ParsableArgumentsValidation.swift

@@ -10,7 +10,7 @@
 //===----------------------------------------------------------------------===//
 
 fileprivate protocol ParsableArgumentsValidator {
-  static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError?
+  static func validate(_ type: ParsableArguments.Type, parent: InputKey?) -> ParsableArgumentsValidatorError?
 }
 
 enum ValidatorErrorKind {
@@ -37,7 +37,7 @@ struct ParsableArgumentsValidationError: Error, CustomStringConvertible {
 }
 
 extension ParsableArguments {
-  static func _validate(parent: InputKey.Parent) throws {
+  static func _validate(parent: InputKey?) throws {
     let validators: [ParsableArgumentsValidator.Type] = [
       PositionalArgumentsValidator.self,
       ParsableArgumentsCodingKeyValidator.self,
@@ -80,7 +80,7 @@ struct PositionalArgumentsValidator: ParsableArgumentsValidator {
     var kind: ValidatorErrorKind { .failure }
   }
 
-  static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
+  static func validate(_ type: ParsableArguments.Type, parent: InputKey?) -> ParsableArgumentsValidatorError? {
     let sets: [ArgumentSet] = Mirror(reflecting: type.init())
       .children
       .compactMap { child in
@@ -190,7 +190,7 @@ struct ParsableArgumentsCodingKeyValidator: ParsableArgumentsValidator {
     }
   }
 
-  static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
+  static func validate(_ type: ParsableArguments.Type, parent: InputKey?) -> ParsableArgumentsValidatorError? {
     let argumentKeys: [InputKey] = Mirror(reflecting: type.init())
       .children
       .compactMap { child in
@@ -235,7 +235,7 @@ struct ParsableArgumentsUniqueNamesValidator: ParsableArgumentsValidator {
     var kind: ValidatorErrorKind { .failure }
   }
 
-  static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
+  static func validate(_ type: ParsableArguments.Type, parent: InputKey?) -> ParsableArgumentsValidatorError? {
     let argSets: [ArgumentSet] = Mirror(reflecting: type.init())
       .children
       .compactMap { child in
@@ -283,7 +283,7 @@ struct NonsenseFlagsValidator: ParsableArgumentsValidator {
     var kind: ValidatorErrorKind { .warning }
   }
 
-  static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
+  static func validate(_ type: ParsableArguments.Type, parent: InputKey?) -> ParsableArgumentsValidatorError? {
     let argSets: [ArgumentSet] = Mirror(reflecting: type.init())
       .children
       .compactMap { child in

Sources/ArgumentParser/Parsable Types/ParsableCommand.swift

@@ -166,7 +166,7 @@ extension ParsableCommand {
   /// `true` if this command contains any array arguments that are declared
   /// with `.unconditionalRemaining`.
   internal static var includesUnconditionalArguments: Bool {
-    ArgumentSet(self, visibility: .private, parent: .root).contains(where: {
+    ArgumentSet(self, visibility: .private, parent: nil).contains(where: {
       $0.isRepeatingPositional && $0.parsingStrategy == .allRemainingInput
     })
   }

Sources/ArgumentParser/Parsing/ArgumentDefinition.swift

@@ -217,7 +217,7 @@ extension ArgumentDefinition {
   ///
   /// This initializer is used for any property defined on a `ParsableArguments`
   /// type that isn't decorated with one of ArgumentParser's property wrappers.
-  init(unparsedKey: String, default defaultValue: Any?, parent: InputKey.Parent) {
+  init(unparsedKey: String, default defaultValue: Any?, parent: InputKey?) {
     self.init(
       container: Bare<Any>.self,
       key: InputKey(name: unparsedKey, parent: parent),

Sources/ArgumentParser/Parsing/ArgumentSet.swift

@@ -438,7 +438,7 @@ extension ArgumentSet {
   func firstPositional(
     named name: String
   ) -> ArgumentDefinition? {
-    let key = InputKey(name: name, parent: .root)
+    let key = InputKey(name: name, parent: nil)
     return first(where: { $0.help.keys.contains(key) })
   }
 

Sources/ArgumentParser/Parsing/CommandParser.swift

@@ -140,7 +140,7 @@ extension CommandParser {
   /// possible.
   fileprivate mutating func parseCurrent(_ split: inout SplitArguments) throws -> ParsableCommand {
     // Build the argument set (i.e. information on how to parse):
-    let commandArguments = ArgumentSet(currentNode.element, visibility: .private, parent: .root)
+    let commandArguments = ArgumentSet(currentNode.element, visibility: .private, parent: nil)
 
     // Parse the arguments, ignoring anything unexpected
     let values = try commandArguments.lenientParse(
@@ -325,7 +325,7 @@ extension CommandParser {
     let completionValues = Array(args)
 
     // Generate the argument set and parse the argument to find in the set
-    let argset = ArgumentSet(current.element, visibility: .private, parent: .root)
+    let argset = ArgumentSet(current.element, visibility: .private, parent: nil)
     let parsedArgument = try! parseIndividualArg(argToMatch, at: 0).first!
 
     // Look up the specified argument and retrieve its custom completion function

Sources/ArgumentParser/Parsing/InputKey.swift

@@ -9,122 +9,48 @@
 //
 //===----------------------------------------------------------------------===//
 
-/// Represents the path to a parsed field, annotated with ``Flag``, ``Option`` or
-/// ``Argument``. It has a parent, which will either be ``InputKey/Parent/root``
-/// if the field is on the root ``ParsableComand`` or ``AsyncParsableCommand``,
-/// or it will have a ``InputKey/Parent/key(InputKey)`` if it is defined in
-/// a ``ParsableArguments`` instance.
+/// Represents the path to a parsed field, annotated with ``Flag``, ``Option``
+/// or ``Argument``. Fields that are directly declared on a ``ParsableComand``
+/// have a path of length 1, while fields that are declared indirectly (and
+/// included via an option group) have longer paths.
 struct InputKey: Hashable {
-  /// Describes the parent of an ``InputKey``.
-  indirect enum Parent: Hashable {
-    /// There is no parent key.
-    case root
-    /// There is a parent key.
-    case key(InputKey)
-    
-    /// Initialises a parent depending on whether the key is provided.
-    init(_ key: InputKey?) {
-      if let key = key {
-        self = .key(key)
-      } else {
-        self = .root
-      }
-    }
-  }
-  
   /// The name of the input key.
-  let name: String
-  
-  /// The parent of this key.
-  let parent: Parent
+  var name: String
+
+  /// The path through the field's parents, if any.
+  var path: [String]
 
+  /// The full path of the field.
+  var fullPath: [String] { path + [name] }
 
-  /// Constructs a new ``InputKey``, cleaing the `name`, with the specified ``InputKey/Parent``.
+  /// Constructs a new input key, cleaning the name, with the specified parent.
   ///
   /// - Parameter name: The name of the key.
-  /// - Parameter parent: The ``InputKey/Parent`` of the key.
-  init(name: String, parent: Parent) {
-    self.name = Self.clean(codingKey: name)
-    self.parent = parent
-  }
-  
-  @inlinable
-  init?(path: [CodingKey]) {
-    var parentPath = path
-    guard let key = parentPath.popLast() else {
-      return nil
-    }
-    self.name = Self.clean(codingKey: key)
-    self.parent = Parent(InputKey(path: parentPath))
-  }
-
-  /// Constructs a new ``InputKey``, "cleaning the `value` and `path` if necessary.
-  ///
-  /// - Parameter value: The base value of the key.
-  /// - Parameter path: The list of ``CodingKey`` values that lead to this one. May be empty.
-  @inlinable
-  init(name: String, path: [CodingKey]) {
-    self.init(name: name, parent: Parent(InputKey(path: path)))
+  /// - Parameter parent: The input key of the parent.
+  init(name: String, parent: InputKey?) {
+    // Property wrappers have underscore-prefixed names, so we remove the
+    // leading `_`, if present.
+    self.name = name.first == "_"
+      ? String(name.dropFirst(1))
+      : name
+    self.path = parent?.fullPath ?? []
   }
 
-  /// Constructs a new ``InputKey``, "cleaning the `value` and `path` if necessary.
+  /// Constructs a new input key from the given coding key and parent path.
   ///
-  /// - Parameter codingKey: The base ``CodingKey``
-  /// - Parameter path: The list of ``CodingKey`` values that lead to this one. May be empty.
+  /// - Parameter codingKey: The base ``CodingKey``. Leading underscores in
+  ///   `codingKey` is preserved.
+  /// - Parameter path: The list of ``CodingKey`` values that lead to this one.
+  ///   `path` may be empty.
   @inlinable
   init(codingKey: CodingKey, path: [CodingKey]) {
-    self.init(name: codingKey.stringValue, parent: Parent(InputKey(path: path)))
-  }
-  
-  /// The full path, including the ``parent`` and the ``name``.
-  var fullPath: [String] {
-    switch parent {
-    case .root:
-      return [name]
-    case .key(let key):
-      var parentPath = key.fullPath
-      parentPath.append(name)
-      return parentPath
-    }
-  }
-  
-  /// Returns a new ``InputKey`` with the same ``path`` and a new ``name``.
-  /// The new value will be cleaned.
-  ///
-  /// - Parameter newName: The new ``String`` value.
-  /// - Returns: A new ``InputKey`` with the cleaned value and the same ``path``.
-  func with(newName: String) -> InputKey {
-    return .init(name: Self.clean(codingKey: newName), parent: self.parent)
-  }
-}
-
-extension InputKey {
-  /// Property wrappers have underscore-prefixed names, so this returns a "clean"
-  /// version of the `codingKey`, which has the leading `'_'` removed, if present.
-  ///
-  /// - Parameter codingKey: The key to clean.
-  /// - Returns: The cleaned key.
-  static func clean(codingKey: String) -> String {
-    String(codingKey.first == "_" ? codingKey.dropFirst(1) : codingKey.dropFirst(0))
-  }
-  
-  /// Property wrappers have underscore-prefixed names, so this returns a "clean"
-  /// version of the `codingKey`, which has the leading `'_'` removed, if present.
-  ///
-  /// - Parameter codingKey: The key to clean.
-  /// - Returns: The cleaned key.
-  static func clean(codingKey: CodingKey) -> String {
-    clean(codingKey: codingKey.stringValue)
+    self.name = codingKey.stringValue
+    self.path = path.map { $0.stringValue }
   }
 }
 
 extension InputKey: CustomStringConvertible {
   var description: String {
-    switch parent {
-    case .key(let parent):
-      return "\(parent).\(name)"
-    case .root:
-      return name
-    }
+    fullPath.joined(separator: ".")
   }
 }

Sources/ArgumentParser/Usage/DumpHelpGenerator.swift

@@ -38,7 +38,7 @@ fileprivate extension BidirectionalCollection where Element == ParsableCommand.T
   /// Returns the ArgumentSet for the last command in this stack, including
   /// help and version flags, when appropriate.
   func allArguments() -> ArgumentSet {
-    guard var arguments = self.last.map({ ArgumentSet($0, visibility: .private, parent: .root) })
+    guard var arguments = self.last.map({ ArgumentSet($0, visibility: .private, parent: nil) })
     else { return ArgumentSet() }
     self.versionArgumentDefinition().map { arguments.append($0) }
     self.helpArgumentDefinition().map { arguments.append($0) }

Sources/ArgumentParser/Usage/HelpGenerator.swift

@@ -97,7 +97,7 @@ internal struct HelpGenerator {
       fatalError()
     }
 
-    let currentArgSet = ArgumentSet(currentCommand, visibility: visibility, parent: .root)
+    let currentArgSet = ArgumentSet(currentCommand, visibility: visibility, parent: nil)
     self.commandStack = commandStack
 
     // Build the tool name and subcommand name from the command configuration
@@ -292,7 +292,7 @@ fileprivate extension NameSpecification {
   /// step, the name are returned in descending order.
   func generateHelpNames(visibility: ArgumentVisibility) -> [Name] {
     self
-      .makeNames(InputKey(name: "help", parent: .root))
+      .makeNames(InputKey(name: "help", parent: nil))
       .compactMap { name in
         guard visibility.base != .default else { return name }
         switch name {
@@ -333,7 +333,7 @@ internal extension BidirectionalCollection where Element == ParsableCommand.Type
         options: [.isOptional],
         help: "Show the version.",
         defaultValue: nil,
-        key: InputKey(name: "", parent: .root),
+        key: InputKey(name: "", parent: nil),
         isComposite: false),
       completion: .default,
       update: .nullary({ _, _, _ in })
@@ -350,7 +350,7 @@ internal extension BidirectionalCollection where Element == ParsableCommand.Type
         options: [.isOptional],
         help: "Show help information.",
         defaultValue: nil,
-        key: InputKey(name: "", parent: .root),
+        key: InputKey(name: "", parent: nil),
         isComposite: false),
       completion: .default,
       update: .nullary({ _, _, _ in })
@@ -365,7 +365,7 @@ internal extension BidirectionalCollection where Element == ParsableCommand.Type
         options: [.isOptional],
         help: ArgumentHelp("Dump help information as JSON."),
         defaultValue: nil,
-        key: InputKey(name: "", parent: .root),
+        key: InputKey(name: "", parent: nil),
         isComposite: false),
       completion: .default,
       update: .nullary({ _, _, _ in })
@@ -375,7 +375,7 @@ internal extension BidirectionalCollection where Element == ParsableCommand.Type
   /// Returns the ArgumentSet for the last command in this stack, including
   /// help and version flags, when appropriate.
   func argumentsForHelp(visibility: ArgumentVisibility) -> ArgumentSet {
-    guard var arguments = self.last.map({ ArgumentSet($0, visibility: visibility, parent: .root) })
+    guard var arguments = self.last.map({ ArgumentSet($0, visibility: visibility, parent: nil) })
       else { return ArgumentSet() }
     self.versionArgumentDefinition().map { arguments.append($0) }
     self.helpArgumentDefinition().map { arguments.append($0) }