From b0390bd5cacf4b6b0d26a314bd0b7157d068f956 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 13 Aug 2025 13:02:04 +0200 Subject: [PATCH 1/2] remove ArgumentBuilder and OptionBuilder that use reflection a lot and are used only by the unit test project --- src/Common/ArgumentBuilder.cs | 51 ---------------- src/Common/OptionBuilder.cs | 60 ------------------- .../Binding/TypeConversionTests.cs | 58 +++++++++--------- .../System.CommandLine.Tests.csproj | 2 - 4 files changed, 27 insertions(+), 144 deletions(-) delete mode 100644 src/Common/ArgumentBuilder.cs delete mode 100644 src/Common/OptionBuilder.cs diff --git a/src/Common/ArgumentBuilder.cs b/src/Common/ArgumentBuilder.cs deleted file mode 100644 index 5c24853f32..0000000000 --- a/src/Common/ArgumentBuilder.cs +++ /dev/null @@ -1,51 +0,0 @@ - -using System; -using System.CommandLine; -using System.Reflection; - -internal static class ArgumentBuilder -{ - private static readonly ConstructorInfo _ctor; - - static ArgumentBuilder() - { - _ctor = typeof(Argument).GetConstructor(new[] { typeof(string) }); - } - - 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); -#else - var ctor = argumentType.GetConstructor(new[] { typeof(string) }); -#endif - - return (Argument)ctor.Invoke(new object[] { name }); - } - - internal static Argument CreateArgument(ParameterInfo argsParam) - { - if (!argsParam.HasDefaultValue) - { - return CreateArgument(argsParam.ParameterType, argsParam.Name); - } - - var argumentType = typeof(Bridge<>).MakeGenericType(argsParam.ParameterType); - - var ctor = argumentType.GetConstructor(new[] { typeof(string), argsParam.ParameterType }); - - return (Argument)ctor.Invoke(new object[] { argsParam.Name, argsParam.DefaultValue }); - } - - private sealed class Bridge : Argument - { - public Bridge(string name, T defaultValue) - : base(name) - { - // this type exists only for an easy T => Func transformation - DefaultValueFactory = (_) => defaultValue; - } - } -} \ No newline at end of file diff --git a/src/Common/OptionBuilder.cs b/src/Common/OptionBuilder.cs deleted file mode 100644 index fa9ca5d232..0000000000 --- a/src/Common/OptionBuilder.cs +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Reflection; - -namespace System.CommandLine.Utility; - -internal static class OptionBuilder -{ - private static readonly ConstructorInfo _ctor; - - static OptionBuilder() - { - _ctor = typeof(Option).GetConstructor(new[] { typeof(string), typeof(string[]) }); - } - - internal static Option CreateOption(string name, Type valueType, string description = null) - { - var optionType = typeof(Option<>).MakeGenericType(valueType); - -#if NET6_0_OR_GREATER - var ctor = (ConstructorInfo)optionType.GetMemberWithSameMetadataDefinitionAs(_ctor); -#else - var ctor = optionType.GetConstructor(new[] { typeof(string), typeof(string[]) }); -#endif - - var option = (Option)ctor.Invoke(new object[] { name, Array.Empty() }); - - option.Description = description; - - return option; - } - - internal static Option CreateOption(string name, Type valueType, string description, Func defaultValueFactory) - { - if (defaultValueFactory == null) - { - return CreateOption(name, valueType, description); - } - - var optionType = typeof(Bridge<>).MakeGenericType(valueType); - - var ctor = optionType.GetConstructor(new[] { typeof(string), typeof(Func), typeof(string) }); - - var option = (Option)ctor.Invoke(new object[] { name, defaultValueFactory, description }); - - return option; - } - - private sealed class Bridge : Option - { - public Bridge(string name, Func defaultValueFactory, string description) - : base(name) - { - // this type exists only for an easy Func => Func transformation - DefaultValueFactory = (_) => (T)defaultValueFactory(); - Description = description; - } - } -} \ No newline at end of file diff --git a/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs b/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs index 454942a873..04d9470b5d 100644 --- a/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs +++ b/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs @@ -4,7 +4,6 @@ using FluentAssertions; using System.Collections; using System.Collections.Generic; -using System.CommandLine.Utility; using System.IO; using System.Linq; using System.Net; @@ -134,14 +133,17 @@ public void Command_Argument_defaults_arity_to_ZeroOrOne_for_nullable_types() command.Arguments.Single().Arity.Should().BeEquivalentTo(ArgumentArity.ZeroOrOne); } - [Theory] - [InlineData(typeof(int[]))] - [InlineData(typeof(IEnumerable))] - [InlineData(typeof(List))] - public void Argument_infers_arity_of_IEnumerable_types_as_OneOrMore(Type type) + public static IEnumerable EnumerableTypes() { - var argument = ArgumentBuilder.CreateArgument(type); + yield return new object[] { new Argument("array") }; + yield return new object[] { new Argument>("enumerable") }; + yield return new object[] { new Argument>("list") }; + } + [Theory] + [MemberData(nameof(EnumerableTypes))] + public void Argument_infers_arity_of_IEnumerable_types_as_OneOrMore(Argument argument) + { argument.Arity.Should().BeEquivalentTo(ArgumentArity.OneOrMore); } @@ -693,34 +695,28 @@ public void Values_can_be_correctly_converted_to_nullable_uint_without_the_parse public void Values_can_be_correctly_converted_to_array_of_int_without_the_parser_specifying_a_custom_converter() => GetValue(new Option("-x"), "-x 1 -x 2 -x 3").Should().BeEquivalentTo(new[] { 1, 2, 3 }); + public static IEnumerable AritiesAndEnumerableTypes() + { + foreach (int minArity in new[] { 0, 1 }) + { + foreach (int maxArity in new[] { 3, 100_000 }) + { + yield return new object[] { minArity, maxArity, new Option("--items") }; + yield return new object[] { minArity, maxArity, new Option>("--items") }; + yield return new object[] { minArity, maxArity, new Option>("--items") }; + yield return new object[] { minArity, maxArity, new Option>("--items") }; + yield return new object[] { minArity, maxArity, new Option>("--items") }; + } + } + } + [Theory] - [InlineData(0, 100_000, typeof(string[]))] - [InlineData(0, 3, typeof(string[]))] - [InlineData(0, 100_000, typeof(IEnumerable))] - [InlineData(0, 3, typeof(IEnumerable))] - [InlineData(0, 100_000, typeof(List))] - [InlineData(0, 3, typeof(List))] - [InlineData(0, 100_000, typeof(IList))] - [InlineData(0, 3, typeof(IList))] - [InlineData(0, 100_000, typeof(ICollection))] - [InlineData(0, 3, typeof(ICollection))] - - [InlineData(1, 100_000, typeof(string[]))] - [InlineData(1, 3, typeof(string[]))] - [InlineData(1, 100_000, typeof(IEnumerable))] - [InlineData(1, 3, typeof(IEnumerable))] - [InlineData(1, 100_000, typeof(List))] - [InlineData(1, 3, typeof(List))] - [InlineData(1, 100_000, typeof(IList))] - [InlineData(1, 3, typeof(IList))] - [InlineData(1, 100_000, typeof(ICollection))] - [InlineData(1, 3, typeof(ICollection))] + [MemberData(nameof(AritiesAndEnumerableTypes))] public void Max_arity_greater_than_1_converts_to_enumerable_types( int minArity, int maxArity, - Type argumentType) + Option option) { - var option = OptionBuilder.CreateOption("--items", valueType: argumentType); option.Arity = new ArgumentArity(minArity, maxArity); var command = new RootCommand @@ -731,7 +727,7 @@ public void Max_arity_greater_than_1_converts_to_enumerable_types( var result = command.Parse("--items one --items two --items three"); result.Errors.Should().BeEmpty(); - result.GetResult(option).GetValueOrDefault().Should().BeAssignableTo(argumentType); + result.GetResult(option).GetValueOrDefault().Should().BeAssignableTo(option.ValueType); } [Fact] diff --git a/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj b/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj index a7695f0dc7..d199ef0a71 100644 --- a/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj +++ b/src/System.CommandLine.Tests/System.CommandLine.Tests.csproj @@ -13,8 +13,6 @@ - - From 6b01d2716fa7731159a31523d889d5bda6a6a9df Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 13 Aug 2025 13:26:35 +0200 Subject: [PATCH 2/2] move the logic responsible for getting executable version from RootCommand to VersionOption: - make it clear when Reflection is used - don't load and compile Assembly type when RootCommand is used for the first time --- src/System.CommandLine/RootCommand.cs | 24 ------------------------ src/System.CommandLine/VersionOption.cs | 19 ++++++++++++++++++- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/System.CommandLine/RootCommand.cs b/src/System.CommandLine/RootCommand.cs index 4bbae711fd..2c73b04b0c 100644 --- a/src/System.CommandLine/RootCommand.cs +++ b/src/System.CommandLine/RootCommand.cs @@ -5,7 +5,6 @@ using System.CommandLine.Completions; using System.CommandLine.Help; using System.IO; -using System.Reflection; namespace System.CommandLine { @@ -19,10 +18,8 @@ namespace System.CommandLine /// public class RootCommand : Command { - private static Assembly? _assembly; private static string? _executablePath; private static string? _executableName; - private static string? _executableVersion; /// The description of the command, shown in help. public RootCommand(string description = "") : base(ExecutableName, description) @@ -45,9 +42,6 @@ public RootCommand(string description = "") : base(ExecutableName, description) /// public void Add(Directive directive) => Directives.Add(directive); - internal static Assembly GetAssembly() - => _assembly ??= (Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly()); - /// /// The name of the currently running executable. /// @@ -58,23 +52,5 @@ public static string ExecutableName /// The path to the currently running executable. /// public static string ExecutablePath => _executablePath ??= Environment.GetCommandLineArgs()[0]; - - internal static string ExecutableVersion => _executableVersion ??= GetExecutableVersion(); - - private static string GetExecutableVersion() - { - var assembly = GetAssembly(); - - var assemblyVersionAttribute = assembly.GetCustomAttribute(); - - if (assemblyVersionAttribute is null) - { - return assembly.GetName().Version?.ToString() ?? ""; - } - else - { - return assemblyVersionAttribute.InformationalVersion; - } - } } } diff --git a/src/System.CommandLine/VersionOption.cs b/src/System.CommandLine/VersionOption.cs index 2aaef04e8a..dfcf51f86a 100644 --- a/src/System.CommandLine/VersionOption.cs +++ b/src/System.CommandLine/VersionOption.cs @@ -4,6 +4,7 @@ using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.Linq; +using System.Reflection; namespace System.CommandLine { @@ -64,11 +65,27 @@ private sealed class VersionOptionAction : SynchronousCommandLineAction { public override int Invoke(ParseResult parseResult) { - parseResult.InvocationConfiguration.Output.WriteLine(RootCommand.ExecutableVersion); + parseResult.InvocationConfiguration.Output.WriteLine(GetExecutableVersion()); return 0; } public override bool ClearsParseErrors => true; + + private static string GetExecutableVersion() + { + var assembly = Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly(); + + var assemblyVersionAttribute = assembly.GetCustomAttribute(); + + if (assemblyVersionAttribute is null) + { + return assembly.GetName().Version?.ToString() ?? ""; + } + else + { + return assemblyVersionAttribute.InformationalVersion; + } + } } } } \ No newline at end of file