all default value factories should be generic (dotnet#1968)

adamsitnik · web-flow · commit 350a618ab449 · 2022-11-17T10:29:08.000+01:00
diff --git a/src/Common/ArgumentBuilder.cs b/src/Common/ArgumentBuilder.cs
@@ -17,13 +17,25 @@ public static Argument CreateArgument(Type valueType, string name = "value")
         var argumentType = typeof(Argument<>).MakeGenericType(valueType);
 
 #if NET6_0_OR_GREATER
-            var ctor = (ConstructorInfo)argumentType.GetMemberWithSameMetadataDefinitionAs(_ctor);
+        var ctor = (ConstructorInfo)argumentType.GetMemberWithSameMetadataDefinitionAs(_ctor);
 #else
         var ctor = argumentType.GetConstructor(new[] { typeof(string), typeof(string) });
 #endif
 
-        var option = (Argument)ctor.Invoke(new object[] { name, null });
+        return (Argument)ctor.Invoke(new object[] { name, null });
+    }
+
+    internal static Argument CreateArgument(ParameterInfo argsParam)
+    {
+        if (!argsParam.HasDefaultValue)
+        {
+            return CreateArgument(argsParam.ParameterType, argsParam.Name);
+        }
+
+        var argumentType = typeof(Argument<>).MakeGenericType(argsParam.ParameterType);
+
+        var ctor = argumentType.GetConstructor(new[] { typeof(string), argsParam.ParameterType, typeof(string) });
 
-        return option;
+        return (Argument)ctor.Invoke(new object[] { argsParam.Name, argsParam.DefaultValue, null });
     }
 }
diff --git a/src/Common/OptionBuilder.cs b/src/Common/OptionBuilder.cs
@@ -14,7 +14,7 @@ static OptionBuilder()
         _ctor = typeof(Option<string>).GetConstructor(new[] { typeof(string), typeof(string) });
     }
 
-    public static Option CreateOption(string name, Type valueType)
+    public static Option CreateOption(string name, Type valueType, string description = null)
     {
         var optionType = typeof(Option<>).MakeGenericType(valueType);
 
@@ -24,8 +24,34 @@ public static Option CreateOption(string name, Type valueType)
         var ctor = optionType.GetConstructor(new[] { typeof(string), typeof(string) });
 #endif
 
-        var option = (Option)ctor.Invoke(new object[] { name, null });
+        var option = (Option)ctor.Invoke(new object[] { name, description });
 
         return option;
     }
+
+    public static Option CreateOption(string name, Type valueType, string description, Func<object> defaultValueFactory)
+    {
+        if (defaultValueFactory == null)
+        {
+            return CreateOption(name, valueType, description);
+        }
+
+        var optionType = typeof(Bridge<>).MakeGenericType(valueType);
+
+        var ctor = optionType.GetConstructor(new[] { typeof(string), typeof(Func<object>), typeof(string) });
+
+        var option = (Option)ctor.Invoke(new object[] { name, defaultValueFactory, description });
+
+        return option;
+    }
+
+    private class Bridge<T> : Option<T>
+    {
+        public Bridge(string name, Func<object> defaultValueFactory, string description)
+            : base(name, 
+                  () => (T)defaultValueFactory(), // this type exists only for an easy Func<object> => Func<T> transformation
+                  description)
+        {
+        }
+    }
 }
diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt
@@ -9,17 +9,16 @@ System.CommandLine
     public System.Object GetDefaultValue()
     public ParseResult Parse(System.String commandLine)
     public ParseResult Parse(System.String[] args)
-    public System.Void SetDefaultValue(System.Object value)
-    public System.Void SetDefaultValueFactory(System.Func<System.Object> defaultValueFactory)
-    public System.Void SetDefaultValueFactory(System.Func<System.CommandLine.Parsing.ArgumentResult,System.Object> defaultValueFactory)
     public System.String ToString()
   public class Argument<T> : Argument, IValueDescriptor<T>, System.CommandLine.Binding.IValueDescriptor
     .ctor()
     .ctor(System.String name, System.String description = null)
     .ctor(System.String name, Func<T> defaultValueFactory, System.String description = null)
+    .ctor(System.String name, T defaultValue, System.String description = null)
     .ctor(Func<T> defaultValueFactory)
     .ctor(System.String name, Func<System.CommandLine.Parsing.ArgumentResult,T> parse, System.Boolean isDefault = False, System.String description = null)
     .ctor(Func<System.CommandLine.Parsing.ArgumentResult,T> parse, System.Boolean isDefault = False)
