Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Parsable Properties/Flag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ extension Flag where Value: CaseIterable, Value: RawRepresentable, Value.RawValu
switch (hasUpdated, previous, exclusivity) {
case (true, let p?, .exclusive):
// This value has already been set.
throw ParserError.duplicateExclusiveValues(previous: p.inputOrigin, duplicate: origin)
throw ParserError.duplicateExclusiveValues(previous: p.inputOrigin, duplicate: origin, originalInput: values.originalInput)
case (false, _, _), (_, _, .chooseLast):
values.set(value, forKey: key, inputOrigin: origin)
default:
Expand Down
14 changes: 13 additions & 1 deletion Sources/ArgumentParser/Parsing/InputOrigin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@
/// This is usually an index into the `SplitArguments`.
/// In some cases it can be multiple indices.
struct InputOrigin: Equatable, ExpressibleByArrayLiteral {
enum Element: Hashable {
enum Element: Comparable, Hashable {
case argumentIndex(SplitArguments.Index)
}

private var _elements: Set<Element> = []
var elements: [Element] {
Array(_elements).sorted()
}

init() {
}
Expand Down Expand Up @@ -61,3 +64,12 @@ struct InputOrigin: Equatable, ExpressibleByArrayLiteral {
_elements.forEach(closure)
}
}

extension InputOrigin.Element {
static func < (lhs: Self, rhs: Self) -> Bool {
switch (lhs, rhs) {
case (.argumentIndex(let l), .argumentIndex(let r)):
return l < r
}
}
}
2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Parsing/ParserError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ enum ParserError: Error {
case missingValueForOption(InputOrigin, Name)
case unexpectedValueForOption(InputOrigin.Element, Name, String)
case unexpectedExtraValues([(InputOrigin, String)])
case duplicateExclusiveValues(previous: InputOrigin, duplicate: InputOrigin)
case duplicateExclusiveValues(previous: InputOrigin, duplicate: InputOrigin, originalInput: [String])
/// We need a value for the given key, but it’s not there. Some non-optional option or argument is missing.
case noValue(forKey: InputKey)
case unableToParseValue(InputOrigin, Name?, String, forKey: InputKey)
Expand Down
23 changes: 19 additions & 4 deletions Sources/ArgumentParser/Usage/UsageGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ extension ErrorMessageGenerator {
return unexpectedValueForOptionMessage(origin: o, name: n, value: v)
case .unexpectedExtraValues(let v):
return unexpectedExtraValuesMessage(values: v)
case .duplicateExclusiveValues(previous: let previous, duplicate: let duplicate):
return duplicateExclusiveValues(previous: previous, duplicate: duplicate)
case .duplicateExclusiveValues(previous: let previous, duplicate: let duplicate, originalInput: let arguments):
return duplicateExclusiveValues(previous: previous, duplicate: duplicate, arguments: arguments)
case .noValue(forKey: let k):
return noValueMessage(key: k)
case .unableToParseValue(let o, let n, let v, forKey: let k):
Expand Down Expand Up @@ -301,8 +301,23 @@ extension ErrorMessageGenerator {
}
}

func duplicateExclusiveValues(previous: InputOrigin, duplicate: InputOrigin) -> String? {
return "Value at position \(duplicate) has already been set from value at position \(previous)."
func duplicateExclusiveValues(previous: InputOrigin, duplicate: InputOrigin, arguments: [String]) -> String? {
func elementString(_ origin: InputOrigin, _ arguments: [String]) -> String? {
guard case .argumentIndex(let split) = origin.elements.first else { return nil }
var argument = "\'\(arguments[split.inputIndex.rawValue])\'"
if case let .sub(offsetIndex) = split.subIndex {
let stringIndex = argument.index(argument.startIndex, offsetBy: offsetIndex+2)
argument = "\'\(argument[stringIndex])\' in \(argument)"
}
return "flag \(argument)"
}

// Note that the RHS of these coalescing operators cannot be reached at this time.
let dupeString = elementString(duplicate, arguments) ?? "position \(duplicate)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add to-do comment here to about updating this when we support environment values? It's worth noting that the RHS of these coalescing operators won't ever get hit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

let origString = elementString(previous, arguments) ?? "position \(previous)"

//TODO: review this message once environment values are supported.
return "Value to be set with \(dupeString) had already been set with \(origString)"
}

func noValueMessage(key: InputKey) -> String? {
Expand Down
16 changes: 16 additions & 0 deletions Tests/UnitTests/ErrorMessageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,19 @@ extension ErrorMessageTests {
AssertErrorMessage(Qwz.self, ["-x"], "Unknown option '-x'")
}
}

private enum OutputBehaviour: String, CaseIterable { case stats, count, list }
private struct Options: ParsableArguments {
@Flag(name: .shortAndLong, help: "Program output")
var behaviour: OutputBehaviour

@Flag() var bool: Bool
}

extension ErrorMessageTests {
func testDuplicateFlags() {
AssertErrorMessage(Options.self, ["--list", "--bool", "-s"], "Value to be set with flag \'-s\' had already been set with flag \'--list\'")
AssertErrorMessage(Options.self, ["-cbl"], "Value to be set with flag \'l\' in \'-cbl\' had already been set with flag \'c\' in \'-cbl\'")
AssertErrorMessage(Options.self, ["-bc", "--stats", "-l"], "Value to be set with flag \'--stats\' had already been set with flag \'c\' in \'-bc\'")
}
}