Make options, arguments, and commands read-only collections on CommandLineApplication (#407)

natemcmaster · natemcmaster · commit 8b08850ece92 · 2021-01-09T16:37:51.000-08:00
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.
diff --git 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
@@ -56,6 +56,10 @@ static CommandLineApplication()
         private readonly Lazy<IServiceProvider> _services;
         private readonly ConventionContext _conventionContext;
         private readonly List<IConvention> _conventions = new List<IConvention>();
+        private readonly List<CommandArgument> _arguments = new List<CommandArgument>();
+        private readonly List<CommandOption> _options = new List<CommandOption>();
+        private readonly List<CommandLineApplication> _subcommands = new List<CommandLineApplication>();
+        internal readonly List<string> _remainingArguments = new List<string>();
 
         /// <summary>
         /// Initializes a new instance of <see cref="CommandLineApplication"/>.
@@ -109,10 +113,6 @@ internal CommandLineApplication(
         {
             _context = context ?? throw new ArgumentNullException(nameof(context));
             Parent = parent;
-            Options = new List<CommandOption>();
-            Arguments = new List<CommandArgument>();
-            Commands = new List<CommandLineApplication>();
-            RemainingArguments = new List<string>();
             _helpTextGenerator = helpTextGenerator ?? throw new ArgumentNullException(nameof(helpTextGenerator));
             _handler = DefaultAction;
             _validationErrorHandler = DefaultValidationErrorHandler;
@@ -186,7 +186,7 @@ public string? Name
         /// <summary>
         /// Available command-line options on this command. Use <see cref="GetOptions"/> to get all available options, which may include inherited options.
         /// </summary>
-        public List<CommandOption> Options { get; private set; }
+        public IReadOnlyCollection<CommandOption> Options => _options;
 
         /// <summary>
         /// Whether a Pager should be used to display help text.
@@ -239,13 +239,13 @@ public CommandOption? OptionHelp
         /// <summary>
         /// Required command-line arguments.
         /// </summary>
-        public List<CommandArgument> Arguments { get; private set; }
+        public IReadOnlyList<CommandArgument> Arguments => _arguments;
 
         /// <summary>
         /// When initialized when <see cref="UnrecognizedArgumentHandling"/> is <see cref="McMaster.Extensions.CommandLineUtils.UnrecognizedArgumentHandling.StopParsingAndCollect" />,
         /// this will contain any unrecognized arguments.
         /// </summary>
-        public List<string> RemainingArguments { get; private set; }
+        public IReadOnlyList<string> RemainingArguments => _remainingArguments;
 
         /// <summary>
         /// Configures what the parser should do when it runs into an unexpected argument.
@@ -274,7 +274,7 @@ public UnrecognizedArgumentHandling UnrecognizedArgumentHandling
         /// <summary>
         /// Subcommands.
         /// </summary>
-        public List<CommandLineApplication> Commands { get; private set; }
+        public IReadOnlyCollection<CommandLineApplication> Commands => _subcommands;
 
         /// <summary>
         /// Determines if '--' can be used to separate known arguments and options from additional content passed to <see cref="RemainingArguments"/>.
@@ -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;
         }
 
+        /// <summary>
+        /// Add an option to the definition of this command
+        /// </summary>
+        /// <param name="option"></param>
+        public void AddOption(CommandOption option)
+        {
+            _options.Add(option);
+        }
+
         /// <summary>
         /// Adds a command line option with values that should be parsable into <typeparamref name="T" />.
         /// </summary>
@@ -591,7 +600,7 @@ public CommandOption<T> Option<T>(string template, string description, CommandOp
                 Description = description,
                 Inherited = inherited
             };
-            Options.Add(option);
+            AddOption(option);
             configuration(option);
             return option;
         }
@@ -662,14 +671,18 @@ public CommandArgument<T> Argument<T>(string name, string description, Action<Co
             return argument;
         }
 
-        private void AddArgument(CommandArgument argument)
+        /// <summary>
+        /// Add an argument to the definition of this command.
+        /// </summary>
+        /// <param name="argument"></param>
+        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);
         }
 
         /// <summary>
@@ -708,7 +721,7 @@ private void Reset()
             }
 
             IsShowingInformation = default;
-            RemainingArguments.Clear();
+            _remainingArguments.Clear();
         }
 
         /// <summary>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<McMaster.Extensions.CommandLineUtils.CommandArgument!>! configuration, bool multipleValues = false) -> McMaster.Extensions.CommandLineUtils.CommandArgument!
 McMaster.Extensions.CommandLineUtils.CommandLineApplication.Argument<T>(string! name, string! description, System.Action<McMaster.Extensions.CommandLineUtils.CommandArgument<T>!>! configuration, bool multipleValues = false) -> McMaster.Extensions.CommandLineUtils.CommandArgument<T>!
