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.

Sign up for GitHub

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

CommandLineConfiguration.ThrowIfInvalid #1582

Merged
merged 2 commits into from
Jan 17, 2022
Merged

CommandLineConfiguration.ThrowIfInvalid #1582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
443932d
CommandLineConfiguration.ThrowIfInvalid
jonsequitur Jan 14, 2022
3f7dec2
add exception constructors:
jonsequitur Jan 14, 2022
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
5 changes: 5 additions & 0 deletions ...ty.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@
public LocalizationResources LocalizationResources { get; }
public Command RootCommand { get; }
public System.CommandLine.Collections.SymbolSet Symbols { get; }
public System.Void ThrowIfInvalid()
public class CommandLineConfigurationException : System.Exception, System.Runtime.Serialization.ISerializable
.ctor(System.String message)
.ctor()
.ctor(System.String message, System.Exception innerException)
public static class CompletionSourceExtensions
public static System.Void Add(this CompletionSourceList completionSources, System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.String>> complete)
public static System.Void Add(this CompletionSourceList completionSources, System.CommandLine.Completions.CompletionDelegate complete)
Expand Down
267 changes: 267 additions & 0 deletions src/System.CommandLine.Tests/CommandLineConfigurationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using FluentAssertions;
using Xunit;

namespace System.CommandLine.Tests;

public class CommandLineConfigurationTests
{
[Fact]
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_option_aliases_on_the_root_command()
{
var option1 = new Option<string>("--dupe");
var option2 = new Option<string>("-y");
option2.AddAlias("--dupe");

var command = new RootCommand
{
option1,
option2
};

var config = new CommandLineConfiguration(command);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be($"Duplicate alias '--dupe' found on command '{command.Name}'.");
}

[Fact]
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_option_aliases_on_a_subcommand()
{
var option1 = new Option<string>("--dupe");
var option2 = new Option<string>("--ok");
option2.AddAlias("--dupe");

var command = new RootCommand
{
new Command("subcommand")
{
option1,
option2
}
};

var config = new CommandLineConfiguration(command);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be("Duplicate alias '--dupe' found on command 'subcommand'.");
}

[Fact]
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_subcommand_aliases_on_the_root_command()
{
var command1 = new Command("dupe");
var command2 = new Command("not-a-dupe");
command2.AddAlias("dupe");

var rootCommand = new RootCommand
{
command1,
command2
};

var config = new CommandLineConfiguration(rootCommand);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be($"Duplicate alias 'dupe' found on command '{rootCommand.Name}'.");
}

[Fact]
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_subcommand_aliases_on_a_subcommand()
{
var command1 = new Command("dupe");
var command2 = new Command("not-a-dupe");
command2.AddAlias("dupe");

var command = new RootCommand
{
new Command("subcommand")
{
command1,
command2
}
};

var config = new CommandLineConfiguration(command);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be("Duplicate alias 'dupe' found on command 'subcommand'.");
}

[Fact]
public void ThrowIfInvalid_throws_if_sibling_command_and_option_aliases_collide_on_the_root_command()
{
var option = new Option("dupe");
var command = new Command("not-a-dupe");
command.AddAlias("dupe");

var rootCommand = new RootCommand
{
option,
command
};

var config = new CommandLineConfiguration(rootCommand);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be($"Duplicate alias 'dupe' found on command '{rootCommand.Name}'.");
}

[Fact]
public void ThrowIfInvalid_throws_if_sibling_command_and_option_aliases_collide_on_a_subcommand()
{
var option = new Option("dupe");
var command = new Command("not-a-dupe");
command.AddAlias("dupe");

var rootCommand = new RootCommand
{
new Command("subcommand")
{
option,
command
}
};

var config = new CommandLineConfiguration(rootCommand);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be("Duplicate alias 'dupe' found on command 'subcommand'.");
}

[Fact]
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_global_option_aliases_on_the_root_command()
{
var option1 = new Option<string>("--dupe");
var option2 = new Option<string>("-y");
option2.AddAlias("--dupe");

var command = new RootCommand();
command.AddGlobalOption(option1);
command.AddGlobalOption(option2);

var config = new CommandLineConfiguration(command);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be($"Duplicate alias '--dupe' found on command '{command.Name}'.");
}

[Fact]
public void ThrowIfInvalid_does_not_throw_if_global_option_alias_is_the_same_as_local_option_alias()
{
var rootCommand = new RootCommand
{
new Command("subcommand")
{
new Option<string>("--dupe")
}
};
rootCommand.AddGlobalOption(new Option<string>("--dupe"));

var config = new CommandLineConfiguration(rootCommand);

var validate = () => config.ThrowIfInvalid();

validate.Should().NotThrow();
}

[Fact]
public void ThrowIfInvalid_does_not_throw_if_global_option_alias_is_the_same_as_subcommand_alias()
{
var rootCommand = new RootCommand
{
new Command("subcommand")
{
new Command("--dupe")
}
};
rootCommand.AddGlobalOption(new Option<string>("--dupe"));

var config = new CommandLineConfiguration(rootCommand);

var validate = () => config.ThrowIfInvalid();

validate.Should().NotThrow();
}

[Fact]
public void ThrowIfInvalid_throws_if_a_command_is_its_own_parent()
{
var command = new RootCommand();
command.Add(command);

var config = new CommandLineConfiguration(command);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be($"Cycle detected in command tree. Command '{command.Name}' is its own ancestor.");
}

[Fact]
public void ThrowIfInvalid_throws_if_a_parentage_cycle_is_detected()
{
var command = new Command("command");
var rootCommand = new RootCommand { command };
command.Add(rootCommand);

var config = new CommandLineConfiguration(rootCommand);

var validate = () => config.ThrowIfInvalid();

validate.Should()
.Throw<CommandLineConfigurationException>()
.Which
.Message
.Should()
.Be($"Cycle detected in command tree. Command '{rootCommand.Name}' is its own ancestor.");
}
}
58 changes: 24 additions & 34 deletions src/System.CommandLine/CommandLineConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.CommandLine.Invocation;
using System.CommandLine.IO;
using System.CommandLine.Parsing;
using System.Linq;

