Skip to content

Commit

Permalink
Abort on ambiguous subcommand names
Browse files Browse the repository at this point in the history
Currently it's possible to have two commands with the same _commandName
by mixing a combination of the automatic command name generation based on
the type name, and explicit use of `CommandConfiguration`'s commandName
parameter. For example, if we edit the math examples `Add` command to
have a commandName of "multiply" this is allowed currently:

```
./.build/debug/math multiply 2 3
5
```

The behavior today is whatever subcommand is registered in the tree first
is what will resolve for that command. So, `Multiply` in this example is
permanently shadowed.

This change just makes sure there's no occurence of this happening in any
level of the tree of potential subcommands, and if so we'll abort early.
  • Loading branch information
dcantah committed Apr 4, 2024
1 parent 4698969 commit 1d3e481
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 0 deletions.
2 changes: 2 additions & 0 deletions Sources/ArgumentParser/Parsing/CommandParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ struct CommandParser {
self.commandTree = try Tree(root: rootCommand)
} catch Tree<ParsableCommand.Type>.InitializationError.recursiveSubcommand(let command) {
fatalError("The ParsableCommand \"\(command)\" can't have itself as its own subcommand.")
} catch Tree<ParsableCommand.Type>.InitializationError.matchingCommandNames(let command, let commandName) {
fatalError("Ambiguous subcommand name \"\(commandName)\" encountered in ParsableCommand \"\(command)\"")
} catch {
fatalError("Unexpected error: \(error).")
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/ArgumentParser/Utilities/Tree.swift
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,12 @@ extension Tree where Element == ParsableCommand.Type {

convenience init(root command: ParsableCommand.Type) throws {
self.init(command)
var subcommandNames = Set<String>()
for subcommand in command.configuration.subcommands {
if subcommandNames.contains(subcommand._commandName) {
throw InitializationError.matchingCommandNames(command, subcommand._commandName)
}
subcommandNames.insert(subcommand._commandName)
if subcommand == command {
throw InitializationError.recursiveSubcommand(subcommand)
}
Expand All @@ -100,5 +105,6 @@ extension Tree where Element == ParsableCommand.Type {

enum InitializationError: Error {
case recursiveSubcommand(ParsableCommand.Type)
case matchingCommandNames(ParsableCommand.Type, String)
}
}
32 changes: 32 additions & 0 deletions Tests/ArgumentParserUnitTests/TreeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,35 @@ extension TreeTests {
XCTAssertThrowsError(try Tree(root: Root.asCommand))
}
}

extension TreeTests {
struct RootWithSubs: ParsableCommand {
static let configuration = CommandConfiguration(subcommands: [Sub.self, SubTwo.self])

struct Sub: ParsableCommand {
}

struct SubTwo: ParsableCommand {
static let configuration = CommandConfiguration(commandName: "sub")
}
}

struct RootWithNestedSubs: ParsableCommand {
static let configuration = CommandConfiguration(subcommands: [Sub.self])

struct Sub: ParsableCommand {
static let configuration = CommandConfiguration(subcommands: [Nested.self, NestedTwo.self])
struct Nested: ParsableCommand {
}

struct NestedTwo: ParsableCommand {
static let configuration = CommandConfiguration(commandName: "nested")
}
}
}

func testInitializationWithMatchingCommandNames() {
XCTAssertThrowsError(try Tree(root: RootWithSubs.asCommand))
XCTAssertThrowsError(try Tree(root: RootWithNestedSubs.asCommand))
}
}

0 comments on commit 1d3e481

Please sign in to comment.