Skip to content

Commit 1b1c487

Browse files
jonsequiturjonsequitur
authored
CommandLineConfiguration.ThrowIfInvalid (#1582)
* CommandLineConfiguration.ThrowIfInvalid * add exception constructors
1 parent 941a651 commit 1b1c487

File tree

6 files changed

+330
-39
lines changed

6 files changed

+330
-39
lines changed

‎src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@
7878
public LocalizationResources LocalizationResources { get; }
7979
public Command RootCommand { get; }
8080
public System.CommandLine.Collections.SymbolSet Symbols { get; }
81+
public System.Void ThrowIfInvalid()
82+
public class CommandLineConfigurationException : System.Exception, System.Runtime.Serialization.ISerializable
83+
.ctor(System.String message)
84+
.ctor()
85+
.ctor(System.String message, System.Exception innerException)
8186
public static class CompletionSourceExtensions
8287
public static System.Void Add(this CompletionSourceList completionSources, System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.String>> complete)
8388
public static System.Void Add(this CompletionSourceList completionSources, System.CommandLine.Completions.CompletionDelegate complete)

‎src/System.CommandLine.Tests/CommandLineConfigurationTests.cs

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using FluentAssertions;
5+
using Xunit;
6+
7+
namespace System.CommandLine.Tests;
8+
9+
public class CommandLineConfigurationTests
10+
{
11+
[Fact]
12+
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_option_aliases_on_the_root_command()
13+
{
14+
var option1 = new Option<string>("--dupe");
15+
var option2 = new Option<string>("-y");
16+
option2.AddAlias("--dupe");
17+
18+
var command = new RootCommand
19+
{
20+
option1,
21+
option2
22+
};
23+
24+
var config = new CommandLineConfiguration(command);
25+
26+
var validate = () => config.ThrowIfInvalid();
27+
28+
validate.Should()
29+
.Throw<CommandLineConfigurationException>()
30+
.Which
31+
.Message
32+
.Should()
33+
.Be($"Duplicate alias '--dupe' found on command '{command.Name}'.");
34+
}
35+
36+
[Fact]
37+
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_option_aliases_on_a_subcommand()
38+
{
39+
var option1 = new Option<string>("--dupe");
40+
var option2 = new Option<string>("--ok");
41+
option2.AddAlias("--dupe");
42+
43+
var command = new RootCommand
44+
{
45+
new Command("subcommand")
46+
{
47+
option1,
48+
option2
49+
}
50+
};
51+
52+
var config = new CommandLineConfiguration(command);
53+
54+
var validate = () => config.ThrowIfInvalid();
55+
56+
validate.Should()
57+
.Throw<CommandLineConfigurationException>()
58+
.Which
59+
.Message
60+
.Should()
61+
.Be("Duplicate alias '--dupe' found on command 'subcommand'.");
62+
}
63+
64+
[Fact]
65+
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_subcommand_aliases_on_the_root_command()
66+
{
67+
var command1 = new Command("dupe");
68+
var command2 = new Command("not-a-dupe");
69+
command2.AddAlias("dupe");
70+
71+
var rootCommand = new RootCommand
72+
{
73+
command1,
74+
command2
75+
};
76+
77+
var config = new CommandLineConfiguration(rootCommand);
78+
79+
var validate = () => config.ThrowIfInvalid();
80+
81+
validate.Should()
82+
.Throw<CommandLineConfigurationException>()
83+
.Which
84+
.Message
85+
.Should()
86+
.Be($"Duplicate alias 'dupe' found on command '{rootCommand.Name}'.");
87+
}
88+
89+
[Fact]
90+
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_subcommand_aliases_on_a_subcommand()
91+
{
92+
var command1 = new Command("dupe");
93+
var command2 = new Command("not-a-dupe");
94+
command2.AddAlias("dupe");
95+
96+
var command = new RootCommand
97+
{
98+
new Command("subcommand")
99+
{
100+
command1,
101+
command2
102+
}
103+
};
104+
105+
var config = new CommandLineConfiguration(command);
106+
107+
var validate = () => config.ThrowIfInvalid();
108+
109+
validate.Should()
110+
.Throw<CommandLineConfigurationException>()
111+
.Which
112+
.Message
113+
.Should()
114+
.Be("Duplicate alias 'dupe' found on command 'subcommand'.");
115+
}
116+
117+
[Fact]
118+
public void ThrowIfInvalid_throws_if_sibling_command_and_option_aliases_collide_on_the_root_command()
119+
{
120+
var option = new Option("dupe");
121+
var command = new Command("not-a-dupe");
122+
command.AddAlias("dupe");
123+
124+
var rootCommand = new RootCommand
125+
{
126+
option,
127+
command
128+
};
129+
130+
var config = new CommandLineConfiguration(rootCommand);
131+
132+
var validate = () => config.ThrowIfInvalid();
133+
134+
validate.Should()
135+
.Throw<CommandLineConfigurationException>()
136+
.Which
137+
.Message
138+
.Should()
139+
.Be($"Duplicate alias 'dupe' found on command '{rootCommand.Name}'.");
140+
}
141+
142+
[Fact]
143+
public void ThrowIfInvalid_throws_if_sibling_command_and_option_aliases_collide_on_a_subcommand()
144+
{
145+
var option = new Option("dupe");
146+
var command = new Command("not-a-dupe");
147+
command.AddAlias("dupe");
148+
149+
var rootCommand = new RootCommand
150+
{
151+
new Command("subcommand")
152+
{
153+
option,
154+
command
155+
}
156+
};
157+
158+
var config = new CommandLineConfiguration(rootCommand);
159+
160+
var validate = () => config.ThrowIfInvalid();
161+
162+
validate.Should()
163+
.Throw<CommandLineConfigurationException>()
164+
.Which
165+
.Message
166+
.Should()
167+
.Be("Duplicate alias 'dupe' found on command 'subcommand'.");
168+
}
169+
170+
[Fact]
171+
public void ThrowIfInvalid_throws_if_there_are_duplicate_sibling_global_option_aliases_on_the_root_command()
172+
{
173+
var option1 = new Option<string>("--dupe");
174+
var option2 = new Option<string>("-y");
175+
option2.AddAlias("--dupe");
176+
177+
var command = new RootCommand();
178+
command.AddGlobalOption(option1);
179+
command.AddGlobalOption(option2);
180+
181+
var config = new CommandLineConfiguration(command);
182+
183+
var validate = () => config.ThrowIfInvalid();
184+
185+
validate.Should()
186+
.Throw<CommandLineConfigurationException>()
187+
.Which
188+
.Message
189+
.Should()
190+
.Be($"Duplicate alias '--dupe' found on command '{command.Name}'.");
191+
}
192+
193+
[Fact]
194+
public void ThrowIfInvalid_does_not_throw_if_global_option_alias_is_the_same_as_local_option_alias()
195+
{
196+
var rootCommand = new RootCommand
197+
{
198+
new Command("subcommand")
199+
{
200+
new Option<string>("--dupe")
201+
}
202+
};
203+
rootCommand.AddGlobalOption(new Option<string>("--dupe"));
204+
205+
var config = new CommandLineConfiguration(rootCommand);
206+
207+
var validate = () => config.ThrowIfInvalid();
208+
209+
validate.Should().NotThrow();
210+
}
211+
212+
[Fact]
213+
public void ThrowIfInvalid_does_not_throw_if_global_option_alias_is_the_same_as_subcommand_alias()
214+
{
215+
var rootCommand = new RootCommand
216+
{
217+
new Command("subcommand")
218+
{
219+
new Command("--dupe")
220+
}
221+
};
222+
rootCommand.AddGlobalOption(new Option<string>("--dupe"));
223+
224+
var config = new CommandLineConfiguration(rootCommand);
225+
226+
var validate = () => config.ThrowIfInvalid();
227+
228+
validate.Should().NotThrow();
229+
}
230+
231+
[Fact]
232+
public void ThrowIfInvalid_throws_if_a_command_is_its_own_parent()
233+
{
234+
var command = new RootCommand();
235+
command.Add(command);
236+
237+
var config = new CommandLineConfiguration(command);
238+
239+
var validate = () => config.ThrowIfInvalid();
240+
241+
validate.Should()
242+
.Throw<CommandLineConfigurationException>()
243+
.Which
244+
.Message
245+
.Should()
246+
.Be($"Cycle detected in command tree. Command '{command.Name}' is its own ancestor.");
247+
}
248+
249+
[Fact]
250+
public void ThrowIfInvalid_throws_if_a_parentage_cycle_is_detected()
251+
{
252+
var command = new Command("command");
253+
var rootCommand = new RootCommand { command };
254+
command.Add(rootCommand);
255+
256+
var config = new CommandLineConfiguration(rootCommand);
257+
258+
var validate = () => config.ThrowIfInvalid();
259+
260+
validate.Should()
261+
.Throw<CommandLineConfigurationException>()
262+
.Which
263+
.Message
264+
.Should()
265+
.Be($"Cycle detected in command tree. Command '{rootCommand.Name}' is its own ancestor.");
266+
}
267+
}

‎src/System.CommandLine/CommandLineConfiguration.cs

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.CommandLine.Invocation;
99
using System.CommandLine.IO;
1010
using System.CommandLine.Parsing;
11+
using System.Linq;
1112

1213
namespace System.CommandLine
1314
{
@@ -53,7 +54,7 @@ public CommandLineConfiguration(
5354

5455
internal static HelpBuilder DefaultHelpBuilderFactory(BindingContext context, int? requestedMaxWidth = null)
5556
{
56-
int maxWidth = requestedMaxWidth ?? int.MaxValue;
57+
int maxWidth = requestedMaxWidth ?? int.MaxValue;
5758
if (context.Console is SystemConsole systemConsole)
5859
{
5960
maxWidth = systemConsole.GetWindowWidth();
@@ -102,58 +103,47 @@ internal static HelpBuilder DefaultHelpBuilderFactory(BindingContext context, in
102103
internal ResponseFileHandling ResponseFileHandling { get; }
103104

104105
/// <summary>
105-
/// Validates all symbols including the child hierarchy.
106+
/// Throws an exception if the parser configuration is ambiguous or otherwise not valid.
106107
/// </summary>
107-
/// <remarks>Due to the performance impact of this method, it's recommended to create
108-
/// a Unit Test that calls this method to verify the RootCommand of every application.</remarks>
109-
internal void ThrowIfInvalid()
108+
/// <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>
109+
/// <exception cref="CommandLineConfigurationException">Thrown if the configuration is found to be invalid.</exception>
110+
public void ThrowIfInvalid()
110111
{
111112
ThrowIfInvalid(RootCommand);
112113

113114
static void ThrowIfInvalid(Command command)
114115
{
115-
for (int i = 0; i < command.Children.Count; i++)
116+
if (command.Parents.FlattenBreadthFirst(c => c.Parents).Any(ancestor => ancestor == command))
116117
{
117-
for (int j = 1; j < command.Children.Count; j++)
118+
throw new CommandLineConfigurationException($"Cycle detected in command tree. Command '{command.Name}' is its own ancestor.");
119+
}
120+
121+
for (var i = 0; i < command.Children.Count; i++)
122+
{
123+
if (command.Children[i] is IdentifierSymbol symbol1AsIdentifier)
118124
{
119-
if (command.Children[j] is IdentifierSymbol identifierSymbol)
125+
for (var j = i + 1; j < command.Children.Count; j++)
120126
{
121-
foreach (string alias in identifierSymbol.Aliases)
127+
if (command.Children[j] is IdentifierSymbol symbol2AsIdentifier)
122128
{
123-
if (command.Children[i].Matches(alias))
129+
foreach (var symbol2Alias in symbol2AsIdentifier.Aliases)
124130
{
125-
throw new ArgumentException($"Alias '{alias}' is already in use.");
131+
if (symbol1AsIdentifier.Name.Equals(symbol2Alias, StringComparison.Ordinal) ||
132+
symbol1AsIdentifier.Aliases.Contains(symbol2Alias))
133+
{
134+
throw new CommandLineConfigurationException($"Duplicate alias '{symbol2Alias}' found on command '{command.Name}'.");
135+
}
126136
}
127137
}
128-
129-
if (identifierSymbol is Command childCommand)
130-
{
131-
if (ReferenceEquals(command, childCommand))
132-
{
133-
throw new ArgumentException("Parent can't be it's own child.");
134-
}
135-
136-
ThrowIfInvalid(childCommand);
137-
}
138138
}
139139

140-
if (command.Children[i].Matches(command.Children[j].Name))
140+
if (symbol1AsIdentifier is Command childCommand)
141141
{
142-
throw new ArgumentException($"Alias '{command.Children[j].Name}' is already in use.");
142+
ThrowIfInvalid(childCommand);
143143
}
144144
}
145-
146-
if (command.Children.Count == 1 && command.Children[0] is Command singleChild)
147-
{
148-
if (ReferenceEquals(command, singleChild))
149-
{
150-
throw new ArgumentException("Parent can't be it's own child.");
151-
}
152-
153-
ThrowIfInvalid(singleChild);
154-
}
155145
}
156146
}
157147
}
158148
}
159-
}
149+
}

0 commit comments

Comments
 (0)