Skip to content

Commit

Permalink
Fixes incorrect value copying when ParseArguments has the same field …
Browse files Browse the repository at this point in the history 
…name+type as the ParseCommand (Issue #322) (#495)

* Contains fixes and test cases for #322
  • Loading branch information
@randomeizer
randomeizer committed Sep 22, 2022
1 parent 774de9c commit 587d26a
Show file tree
Hide file tree
Showing 23 changed files with 301 additions and 137 deletions.
6 changes: 3 additions & 3 deletions Sources/ArgumentParser/Completions/BashCompletionsGenerator.swift
View file
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
2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Completions/CompletionsGenerator.swift
View file
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
View file
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
6 changes: 3 additions & 3 deletions Sources/ArgumentParser/Parsable Properties/NameSpecification.swift
View file
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
View file
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
View file
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
52 changes: 21 additions & 31 deletions Sources/ArgumentParser/Parsable Types/ParsableArgumentsValidation.swift
View file
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
2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Parsable Types/ParsableCommand.swift
View file
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

0 comments on commit 587d26a

Please sign in to comment.