+    public System.Boolean HasDefaultValue { get; }
     public System.Type ValueType { get; }
     public Argument<T> AcceptLegalFileNamesOnly()
     public Argument<T> AcceptLegalFilePathsOnly()
@@ -28,6 +27,9 @@ System.CommandLine
     public Argument<T> AddCompletions(System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.String>> completionsDelegate)
     public Argument<T> AddCompletions(System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem>> completionsDelegate)
     public Argument<T> AddValidator(System.Action<System.CommandLine.Parsing.ArgumentResult> validate)
+    public System.Void SetDefaultValue(T value)
+    public System.Void SetDefaultValueFactory(Func<T> defaultValueFactory)
+    public System.Void SetDefaultValueFactory(Func<System.CommandLine.Parsing.ArgumentResult,T> defaultValueFactory)
   public struct ArgumentArity : System.ValueType, System.IEquatable<ArgumentArity>
     public static ArgumentArity ExactlyOne { get; }
     public static ArgumentArity OneOrMore { get; }
@@ -197,8 +199,6 @@ System.CommandLine
     public System.Boolean HasAliasIgnoringPrefix(System.String alias)
     public ParseResult Parse(System.String commandLine)
     public ParseResult Parse(System.String[] args)
-    public System.Void SetDefaultValue(System.Object value)
-    public System.Void SetDefaultValueFactory(System.Func<System.Object> defaultValueFactory)
   public class Option<T> : Option, IValueDescriptor<T>, System.CommandLine.Binding.IValueDescriptor
     .ctor(System.String name, System.String description = null)
     .ctor(System.String[] aliases, System.String description = null)
@@ -213,6 +213,8 @@ System.CommandLine
     public Option<T> AddCompletions(System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.String>> completionsDelegate)
     public Option<T> AddCompletions(System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem>> completionsDelegate)
     public Option<T> AddValidator(System.Action<System.CommandLine.Parsing.OptionResult> validate)
+    public System.Void SetDefaultValue(T value)
+    public System.Void SetDefaultValueFactory(Func<T> defaultValueFactory)
   public static class OptionValidation
     public static Option<System.IO.FileInfo> AcceptExistingOnly(this Option<System.IO.FileInfo> option)
     public static Option<System.IO.DirectoryInfo> AcceptExistingOnly(this Option<System.IO.DirectoryInfo> option)
diff --git a/src/System.CommandLine.DragonFruit/CommandLine.cs b/src/System.CommandLine.DragonFruit/CommandLine.cs
@@ -165,21 +165,7 @@ public static void ConfigureFromMethod(
 
             if (method.GetParameters().FirstOrDefault(p => _argumentParameterNames.Contains(p.Name)) is { } argsParam)
             {
-                var argument = ArgumentBuilder.CreateArgument(argsParam.ParameterType, argsParam.Name);
-
-                if (argsParam.HasDefaultValue)
-                {
-                    if (argsParam.DefaultValue is not null)
-                    {
-                        argument.SetDefaultValue(argsParam.DefaultValue);
-                    }
-                    else
-                    {
-                        argument.SetDefaultValueFactory(() => null);
-                    }
-                }
-
-                command.AddArgument(argument);
+                command.AddArgument(ArgumentBuilder.CreateArgument(argsParam));
             }
 
             command.Handler = CommandHandler.Create(method, target);
@@ -285,24 +271,11 @@ public static IEnumerable<Option> BuildOptions(this MethodInfo method)
         }
 
         public static Option BuildOption(this ParameterDescriptor parameter)
