Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes incorrect value copying when ParseArguments has the same field name+type as the ParseCommand (Issue #322) #495

Expand Up @@ -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)
ArgumentSet(commands.last!, visibility: .default, parent: .root)
.compactMap { arg -> String? in
guard arg.isPositional else { return nil }

Expand All @@ -148,7 +148,7 @@ struct BashCompletionsGenerator {
let subcommandNames = commands.dropFirst().map { $0._commandName }.joined(separator: " ")
// TODO: Make this work for @Arguments
let argumentName = arg.names.preferredName?.synopsisString
?? arg.help.keys.first?.rawValue ?? "---"
?? arg.help.keys.first?.name ?? "---"

return """
$("${COMP_WORDS[0]}" ---completion \(subcommandNames) -- \(argumentName) "${COMP_WORDS[@]}")
Expand All @@ -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)
ArgumentSet(commands.last!, visibility: .default, parent: .root)
.compactMap { arg -> String? in
let words = arg.bashCompletionWords()
if words.isEmpty { return nil }
Expand Down
Expand Up @@ -105,7 +105,7 @@ extension ArgumentDefinition {
func customCompletionCall(_ commands: [ParsableCommand.Type]) -> String {
let subcommandNames = commands.dropFirst().map { $0._commandName }.joined(separator: " ")
let argumentName = names.preferredName?.synopsisString
?? self.help.keys.first?.rawValue ?? "---"
?? self.help.keys.first?.name ?? "---"
return "---completion \(subcommandNames) -- \(argumentName)"
}
}
Expand Down
24 changes: 12 additions & 12 deletions Sources/ArgumentParser/Parsable Properties/Flag.swift
Expand Up @@ -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(rawValue: String(describing: value))
let defaultKey = InputKey(name: String(describing: value), parent: .key(key))
let defaultNames = Value.name(for: value).makeNames(defaultKey)
return defaultNames.first?.synopsisString
}
Expand All @@ -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(rawValue: String(describing: value))
let caseKey = InputKey(name: String(describing: value), parent: .key(key))
let name = Value.name(for: value)

let helpForCase = caseHelps[i] ?? help
Expand Down Expand Up @@ -510,7 +510,7 @@ extension Flag {
exclusivity: FlagExclusivity = .exclusive,
help: ArgumentHelp? = nil
) where Value == Element?, Element: EnumerableFlag {
self.init(_parsedValue: .init { key in
self.init(_parsedValue: .init { parentKey in
// This gets flipped to `true` the first time one of these flags is
// encountered.
var hasUpdated = false
Expand All @@ -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(rawValue: String(describing: value))
let caseKey = InputKey(name: String(describing: value), parent: .key(parentKey))
let name = Element.name(for: value)
let helpForCase = hasCustomCaseHelp ? (caseHelps[i] ?? help) : help

Expand All @@ -528,11 +528,11 @@ extension Flag {
options: [.isOptional],
help: helpForCase,
defaultValue: nil,
key: key,
key: parentKey,
isComposite: !hasCustomCaseHelp)

return ArgumentDefinition.flag(name: name, key: key, caseKey: caseKey, help: help, parsingStrategy: .default, initialValue: nil as Element?, update: .nullary({ (origin, name, values) in
hasUpdated = try ArgumentSet.updateFlag(key: key, value: value, origin: origin, values: &values, hasUpdated: hasUpdated, exclusivity: exclusivity)
return ArgumentDefinition.flag(name: name, key: parentKey, caseKey: caseKey, help: help, parsingStrategy: .default, initialValue: nil as Element?, update: .nullary({ (origin, name, values) in
hasUpdated = try ArgumentSet.updateFlag(key: parentKey, value: value, origin: origin, values: &values, hasUpdated: hasUpdated, exclusivity: exclusivity)
}))

}
Expand All @@ -547,24 +547,24 @@ extension Flag {
initial: [Element]?,
help: ArgumentHelp? = nil
) where Value == Array<Element>, Element: EnumerableFlag {
self.init(_parsedValue: .init { key in
self.init(_parsedValue: .init { parentKey in
let caseHelps = Element.allCases.map { Element.help(for: $0) }
let hasCustomCaseHelp = caseHelps.contains(where: { $0 != nil })

let args = Element.allCases.enumerated().map { (i, value) -> ArgumentDefinition in
let caseKey = InputKey(rawValue: String(describing: value))
let caseKey = InputKey(name: String(describing: value), parent: .key(parentKey))
let name = Element.name(for: value)
let helpForCase = hasCustomCaseHelp ? (caseHelps[i] ?? help) : help
let help = ArgumentDefinition.Help(
allValues: [],
options: [.isOptional],
help: helpForCase,
defaultValue: nil,
key: key,
key: parentKey,
isComposite: !hasCustomCaseHelp)

return ArgumentDefinition.flag(name: name, key: key, caseKey: caseKey, help: help, parsingStrategy: .default, initialValue: initial, update: .nullary({ (origin, name, values) in
values.update(forKey: key, inputOrigin: origin, initial: [Element](), closure: {
return ArgumentDefinition.flag(name: name, key: parentKey, caseKey: caseKey, help: help, parsingStrategy: .default, initialValue: initial, update: .nullary({ (origin, name, values) in
values.update(forKey: parentKey, inputOrigin: origin, initial: [Element](), closure: {
$0.append(value)
})
}))
Expand Down
Expand Up @@ -136,9 +136,9 @@ extension NameSpecification.Element {
internal func name(for key: InputKey) -> Name? {
switch self.base {
case .long:
return .long(key.rawValue.convertedToSnakeCase(separator: "-"))
return .long(key.name.convertedToSnakeCase(separator: "-"))
case .short:
guard let c = key.rawValue.first else { fatalError("Key '\(key.rawValue)' has not characters to form short option name.") }
guard let c = key.name.first else { fatalError("Key '\(key.name)' has not characters to form short option name.") }
return .short(c)
case .customLong(let name, let withSingleDash):
return withSingleDash
Expand Down Expand Up @@ -167,7 +167,7 @@ extension FlagInversion {
case .short, .customShort:
return includingShort ? element.name(for: key) : nil
case .long:
let modifiedKey = InputKey(rawValue: key.rawValue.addingIntercappedPrefix(prefix))
let modifiedKey = key.with(newName: key.name.addingIntercappedPrefix(prefix))
return element.name(for: modifiedKey)
case .customLong(let name, let withSingleDash):
let modifiedName = name.addingPrefixWithAutodetectedStyle(prefix)
Expand Down
4 changes: 2 additions & 2 deletions Sources/ArgumentParser/Parsable Properties/OptionGroup.swift
Expand Up @@ -64,8 +64,8 @@ public struct OptionGroup<Value: ParsableArguments>: Decodable, ParsedWrapper {
/// Creates a property that represents another parsable type, using the
/// specified visibility.
public init(visibility: ArgumentVisibility = .default) {
self.init(_parsedValue: .init { _ in
ArgumentSet(Value.self, visibility: .private)
self.init(_parsedValue: .init { parentKey in
return ArgumentSet(Value.self, visibility: .private, parent: .key(parentKey))
})
self._visibility = visibility
}
Expand Down
14 changes: 5 additions & 9 deletions Sources/ArgumentParser/Parsable Types/ParsableArguments.swift
Expand Up @@ -269,10 +269,10 @@ extension ArgumentSetProvider {
}

extension ArgumentSet {
init(_ type: ParsableArguments.Type, visibility: ArgumentVisibility) {
init(_ type: ParsableArguments.Type, visibility: ArgumentVisibility, parent: InputKey.Parent) {
#if DEBUG
do {
try type._validate()
try type._validate(parent: parent)
} catch {
assertionFailure("\(error)")
}
Expand All @@ -281,22 +281,18 @@ extension ArgumentSet {
let a: [ArgumentSet] = Mirror(reflecting: type.init())
.children
.compactMap { child -> ArgumentSet? in
guard var codingKey = child.label else { return nil }
guard let codingKey = child.label else { return nil }

if let parsed = child.value as? ArgumentSetProvider {
guard parsed._visibility.isAtLeastAsVisible(as: visibility)
else { return nil }

// Property wrappers have underscore-prefixed names
codingKey = String(codingKey.first == "_"
? codingKey.dropFirst(1)
: codingKey.dropFirst(0))
let key = InputKey(rawValue: codingKey)
let key = InputKey(name: codingKey, parent: parent)
return parsed.argumentSet(for: key)
} else {
let arg = ArgumentDefinition(
unparsedKey: codingKey,
default: nilOrValue(child.value))
default: nilOrValue(child.value), parent: parent)

// Save a non-wrapped property as is
return ArgumentSet(arg)
Expand Down
Expand Up @@ -10,7 +10,7 @@
//===----------------------------------------------------------------------===//

fileprivate protocol ParsableArgumentsValidator {
static func validate(_ type: ParsableArguments.Type) -> ParsableArgumentsValidatorError?
static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError?
}

enum ValidatorErrorKind {
Expand All @@ -37,15 +37,15 @@ struct ParsableArgumentsValidationError: Error, CustomStringConvertible {
}

extension ParsableArguments {
static func _validate() throws {
static func _validate(parent: InputKey.Parent) throws {
let validators: [ParsableArgumentsValidator.Type] = [
PositionalArgumentsValidator.self,
ParsableArgumentsCodingKeyValidator.self,
ParsableArgumentsUniqueNamesValidator.self,
NonsenseFlagsValidator.self,
]
let errors = validators.compactMap { validator in
validator.validate(self)
validator.validate(self, parent: parent)
}
if errors.count > 0 {
throw ParsableArgumentsValidationError(parsableArgumentsType: self, underlayingErrors: errors)
Expand All @@ -68,7 +68,6 @@ fileprivate extension ArgumentSet {
/// in the argument list. Any other configuration leads to ambiguity in
/// parsing the arguments.
struct PositionalArgumentsValidator: ParsableArgumentsValidator {

struct Error: ParsableArgumentsValidatorError, CustomStringConvertible {
let repeatedPositionalArgument: String

Expand All @@ -81,19 +80,16 @@ struct PositionalArgumentsValidator: ParsableArgumentsValidator {
var kind: ValidatorErrorKind { .failure }
}

static func validate(_ type: ParsableArguments.Type) -> ParsableArgumentsValidatorError? {
static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
let sets: [ArgumentSet] = Mirror(reflecting: type.init())
.children
.compactMap { child in
guard
var codingKey = child.label,
let codingKey = child.label,
let parsed = child.value as? ArgumentSetProvider
else { return nil }

// Property wrappers have underscore-prefixed names
codingKey = String(codingKey.first == "_" ? codingKey.dropFirst(1) : codingKey.dropFirst(0))

let key = InputKey(rawValue: codingKey)
let key = InputKey(name: codingKey, parent: parent)
return parsed.argumentSet(for: key)
}

Expand All @@ -107,20 +103,20 @@ struct PositionalArgumentsValidator: ParsableArgumentsValidator {
let firstRepeatedPositionalArgument: ArgumentDefinition = sets[repeatedPositional].firstRepeatedPositionalArgument!
let positionalFollowingRepeatedArgument: ArgumentDefinition = positionalFollowingRepeated.firstPositionalArgument!
return Error(
repeatedPositionalArgument: firstRepeatedPositionalArgument.help.keys.first!.rawValue,
positionalArgumentFollowingRepeated: positionalFollowingRepeatedArgument.help.keys.first!.rawValue)
repeatedPositionalArgument: firstRepeatedPositionalArgument.help.keys.first!.name,
positionalArgumentFollowingRepeated: positionalFollowingRepeatedArgument.help.keys.first!.name)
}
}

/// Ensure that all arguments have corresponding coding keys
struct ParsableArgumentsCodingKeyValidator: ParsableArgumentsValidator {

private struct Validator: Decoder {
let argumentKeys: [String]
let argumentKeys: [InputKey]

enum ValidationResult: Swift.Error {
case success
case missingCodingKeys([String])
case missingCodingKeys([InputKey])
}

let codingPath: [CodingKey] = []
Expand All @@ -135,7 +131,7 @@ struct ParsableArgumentsCodingKeyValidator: ParsableArgumentsValidator {
}

func container<Key>(keyedBy type: Key.Type) throws -> KeyedDecodingContainer<Key> where Key : CodingKey {
let missingKeys = argumentKeys.filter { Key(stringValue: $0) == nil }
let missingKeys = argumentKeys.filter { Key(stringValue: $0.name) == nil }
if missingKeys.isEmpty {
throw ValidationResult.success
} else {
Expand All @@ -147,7 +143,7 @@ struct ParsableArgumentsCodingKeyValidator: ParsableArgumentsValidator {
/// This error indicates that an option, a flag, or an argument of
/// a `ParsableArguments` is defined without a corresponding `CodingKey`.
struct MissingKeysError: ParsableArgumentsValidatorError, CustomStringConvertible {
let missingCodingKeys: [String]
let missingCodingKeys: [InputKey]

var description: String {
let resolution = """
Expand Down Expand Up @@ -194,8 +190,8 @@ struct ParsableArgumentsCodingKeyValidator: ParsableArgumentsValidator {
}
}

static func validate(_ type: ParsableArguments.Type) -> ParsableArgumentsValidatorError? {
let argumentKeys: [String] = Mirror(reflecting: type.init())
static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
let argumentKeys: [InputKey] = Mirror(reflecting: type.init())
.children
.compactMap { child in
guard
Expand All @@ -204,7 +200,7 @@ struct ParsableArgumentsCodingKeyValidator: ParsableArgumentsValidator {
else { return nil }

// Property wrappers have underscore-prefixed names
return String(codingKey.first == "_" ? codingKey.dropFirst(1) : codingKey.dropFirst(0))
return InputKey(name: codingKey, parent: parent)
}
guard argumentKeys.count > 0 else {
return nil
Expand Down Expand Up @@ -239,19 +235,16 @@ struct ParsableArgumentsUniqueNamesValidator: ParsableArgumentsValidator {
var kind: ValidatorErrorKind { .failure }
}

static func validate(_ type: ParsableArguments.Type) -> ParsableArgumentsValidatorError? {
static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
let argSets: [ArgumentSet] = Mirror(reflecting: type.init())
.children
.compactMap { child in
guard
var codingKey = child.label,
let codingKey = child.label,
let parsed = child.value as? ArgumentSetProvider
else { return nil }

// Property wrappers have underscore-prefixed names
codingKey = String(codingKey.first == "_" ? codingKey.dropFirst(1) : codingKey.dropFirst(0))

let key = InputKey(rawValue: codingKey)
let key = InputKey(name: codingKey, parent: parent)
return parsed.argumentSet(for: key)
}

Expand Down Expand Up @@ -290,19 +283,16 @@ struct NonsenseFlagsValidator: ParsableArgumentsValidator {
var kind: ValidatorErrorKind { .warning }
}

static func validate(_ type: ParsableArguments.Type) -> ParsableArgumentsValidatorError? {
static func validate(_ type: ParsableArguments.Type, parent: InputKey.Parent) -> ParsableArgumentsValidatorError? {
let argSets: [ArgumentSet] = Mirror(reflecting: type.init())
.children
.compactMap { child in
guard
var codingKey = child.label,
let codingKey = child.label,
let parsed = child.value as? ArgumentSetProvider
else { return nil }

// Property wrappers have underscore-prefixed names
codingKey = String(codingKey.first == "_" ? codingKey.dropFirst(1) : codingKey.dropFirst(0))

let key = InputKey(rawValue: codingKey)
let key = InputKey(name: codingKey, parent: parent)
return parsed.argumentSet(for: key)
}

Expand Down
Expand Up @@ -160,7 +160,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).contains(where: {
ArgumentSet(self, visibility: .private, parent: .root).contains(where: {
$0.isRepeatingPositional && $0.parsingStrategy == .allRemainingInput
})
}
Expand Down