Skip to content

Commit b2fb642

Browse files
committed
Assert that subcommand names are unique
1 parent bf2710b commit b2fb642

File tree

6 files changed

+74
-2
lines changed

6 files changed

+74
-2
lines changed

src/CommandLineUtils/Attributes/SubcommandAttribute.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ public SubcommandAttribute(string name, Type commandType)
3434

3535
internal void Configure(CommandLineApplication app)
3636
{
37+
if (string.IsNullOrEmpty(Name))
38+
{
39+
throw new ArgumentException(Strings.IsNullOrEmpty, nameof(Name));
40+
}
41+
3742
app.Name = Name;
3843
}
3944
}

src/CommandLineUtils/Internal/ReflectionAppBuilder.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,15 @@ private int OnExecute()
395395
private void AddSubcommand(SubcommandAttribute subcommand)
396396
{
397397
var impl = AddSubcommandMethod.MakeGenericMethod(subcommand.CommandType);
398-
impl.Invoke(this, new object[] { subcommand });
398+
try
399+
{
400+
impl.Invoke(this, new object[] { subcommand });
401+
}
402+
catch (TargetInvocationException ex)
403+
{
404+
// unwrap
405+
throw ex.InnerException ?? ex;
406+
}
399407
}
400408

401409
private static readonly MethodInfo AddSubcommandMethod
@@ -404,8 +412,12 @@ private static readonly MethodInfo AddSubcommandMethod
404412
private void AddSubcommandImpl<TSubCommand>(SubcommandAttribute subcommand)
405413
where TSubCommand : class, new()
406414
{
407-
var childApp = App.Command(subcommand.Name, subcommand.Configure);
415+
if (App.Commands.Any(c => c.Name.Equals(subcommand.Name, StringComparison.OrdinalIgnoreCase)))
416+
{
417+
throw new InvalidOperationException(Strings.DuplicateSubcommandName(subcommand.Name));
418+
}
408419

420+
var childApp = App.Command(subcommand.Name, subcommand.Configure);
409421
var builder = new ReflectionAppBuilder<TSubCommand>(childApp);
410422
builder._bindResult = _bindResult;
411423
builder.Initialize();

src/CommandLineUtils/Properties/Strings.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ internal static class Strings
1414
public const string DefaultVersionTemplate = "--version";
1515
public const string DefaultVersionOptionDescription = "Show version information";
1616

17+
public const string IsNullOrEmpty = "Value is null or empty.";
18+
1719
public const string NoValueTypesMustBeBoolean
1820
= "Cannot specify CommandOptionType.NoValue unless the type is boolean.";
1921

@@ -33,6 +35,9 @@ public static string CannotDetermineOptionType(PropertyInfo member)
3335
public static string OptionNameIsAmbiguous(string optionName, PropertyInfo first, PropertyInfo second)
3436
=> $"Ambiguous option name. Both {first.DeclaringType.FullName}.{first.Name} and {second.DeclaringType.FullName}.{second.Name} produce a CommandOption with the name '{optionName}'";
3537

38+
public static string DuplicateSubcommandName(string commandName)
39+
=> $"The subcommand name '{commandName}' has already been been specified. Subcommand names are case insensitive and must be unique.";
40+
3641
public static string BothOptionAndArgumentAttributesCannotBeSpecified(PropertyInfo prop)
3742
=> $"Cannot specify both {nameof(OptionAttribute)} and {nameof(ArgumentAttribute)} on a property {prop.DeclaringType.Name}.{prop.Name}.";
3843

test/CommandLineUtils.Tests/CommandLineApplicationTests.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,5 +750,25 @@ public void OptionsCanVaryByCase()
750750
Assert.False(optBig.HasValue(), "File should not be set");
751751
Assert.True(optLittle.HasValue(), "force should be set");
752752
}
753+
754+
// Assert compatibility with 2.0.0 and Microsoft.Extensions.CommandLineUtils.
755+
[Fact]
756+
public void CommandNamesCannotDifferByCase()
757+
{
758+
var app = new CommandLineApplication(throwOnUnexpectedArg: false);
759+
var cmdUpper = app.Command("CMD1", c =>
760+
{
761+
c.OnExecute(() => 101);
762+
});
763+
var cmdLower = app.Command("cmd1", c =>
764+
{
765+
c.Invoke = () => throw new InvalidOperationException();
766+
});
767+
app.Invoke = () => throw new InvalidOperationException();
768+
769+
Assert.Equal(101, app.Execute("CMD1"));
770+
Assert.Equal(101, app.Execute("cmd1"));
771+
Assert.Equal(101, app.Execute("CmD1"));
772+
}
753773
}
754774
}

test/CommandLineUtils.Tests/ReflectionAppBuilderTests.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,19 @@ public void BindsToStaticPropertiesWithSetterMethod()
370370
Assert.Equal(2, PrivateSetterProgram.StaticValue);
371371
}
372372

373+
[Fact]
374+
public void ItSetsAppNameToEntryAssemblyIfNotSpecified()
375+
{
376+
if (Assembly.GetEntryAssembly() == null)
377+
{
378+
return;
379+
}
380+
var expected = Assembly.GetEntryAssembly().GetName().Name;
381+
var builder = new ReflectionAppBuilder<PrivateSetterProgram>();
382+
builder.Initialize();
383+
Assert.Equal(expected, builder.App.Name);
384+
}
385+
373386
[Theory]
374387
[InlineData("Option123", "o", "option123", "OPTION123")]
375388
[InlineData("dWORD", "d", "d-word", "D_WORD")]

test/CommandLineUtils.Tests/SubCommandTests.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,5 +150,22 @@ public void ItBindsOptionsOnParentItems()
150150
Assert.True(sub.Parent.Parent.Verbose);
151151
Assert.Equal(6, sub.Value);
152152
}
153+
154+
[Subcommand("level1", typeof(Level1Cmd))]
155+
[Subcommand("LEVEL1", typeof(Level2Cmd))]
156+
private class DuplicateSubCommands
157+
{
158+
private void OnExecute()
159+
{
160+
}
161+
}
162+
163+
[Fact]
164+
public void CommandNamesCannotDifferByCaseOnly()
165+
{
166+
var ex = Assert.Throws<InvalidOperationException>(
167+
() => CommandLineApplication.Execute<DuplicateSubCommands>(new TestConsole(_output)));
168+
Assert.Equal(Strings.DuplicateSubcommandName("LEVEL1"), ex.Message);
169+
}
153170
}
154171
}

0 commit comments

Comments
 (0)