-        {
-            Func<object> getDefaultValue = null;
-            if (parameter.HasDefaultValue)
-            {
-                getDefaultValue = parameter.GetDefaultValue;
-            }
-
-            var option = OptionBuilder.CreateOption(parameter.BuildAlias(), parameter.ValueType);
-            
-            option.Description = parameter.ValueName;
-
-            if (getDefaultValue is not null)
-            {
-                option.SetDefaultValueFactory(getDefaultValue);
-            }
-
-            return option;
-        }
+            => OptionBuilder.CreateOption(
+                parameter.BuildAlias(),
+                parameter.ValueType,
+                parameter.ValueName,
+                parameter.HasDefaultValue ? parameter.GetDefaultValue : null);
 
         private static string GetDefaultXmlDocsFileLocation(Assembly assembly)
         {
diff --git a/src/System.CommandLine/Argument.cs b/src/System.CommandLine/Argument.cs
@@ -14,7 +14,6 @@ namespace System.CommandLine
     /// </summary>
     public abstract class Argument : Symbol, IValueDescriptor
     {
-        private Func<ArgumentResult, object?>? _defaultValueFactory;
         private ArgumentArity _arity;
         private TryConvertArgument? _convertArguments;
         private List<Func<CompletionContext, IEnumerable<CompletionItem>>>? _completions = null;
@@ -115,54 +114,12 @@ private protected override string DefaultName
             return GetDefaultValue(new ArgumentResult(this, null));
         }
 
-        internal object? GetDefaultValue(ArgumentResult argumentResult)
-        {
-            if (_defaultValueFactory is null)
-            {
-                throw new InvalidOperationException($"Argument \"{Name}\" does not have a default value");
-            }
-
-            return _defaultValueFactory.Invoke(argumentResult);
-        }
-
-        /// <summary>
-        /// Sets the default value for the argument.
-        /// </summary>
-        /// <param name="value">The default value for the argument.</param>
-        public void SetDefaultValue(object? value)
-        {
-            SetDefaultValueFactory(() => value);
-        }
-
-        /// <summary>
-        /// Sets a delegate to invoke when the default value for the argument is required.
-        /// </summary>
-        /// <param name="defaultValueFactory">The delegate to invoke to return the default value.</param>
-        /// <exception cref="ArgumentNullException">Thrown when <paramref name="defaultValueFactory"/> is null.</exception>
-        public void SetDefaultValueFactory(Func<object?> defaultValueFactory)
-        {
-            if (defaultValueFactory is null)
-            {
-                throw new ArgumentNullException(nameof(defaultValueFactory));
-            }
-
-            SetDefaultValueFactory(_ => defaultValueFactory());
-        }
-        
-        /// <summary>
-        /// Sets a delegate to invoke when the default value for the argument is required.
-        /// </summary>
-        /// <param name="defaultValueFactory">The delegate to invoke to return the default value.</param>
-        /// <remarks>In this overload, the <see cref="ArgumentResult"/> is provided to the delegate.</remarks>
-        public void SetDefaultValueFactory(Func<ArgumentResult, object?> defaultValueFactory)
-        {
-            _defaultValueFactory = defaultValueFactory ?? throw new ArgumentNullException(nameof(defaultValueFactory));
-        }
+        internal abstract object? GetDefaultValue(ArgumentResult argumentResult);
 
         /// <summary>
         /// Specifies if a default value is defined for the argument.
         /// </summary>
-        public bool HasDefaultValue => _defaultValueFactory is not null;
+        public abstract bool HasDefaultValue { get; }
 
         internal virtual bool HasCustomParser => false;
 
diff --git a/src/System.CommandLine/Argument{T}.cs b/src/System.CommandLine/Argument{T}.cs
@@ -12,6 +12,7 @@ namespace System.CommandLine
     /// <inheritdoc cref="Argument" />
     public class Argument<T> : Argument, IValueDescriptor<T>
     {
+        private Func<ArgumentResult, T>? _defaultValueFactory;
         private readonly bool _hasCustomParser;
 
         /// <summary>
@@ -40,26 +41,30 @@ public Argument(
             Func<T> defaultValueFactory, 
             string? description = null) : this(name, description)
         {
-            if (defaultValueFactory is null)
-            {
-                throw new ArgumentNullException(nameof(defaultValueFactory));
-            }
-
             SetDefaultValueFactory(() => defaultValueFactory());
         }
 
+        /// <summary>
+        /// Initializes a new instance of the Argument class.
+        /// </summary>
+        /// <param name="name">The name of the argument.</param>
+        /// <param name="defaultValue">The default value.</param>
+        /// <param name="description">The description of the argument, shown in help.</param>
+        public Argument(
+            string name,
+            T defaultValue,
+            string? description = null) : this(name, description)
+        {
+            SetDefaultValue(defaultValue);
+        }
+
         /// <summary>
         /// Initializes a new instance of the Argument class.
         /// </summary>
         /// <param name="defaultValueFactory">The delegate to invoke to return the default value.</param>
         /// <exception cref="ArgumentNullException">Thrown when <paramref name="defaultValueFactory"/> is null.</exception>
         public Argument(Func<T> defaultValueFactory) : this()
         {
-            if (defaultValueFactory is null)
-            {
-                throw new ArgumentNullException(nameof(defaultValueFactory));
-            }
-
             SetDefaultValueFactory(() => defaultValueFactory());
         }
 
@@ -73,7 +78,7 @@ public Argument(Func<T> defaultValueFactory) : this()
         /// <exception cref="ArgumentNullException">Thrown when <paramref name="parse"/> is null.</exception>
         public Argument(
             string? name,
-            Func<ArgumentResult, T> parse, 
+            Func<ArgumentResult, T> parse,
             bool isDefault = false,
             string? description = null) : this(name, description)
         {
@@ -84,7 +89,7 @@ public Argument(
 
             if (isDefault)
             {
-                SetDefaultValueFactory(argumentResult => parse(argumentResult));
+                SetDefaultValueFactory(parse);
             }
 
             ConvertArguments = (ArgumentResult argumentResult, out object? value) =>
@@ -120,6 +125,53 @@ public Argument(Func<ArgumentResult, T> parse, bool isDefault = false) : this(nu
         /// <inheritdoc />
         public override Type ValueType => typeof(T);
 
+        /// <inheritdoc />
+        public override bool HasDefaultValue => _defaultValueFactory is not null;
+
+        /// <summary>
+        /// Sets the default value for the argument.
+        /// </summary>
+        /// <param name="value">The default value for the argument.</param>
+        public void SetDefaultValue(T value)
+        {
+            SetDefaultValueFactory(() => value);
+        }
+
+        /// <summary>
+        /// Sets a delegate to invoke when the default value for the argument is required.
+        /// </summary>
+        /// <param name="defaultValueFactory">The delegate to invoke to return the default value.</param>
+        /// <exception cref="ArgumentNullException">Thrown when <paramref name="defaultValueFactory"/> is null.</exception>
+        public void SetDefaultValueFactory(Func<T> defaultValueFactory)
+        {
+            if (defaultValueFactory is null)
+            {
+                throw new ArgumentNullException(nameof(defaultValueFactory));
+            }
+
+            SetDefaultValueFactory(_ => defaultValueFactory());
+        }
+
+        /// <summary>
+        /// Sets a delegate to invoke when the default value for the argument is required.
+        /// </summary>
+        /// <param name="defaultValueFactory">The delegate to invoke to return the default value.</param>
+        /// <remarks>In this overload, the <see cref="ArgumentResult"/> is provided to the delegate.</remarks>
+        public void SetDefaultValueFactory(Func<ArgumentResult, T> defaultValueFactory)
+        {
+            _defaultValueFactory = defaultValueFactory ?? throw new ArgumentNullException(nameof(defaultValueFactory));
+        }
+
+        internal override object? GetDefaultValue(ArgumentResult argumentResult)
+        {
+            if (_defaultValueFactory is null)
+            {
+                throw new InvalidOperationException($"Argument \"{Name}\" does not have a default value");
+            }
+
+            return _defaultValueFactory.Invoke(argumentResult);
+        }
+
         /// <summary>
         /// Adds completions for the argument.
         /// </summary>
diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs
@@ -121,21 +121,6 @@ public bool HasAliasIgnoringPrefix(string alias)
             return false;
         }
 
-        /// <summary>
-        /// Sets the default value for the option.
-        /// </summary>
-        /// <param name="value">The default value for the option.</param>
-        public void SetDefaultValue(object? value) =>
-            Argument.SetDefaultValue(value);
-
-        /// <summary>
-        /// Sets a delegate to invoke when the default value for the option is required.
-        /// </summary>
-        /// <param name="defaultValueFactory">The delegate to invoke to return the default value.</param>
-        /// <exception cref="ArgumentNullException">Thrown when <paramref name="defaultValueFactory"/> is null.</exception>
-        public void SetDefaultValueFactory(Func<object?> defaultValueFactory) =>
-            Argument.SetDefaultValueFactory(defaultValueFactory);
-
         /// <summary>
         /// Gets a value that indicates whether multiple argument tokens are allowed for each option identifier token.
         /// </summary>
diff --git a/src/System.CommandLine/Option{T}.cs b/src/System.CommandLine/Option{T}.cs