-McMaster.Extensions.CommandLineUtils.CommandLineApplication.Arguments.get -> System.Collections.Generic.List<McMaster.Extensions.CommandLineUtils.CommandArgument!>!
+McMaster.Extensions.CommandLineUtils.CommandLineApplication.Arguments.get -> System.Collections.Generic.IReadOnlyList<McMaster.Extensions.CommandLineUtils.CommandArgument!>!
 McMaster.Extensions.CommandLineUtils.CommandLineApplication.ClusterOptions.get -> bool
 McMaster.Extensions.CommandLineUtils.CommandLineApplication.ClusterOptions.set -> void
 McMaster.Extensions.CommandLineUtils.CommandLineApplication.Command(string! name, System.Action<McMaster.Extensions.CommandLineUtils.CommandLineApplication!>! 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!>!
+McMaster.Extensions.CommandLineUtils.CommandLineApplication.Commands.get -> System.Collections.Generic.IReadOnlyCollection<McMaster.Extensions.CommandLineUtils.CommandLineApplication!>!
 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<T>(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.CommandOption!>!
+McMaster.Extensions.CommandLineUtils.CommandLineApplication.Options.get -> System.Collections.Generic.IReadOnlyCollection<McMaster.Extensions.CommandLineUtils.CommandOption!>!
 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<string!>!
+McMaster.Extensions.CommandLineUtils.CommandLineApplication.RemainingArguments.get -> System.Collections.Generic.IReadOnlyList<string!>!
 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<string?>?
diff --git 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
@@ -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<CommandParsingException>(() =>
                 app.Parse("--debug:hive", "abc"));
diff --git 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<Parent> { 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
@@ -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<object>());
             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
@@ -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<CustomParserProgramAttributesSubCommand>)app.Commands[0]).Model.SubDate);
+            Assert.Equal(expectedDate.AddSeconds(123456), app.Commands.OfType<CommandLineApplication<CustomParserProgramAttributesSubCommand>>().Single().Model.SubDate);
         }
 
         [Fact]
diff --git 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;

Original file line number	Diff line number	Diff line change
`@@ -90,7 +90,7 @@ internal CommandOption Configure(CommandLineApplication app, PropertyInfo prop)`
`90`	`90`
`91`	`91`	`option.Description ??= prop.Name;`
`92`	`92`
`93`		`- app.Options.Add(option);`
	`93`	`+ app.AddOption(option);`
`94`	`94`	`return option;`
`95`	`95`	`}`
`96`	`96`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ public virtual void Apply(ConventionContext context)`
`64`	`64`	`}`
`65`	`65`	`}`
`66`	`66`
`67`		`- context.Application.Arguments.Add(arg.Value);`
	`67`	`+ context.Application.AddArgument(arg.Value);`
`68`	`68`	`}`
`69`	`69`	`}`
`70`	`70`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ public void Apply(ConventionContext context)`
`70`	`70`	`if (help.LongName != null \|\| help.ShortName != null \|\| help.SymbolName != null)`
`71`	`71`	`{`
`72`	`72`	`context.Application.OptionHelp = help;`
`73`		`- context.Application.Options.Add(help);`
	`73`	`+ context.Application.AddOption(help);`
`74`	`74`	`}`
`75`	`75`	`}`
`76`	`76`	`}`
Original file line number	Diff line number	Diff line change
`@@ -335,7 +335,7 @@ private bool ProcessUnexpectedArg(string argTypeName, string? argValue = null)`
`335`	`335`
`336`	`336`	`case UnrecognizedArgumentHandling.CollectAndContinue:`
`337`	`337`	`var arg = _enumerator.Current;`
`338`		`- _currentCommand.RemainingArguments.Add(arg.Raw);`
	`338`	`+ _currentCommand._remainingArguments.Add(arg.Raw);`
`339`	`339`	`return true;`
`340`	`340`
`341`	`341`	`case UnrecognizedArgumentHandling.StopParsingAndCollect:`
`@@ -354,7 +354,7 @@ private void AddRemainingArgumentValues()`
`354`	`354`	`var arg = _enumerator.Current;`
`355`	`355`	`if (arg != null)`
`356`	`356`	`{`
`357`		`- _currentCommand.RemainingArguments.Add(arg.Raw);`
	`357`	`+ _currentCommand._remainingArguments.Add(arg.Raw);`
`358`	`358`	`}`
`359`	`359`	`} while (_enumerator.MoveNext());`
`360`	`360`	`}`
Original file line number	Diff line number	Diff line change
`@@ -745,7 +745,7 @@ public void OptionNameCanHaveSemicolon()`
`745`	`745`	`{`
`746`	`746`	`LongName = "debug:hive"`
`747`	`747`	`};`
`748`		`- app.Options.Add(option);`
	`748`	`+ app.AddOption(option);`
`749`	`749`	`app.Parse("--debug:hive", "abc");`
`750`	`750`	`Assert.Equal("abc", option.Value());`
`751`	`751`	`}`
`@@ -759,7 +759,7 @@ public void OptionSeparatorMustNotUseSpace()`
`759`	`759`	`{`
`760`	`760`	`LongName = "debug:hive"`
`761`	`761`	`};`
`762`		`- app.Options.Add(option);`
	`762`	`+ app.AddOption(option);`
`763`	`763`
`764`	`764`	`var ex = Assert.ThrowsAny<CommandParsingException>(() =>`
`765`	`765`	`app.Parse("--debug:hive", "abc"));`