namespace System.CommandLine
{
Expand Down Expand Up @@ -53,7 +54,7 @@ public CommandLineConfiguration(

internal static HelpBuilder DefaultHelpBuilderFactory(BindingContext context, int? requestedMaxWidth = null)
{
int maxWidth = requestedMaxWidth ?? int.MaxValue;
int maxWidth = requestedMaxWidth ?? int.MaxValue;
if (context.Console is SystemConsole systemConsole)
{
maxWidth = systemConsole.GetWindowWidth();
Expand Down Expand Up @@ -102,58 +103,47 @@ internal static HelpBuilder DefaultHelpBuilderFactory(BindingContext context, in
internal ResponseFileHandling ResponseFileHandling { get; }

/// <summary>
/// Validates all symbols including the child hierarchy.
/// Throws an exception if the parser configuration is ambiguous or otherwise not valid.
/// </summary>
/// <remarks>Due to the performance impact of this method, it's recommended to create
/// a Unit Test that calls this method to verify the RootCommand of every application.</remarks>
internal void ThrowIfInvalid()
/// <remarks>Due to the performance cost of this method, it is recommended to be used in unit testing or in scenarios where the parser is configured dynamically at runtime.</remarks>
/// <exception cref="CommandLineConfigurationException">Thrown if the configuration is found to be invalid.</exception>
public void ThrowIfInvalid()
{
ThrowIfInvalid(RootCommand);

static void ThrowIfInvalid(Command command)
{
for (int i = 0; i < command.Children.Count; i++)
if (command.Parents.FlattenBreadthFirst(c => c.Parents).Any(ancestor => ancestor == command))
{
for (int j = 1; j < command.Children.Count; j++)
throw new CommandLineConfigurationException($"Cycle detected in command tree. Command '{command.Name}' is its own ancestor.");
}

for (var i = 0; i < command.Children.Count; i++)
{
if (command.Children[i] is IdentifierSymbol symbol1AsIdentifier)
{
if (command.Children[j] is IdentifierSymbol identifierSymbol)
for (var j = i + 1; j < command.Children.Count; j++)
{
foreach (string alias in identifierSymbol.Aliases)
if (command.Children[j] is IdentifierSymbol symbol2AsIdentifier)
{
if (command.Children[i].Matches(alias))
foreach (var symbol2Alias in symbol2AsIdentifier.Aliases)
{
throw new ArgumentException($"Alias '{alias}' is already in use.");
if (symbol1AsIdentifier.Name.Equals(symbol2Alias, StringComparison.Ordinal) ||
symbol1AsIdentifier.Aliases.Contains(symbol2Alias))
{
throw new CommandLineConfigurationException($"Duplicate alias '{symbol2Alias}' found on command '{command.Name}'.");
}
}
}

if (identifierSymbol is Command childCommand)
{
if (ReferenceEquals(command, childCommand))
{
throw new ArgumentException("Parent can't be it's own child.");
}

ThrowIfInvalid(childCommand);
}
}

if (command.Children[i].Matches(command.Children[j].Name))
if (symbol1AsIdentifier is Command childCommand)
{
throw new ArgumentException($"Alias '{command.Children[j].Name}' is already in use.");
ThrowIfInvalid(childCommand);
}
}

if (command.Children.Count == 1 && command.Children[0] is Command singleChild)
{
if (ReferenceEquals(command, singleChild))
{
throw new ArgumentException("Parent can't be it's own child.");
}

ThrowIfInvalid(singleChild);
}
}
}
}
}
}
}
Loading