From f6cfd55a62a0651d5438e380dc5934c2a504b457 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Sat, 7 Nov 2020 15:25:33 -0800 Subject: [PATCH] cleanup: make options, arguments, and commands read-only collections on CommandLineApplication This gives CommandLineApplication more control over how the definition of the application is constructed. Validation can be enforced, and refactoring of the internal storage mechanism can happen without breaking the contract with callers. --- .../Attributes/OptionAttribute.cs | 2 +- .../CommandLineApplication.cs | 41 ++++++++++++------- .../ArgumentAttributeConvention.cs | 2 +- .../DefaultHelpOptionConvention.cs | 2 +- .../RemainingArgsPropertyConvention.cs | 1 + .../Internal/CommandLineProcessor.cs | 4 +- src/CommandLineUtils/PublicAPI.Shipped.txt | 8 ++-- src/CommandLineUtils/PublicAPI.Unshipped.txt | 2 + .../CommandLineApplicationTests.cs | 4 +- .../HelpOptionAttributeTests.cs | 10 ++++- .../OptionAttributeTests.cs | 3 +- .../ValueParserProviderCustomTests.cs | 3 +- .../HostBuilderExtensionsBuilderAPITests.cs | 1 + 13 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/CommandLineUtils/Attributes/OptionAttribute.cs b/src/CommandLineUtils/Attributes/OptionAttribute.cs index 36fd21f8..6f7e1ac9 100755 --- a/src/CommandLineUtils/Attributes/OptionAttribute.cs +++ b/src/CommandLineUtils/Attributes/OptionAttribute.cs @@ -90,7 +90,7 @@ internal CommandOption Configure(CommandLineApplication app, PropertyInfo prop) option.Description ??= prop.Name; - app.Options.Add(option); + app.AddOption(option); return option; } diff --git a/src/CommandLineUtils/CommandLineApplication.cs b/src/CommandLineUtils/CommandLineApplication.cs index 85a0fc36..fa2cfe76 100644 --- a/src/CommandLineUtils/CommandLineApplication.cs +++ b/src/CommandLineUtils/CommandLineApplication.cs @@ -56,6 +56,10 @@ static CommandLineApplication() private readonly Lazy _services; private readonly ConventionContext _conventionContext; private readonly List _conventions = new List(); + private readonly List _arguments = new List(); + private readonly List _options = new List(); + private readonly List _subcommands = new List(); + internal readonly List _remainingArguments = new List(); /// /// Initializes a new instance of . @@ -109,10 +113,6 @@ internal CommandLineApplication( { _context = context ?? throw new ArgumentNullException(nameof(context)); Parent = parent; - Options = new List(); - Arguments = new List(); - Commands = new List(); - RemainingArguments = new List(); _helpTextGenerator = helpTextGenerator ?? throw new ArgumentNullException(nameof(helpTextGenerator)); _handler = DefaultAction; _validationErrorHandler = DefaultValidationErrorHandler; @@ -186,7 +186,7 @@ public string? Name /// /// Available command-line options on this command. Use to get all available options, which may include inherited options. /// - public List Options { get; private set; } + public IReadOnlyCollection Options => _options; /// /// Whether a Pager should be used to display help text. @@ -239,13 +239,13 @@ public CommandOption? OptionHelp /// /// Required command-line arguments. /// - public List Arguments { get; private set; } + public IReadOnlyList Arguments => _arguments; /// /// When initialized when is , /// this will contain any unrecognized arguments. /// - public List RemainingArguments { get; private set; } + public IReadOnlyList RemainingArguments => _remainingArguments; /// /// Configures what the parser should do when it runs into an unexpected argument. @@ -274,7 +274,7 @@ public UnrecognizedArgumentHandling UnrecognizedArgumentHandling /// /// Subcommands. /// - public List Commands { get; private set; } + public IReadOnlyCollection Commands => _subcommands; /// /// Determines if '--' can be used to separate known arguments and options from additional content passed to . @@ -454,7 +454,7 @@ public void AddSubcommand(CommandLineApplication subcommand) subcommand.Parent = this; - Commands.Add(subcommand); + _subcommands.Add(subcommand); } private void AssertCommandNameIsUnique(string? name, CommandLineApplication? commandToIgnore) @@ -562,11 +562,20 @@ public CommandOption Option(string template, string description, CommandOptionTy Description = description, Inherited = inherited }; - Options.Add(option); + AddOption(option); configuration(option); return option; } + /// + /// Add an option to the definition of this command + /// + /// + public void AddOption(CommandOption option) + { + _options.Add(option); + } + /// /// Adds a command line option with values that should be parsable into . /// @@ -591,7 +600,7 @@ public CommandOption Option(string template, string description, CommandOp Description = description, Inherited = inherited }; - Options.Add(option); + AddOption(option); configuration(option); return option; } @@ -662,14 +671,18 @@ public CommandArgument Argument(string name, string description, Action + /// Add an argument to the definition of this command. + /// + /// + public void AddArgument(CommandArgument argument) { var lastArg = Arguments.LastOrDefault(); if (lastArg != null && lastArg.MultipleValues) { throw new InvalidOperationException(Strings.OnlyLastArgumentCanAllowMultipleValues(lastArg.Name)); } - Arguments.Add(argument); + _arguments.Add(argument); } /// @@ -708,7 +721,7 @@ private void Reset() } IsShowingInformation = default; - RemainingArguments.Clear(); + _remainingArguments.Clear(); } /// diff --git a/src/CommandLineUtils/Conventions/ArgumentAttributeConvention.cs b/src/CommandLineUtils/Conventions/ArgumentAttributeConvention.cs index 132e2455..860e67d1 100644 --- a/src/CommandLineUtils/Conventions/ArgumentAttributeConvention.cs +++ b/src/CommandLineUtils/Conventions/ArgumentAttributeConvention.cs @@ -64,7 +64,7 @@ public virtual void Apply(ConventionContext context) } } - context.Application.Arguments.Add(arg.Value); + context.Application.AddArgument(arg.Value); } } diff --git a/src/CommandLineUtils/Conventions/DefaultHelpOptionConvention.cs b/src/CommandLineUtils/Conventions/DefaultHelpOptionConvention.cs index bcadccc3..34a53ba9 100644 --- a/src/CommandLineUtils/Conventions/DefaultHelpOptionConvention.cs +++ b/src/CommandLineUtils/Conventions/DefaultHelpOptionConvention.cs @@ -70,7 +70,7 @@ public void Apply(ConventionContext context) if (help.LongName != null || help.ShortName != null || help.SymbolName != null) { context.Application.OptionHelp = help; - context.Application.Options.Add(help); + context.Application.AddOption(help); } } } diff --git a/src/CommandLineUtils/Conventions/RemainingArgsPropertyConvention.cs b/src/CommandLineUtils/Conventions/RemainingArgsPropertyConvention.cs index fdf1ec2e..94940ce7 100644 --- a/src/CommandLineUtils/Conventions/RemainingArgsPropertyConvention.cs +++ b/src/CommandLineUtils/Conventions/RemainingArgsPropertyConvention.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Reflection; namespace McMaster.Extensions.CommandLineUtils.Conventions diff --git a/src/CommandLineUtils/Internal/CommandLineProcessor.cs b/src/CommandLineUtils/Internal/CommandLineProcessor.cs index 120803af..116fb701 100644 --- a/src/CommandLineUtils/Internal/CommandLineProcessor.cs +++ b/src/CommandLineUtils/Internal/CommandLineProcessor.cs @@ -335,7 +335,7 @@ private bool ProcessUnexpectedArg(string argTypeName, string? argValue = null) case UnrecognizedArgumentHandling.CollectAndContinue: var arg = _enumerator.Current; - _currentCommand.RemainingArguments.Add(arg.Raw); + _currentCommand._remainingArguments.Add(arg.Raw); return true; case UnrecognizedArgumentHandling.StopParsingAndCollect: @@ -354,7 +354,7 @@ private void AddRemainingArgumentValues() var arg = _enumerator.Current; if (arg != null) { - _currentCommand.RemainingArguments.Add(arg.Raw); + _currentCommand._remainingArguments.Add(arg.Raw); } } while (_enumerator.MoveNext()); } diff --git a/src/CommandLineUtils/PublicAPI.Shipped.txt b/src/CommandLineUtils/PublicAPI.Shipped.txt index 7ceb21b6..1476d1c4 100644 --- a/src/CommandLineUtils/PublicAPI.Shipped.txt +++ b/src/CommandLineUtils/PublicAPI.Shipped.txt @@ -105,7 +105,7 @@ McMaster.Extensions.CommandLineUtils.CommandLineApplication.AllowArgumentSeparat McMaster.Extensions.CommandLineUtils.CommandLineApplication.Argument(string! name, string! description, bool multipleValues = false) -> McMaster.Extensions.CommandLineUtils.CommandArgument! McMaster.Extensions.CommandLineUtils.CommandLineApplication.Argument(string! name, string! description, System.Action! configuration, bool multipleValues = false) -> McMaster.Extensions.CommandLineUtils.CommandArgument! McMaster.Extensions.CommandLineUtils.CommandLineApplication.Argument(string! name, string! description, System.Action!>! configuration, bool multipleValues = false) -> McMaster.Extensions.CommandLineUtils.CommandArgument! -McMaster.Extensions.CommandLineUtils.CommandLineApplication.Arguments.get -> System.Collections.Generic.List! +McMaster.Extensions.CommandLineUtils.CommandLineApplication.Arguments.get -> System.Collections.Generic.IReadOnlyList! McMaster.Extensions.CommandLineUtils.CommandLineApplication.ClusterOptions.get -> bool McMaster.Extensions.CommandLineUtils.CommandLineApplication.ClusterOptions.set -> void McMaster.Extensions.CommandLineUtils.CommandLineApplication.Command(string! name, System.Action! configuration) -> McMaster.Extensions.CommandLineUtils.CommandLineApplication! @@ -114,7 +114,7 @@ McMaster.Extensions.CommandLineUtils.CommandLineApplication.CommandLineApplicati McMaster.Extensions.CommandLineUtils.CommandLineApplication.CommandLineApplication(McMaster.Extensions.CommandLineUtils.HelpText.IHelpTextGenerator! helpTextGenerator, McMaster.Extensions.CommandLineUtils.IConsole! console, string! workingDirectory) -> void McMaster.Extensions.CommandLineUtils.CommandLineApplication.CommandLineApplication(McMaster.Extensions.CommandLineUtils.IConsole! console, string! workingDirectory) -> void McMaster.Extensions.CommandLineUtils.CommandLineApplication.CommandLineApplication(McMaster.Extensions.CommandLineUtils.IConsole! console) -> void -McMaster.Extensions.CommandLineUtils.CommandLineApplication.Commands.get -> System.Collections.Generic.List! +McMaster.Extensions.CommandLineUtils.CommandLineApplication.Commands.get -> System.Collections.Generic.IReadOnlyCollection! McMaster.Extensions.CommandLineUtils.CommandLineApplication.Conventions.get -> McMaster.Extensions.CommandLineUtils.Conventions.IConventionBuilder! McMaster.Extensions.CommandLineUtils.CommandLineApplication.Description.get -> string? McMaster.Extensions.CommandLineUtils.CommandLineApplication.Description.set -> void @@ -152,7 +152,7 @@ McMaster.Extensions.CommandLineUtils.CommandLineApplication.Option(string! te McMaster.Extensions.CommandLineUtils.CommandLineApplication.OptionHelp.get -> McMaster.Extensions.CommandLineUtils.CommandOption? McMaster.Extensions.CommandLineUtils.CommandLineApplication.OptionNameValueSeparators.get -> char[]! McMaster.Extensions.CommandLineUtils.CommandLineApplication.OptionNameValueSeparators.set -> void -McMaster.Extensions.CommandLineUtils.CommandLineApplication.Options.get -> System.Collections.Generic.List! +McMaster.Extensions.CommandLineUtils.CommandLineApplication.Options.get -> System.Collections.Generic.IReadOnlyCollection! McMaster.Extensions.CommandLineUtils.CommandLineApplication.OptionsComparison.get -> System.StringComparison McMaster.Extensions.CommandLineUtils.CommandLineApplication.OptionsComparison.set -> void McMaster.Extensions.CommandLineUtils.CommandLineApplication.OptionVersion.get -> McMaster.Extensions.CommandLineUtils.CommandOption? @@ -161,7 +161,7 @@ McMaster.Extensions.CommandLineUtils.CommandLineApplication.Out.set -> void McMaster.Extensions.CommandLineUtils.CommandLineApplication.Parent.get -> McMaster.Extensions.CommandLineUtils.CommandLineApplication? McMaster.Extensions.CommandLineUtils.CommandLineApplication.Parent.set -> void McMaster.Extensions.CommandLineUtils.CommandLineApplication.Parse(params string![]! args) -> McMaster.Extensions.CommandLineUtils.Abstractions.ParseResult! -McMaster.Extensions.CommandLineUtils.CommandLineApplication.RemainingArguments.get -> System.Collections.Generic.List! +McMaster.Extensions.CommandLineUtils.CommandLineApplication.RemainingArguments.get -> System.Collections.Generic.IReadOnlyList! McMaster.Extensions.CommandLineUtils.CommandLineApplication.ResponseFileHandling.get -> McMaster.Extensions.CommandLineUtils.ResponseFileHandling McMaster.Extensions.CommandLineUtils.CommandLineApplication.ResponseFileHandling.set -> void McMaster.Extensions.CommandLineUtils.CommandLineApplication.ShortVersionGetter.get -> System.Func? diff --git a/src/CommandLineUtils/PublicAPI.Unshipped.txt b/src/CommandLineUtils/PublicAPI.Unshipped.txt index c5893cb1..5018755e 100644 --- a/src/CommandLineUtils/PublicAPI.Unshipped.txt +++ b/src/CommandLineUtils/PublicAPI.Unshipped.txt @@ -3,3 +3,5 @@ McMaster.Extensions.CommandLineUtils.CommandArgument.HasValue.get -> bool McMaster.Extensions.CommandLineUtils.CommandArgument.TryParse(string? value) -> bool virtual McMaster.Extensions.CommandLineUtils.CommandArgument.Reset() -> void virtual McMaster.Extensions.CommandLineUtils.CommandOption.Reset() -> void +McMaster.Extensions.CommandLineUtils.CommandLineApplication.AddArgument(McMaster.Extensions.CommandLineUtils.CommandArgument! argument) -> void +McMaster.Extensions.CommandLineUtils.CommandLineApplication.AddOption(McMaster.Extensions.CommandLineUtils.CommandOption! option) -> void diff --git a/test/CommandLineUtils.Tests/CommandLineApplicationTests.cs b/test/CommandLineUtils.Tests/CommandLineApplicationTests.cs index 9e2fba19..daec85cb 100644 --- a/test/CommandLineUtils.Tests/CommandLineApplicationTests.cs +++ b/test/CommandLineUtils.Tests/CommandLineApplicationTests.cs @@ -745,7 +745,7 @@ public void OptionNameCanHaveSemicolon() { LongName = "debug:hive" }; - app.Options.Add(option); + app.AddOption(option); app.Parse("--debug:hive", "abc"); Assert.Equal("abc", option.Value()); } @@ -759,7 +759,7 @@ public void OptionSeparatorMustNotUseSpace() { LongName = "debug:hive" }; - app.Options.Add(option); + app.AddOption(option); var ex = Assert.ThrowsAny(() => app.Parse("--debug:hive", "abc")); diff --git a/test/CommandLineUtils.Tests/HelpOptionAttributeTests.cs b/test/CommandLineUtils.Tests/HelpOptionAttributeTests.cs index 08b032c6..e3a6f9d6 100644 --- a/test/CommandLineUtils.Tests/HelpOptionAttributeTests.cs +++ b/test/CommandLineUtils.Tests/HelpOptionAttributeTests.cs @@ -169,7 +169,10 @@ public void HelpOptionIsInherited() var outWriter = new StringWriter(sb); var app = new CommandLineApplication { Out = outWriter }; app.Conventions.UseDefaultConventions(); - app.Commands.ForEach(f => f.Out = outWriter); + foreach (var subcmd in app.Commands) + { + subcmd.Out = outWriter; + } app.Execute("lvl2", "--help"); var outData = sb.ToString(); @@ -210,7 +213,10 @@ public void NestedHelpOptionsChoosesHelpOptionNearestSelectedCommand(string[] ar return 1; }); - app.Commands.ForEach(f => f.Out = outWriter); + foreach (var subcmd in app.Commands) + { + subcmd.Out = outWriter; + } app.Execute(args); var outData = sb.ToString(); diff --git a/test/CommandLineUtils.Tests/OptionAttributeTests.cs b/test/CommandLineUtils.Tests/OptionAttributeTests.cs index cb8de4c7..fd2878d9 100644 --- a/test/CommandLineUtils.Tests/OptionAttributeTests.cs +++ b/test/CommandLineUtils.Tests/OptionAttributeTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Linq; using System.Reflection; using System.Reflection.Emit; using McMaster.Extensions.CommandLineUtils.Conventions; @@ -314,7 +315,7 @@ private CommandOption CreateOption(Type propType, string propName) var appBuilder = typeof(CommandLineApplication<>).MakeGenericType(program); var app = (CommandLineApplication)Activator.CreateInstance(appBuilder, Util.EmptyArray()); app.Conventions.UseOptionAttributes(); - return app.Options[0]; + return app.Options.First(); } } } diff --git a/test/CommandLineUtils.Tests/ValueParserProviderCustomTests.cs b/test/CommandLineUtils.Tests/ValueParserProviderCustomTests.cs index f4a14ebd..77edfb54 100644 --- a/test/CommandLineUtils.Tests/ValueParserProviderCustomTests.cs +++ b/test/CommandLineUtils.Tests/ValueParserProviderCustomTests.cs @@ -3,6 +3,7 @@ using System; using System.Globalization; +using System.Linq; using System.Threading.Tasks; using McMaster.Extensions.CommandLineUtils.Abstractions; using Xunit; @@ -227,7 +228,7 @@ public void CustomParsersAreAvailableToSubCommands() Assert.Equal(1, result); Assert.Equal(expectedDate, app.Model.MainDate); - Assert.Equal(expectedDate.AddSeconds(123456), ((CommandLineApplication)app.Commands[0]).Model.SubDate); + Assert.Equal(expectedDate.AddSeconds(123456), app.Commands.OfType>().Single().Model.SubDate); } [Fact] diff --git a/test/Hosting.CommandLine.Tests/HostBuilderExtensionsBuilderAPITests.cs b/test/Hosting.CommandLine.Tests/HostBuilderExtensionsBuilderAPITests.cs index c3aa1d2d..84c395d5 100644 --- a/test/Hosting.CommandLine.Tests/HostBuilderExtensionsBuilderAPITests.cs +++ b/test/Hosting.CommandLine.Tests/HostBuilderExtensionsBuilderAPITests.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Linq; using System.Threading.Tasks; using McMaster.Extensions.CommandLineUtils; using McMaster.Extensions.CommandLineUtils.Abstractions;