remove arity parameter from Option<T> constructor, expose it as a pro…

…perty
darkoa-msft · May 31, 2021 · ce31135 · ce31135
1 parent 997ec58
commit ce31135
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 66 deletions.
diff --git a/src/System.CommandLine.Tests/OptionTests.cs b/src/System.CommandLine.Tests/OptionTests.cs
@@ -505,6 +505,14 @@ public void Option_of_boolean_defaults_to_false_when_not_specified()
                   .BeFalse();
         }
 
+        [Fact]
+        public void Arity_of_non_generic_option_defaults_to_zero()
+        {
+            var option = new Option("-x");
+
+            option.Arity.Should().BeEquivalentTo(ArgumentArity.Zero);
+        }
+
         protected override Symbol CreateSymbol(string name) => new Option(name);
     }
 }
diff --git a/src/System.CommandLine.Tests/ParserTests.cs b/src/System.CommandLine.Tests/ParserTests.cs
diff --git a/src/System.CommandLine/Argument.cs b/src/System.CommandLine/Argument.cs
@@ -32,12 +32,14 @@ public Argument()
         /// Initializes a new instance of the Argument class.
         /// </summary>
         /// <param name="name">The name of the argument.</param>
-        public Argument(string name) 
+        public Argument(string name)
         {
-            if (!string.IsNullOrWhiteSpace(name))
+            if (string.IsNullOrWhiteSpace(name))
             {
-                Name = name!;
+                throw new ArgumentException("Value cannot be null or whitespace.", nameof(name));
             }
+
+            Name = name!;
         }
 
         internal HashSet<string>? AllowedValues { get; private set; }
@@ -194,7 +196,7 @@ public void SetDefaultValueFactory(Func<ArgumentResult, object?> getDefaultValue
         /// </summary>
         public bool HasDefaultValue => _defaultValueFactory != null;
 
-        internal static Argument None => new Argument { Arity = ArgumentArity.Zero };
+        internal static Argument None() => new Argument { Arity = ArgumentArity.Zero };
 
         internal void AddAllowedValues(IEnumerable<string> values)
         {

diff --git a/src/System.CommandLine/Command.cs b/src/System.CommandLine/Command.cs
@@ -135,8 +135,6 @@ private protected override void AddSymbol(Symbol symbol)
                 _globalOptions.ThrowIfAnyAliasIsInUse(option);
             }
 
-            symbol.AddParent(this);
-
             base.AddSymbol(symbol);
         }
 

diff --git a/src/System.CommandLine/Help/HelpOption.cs b/src/System.CommandLine/Help/HelpOption.cs
@@ -17,10 +17,7 @@ public HelpOption() : base(new[]
             DisallowBinding = true;
         }
 
-        internal override Argument Argument
-        {
-            get => Argument.None;
-        }
+        internal override Argument Argument => Argument.None();
 
         public override bool Equals(object obj)
         {

diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs
@@ -93,8 +93,8 @@ internal virtual Argument Argument
                 switch (Children.Arguments.Count)
                 {
                     case 0:
-                        var none = Argument.None;
-                        Children.Add(none);
+                        var none = Argument.None();
+                        AddSymbol(none);
                         return none;
 
                     default:
@@ -111,7 +111,13 @@ public string ArgumentHelpName
             set => Argument.Name = value;
         }
 
-        internal bool DisallowBinding { get; set; } 
+        public IArgumentArity Arity
+        {
+            get => Argument.Arity;
+            set => Argument.Arity = value;
+        }
+
+        internal bool DisallowBinding { get; set; }
 
         public override string Name
         {

diff --git a/src/System.CommandLine/Option{T}.cs b/src/System.CommandLine/Option{T}.cs
@@ -9,9 +9,8 @@ public class Option<T> : Option
     {
         public Option(
             string alias,
-            string? description = null,
-            IArgumentArity? arity = null) 
-            : base(new[] { alias }, description, new Argument<T> { Arity = arity })
+            string? description = null) 
+            : base(new[] { alias }, description, new Argument<T>())
         { }
 
         public Option(

diff --git a/src/System.CommandLine/Symbol.cs b/src/System.CommandLine/Symbol.cs
@@ -48,12 +48,12 @@ internal void AddParent(Symbol symbol)
         private protected virtual void AddSymbol(Symbol symbol)
         {
             Children.Add(symbol);
+            symbol.AddParent(this);
         }
 
         private protected void AddArgumentInner(Argument argument)
         {
             argument.AddParent(this);
-
             Children.Add(argument);
         }