Skip to content

Commit ec61e86

Browse files
adamsitnikadamsitnik
authored
Reduce Reflection usage (#2662)
* remove ArgumentBuilder and OptionBuilder that use reflection a lot and are used only by the unit test project * 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
1 parent 0b4618b commit ec61e86

File tree

6 files changed

+45
-169
lines changed

6 files changed

+45
-169
lines changed

‎src/Common/ArgumentBuilder.cs‎

Lines changed: 0 additions & 51 deletions
This file was deleted.

‎src/Common/OptionBuilder.cs‎

Lines changed: 0 additions & 60 deletions
This file was deleted.

‎src/System.CommandLine.Tests/Binding/TypeConversionTests.cs‎

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using FluentAssertions;
55
using System.Collections;
66
using System.Collections.Generic;
7-
using System.CommandLine.Utility;
87
using System.IO;
98
using System.Linq;
109
using System.Net;
@@ -134,14 +133,17 @@ public void Command_Argument_defaults_arity_to_ZeroOrOne_for_nullable_types()
134133
command.Arguments.Single().Arity.Should().BeEquivalentTo(ArgumentArity.ZeroOrOne);
135134
}
136135

137-
[Theory]
138-
[InlineData(typeof(int[]))]
139-
[InlineData(typeof(IEnumerable<int>))]
140-
[InlineData(typeof(List<int>))]
141-
public void Argument_infers_arity_of_IEnumerable_types_as_OneOrMore(Type type)
136+
public static IEnumerable<object[]> EnumerableTypes()
142137
{
143-
var argument = ArgumentBuilder.CreateArgument(type);
138+
yield return new object[] { new Argument<int[]>("array") };
139+
yield return new object[] { new Argument<IEnumerable<int>>("enumerable") };
140+
yield return new object[] { new Argument<List<int>>("list") };
141+
}
144142

143+
[Theory]
144+
[MemberData(nameof(EnumerableTypes))]
145+
public void Argument_infers_arity_of_IEnumerable_types_as_OneOrMore(Argument argument)
146+
{
145147
argument.Arity.Should().BeEquivalentTo(ArgumentArity.OneOrMore);
146148
}
147149

@@ -693,34 +695,28 @@ public void Values_can_be_correctly_converted_to_nullable_uint_without_the_parse
693695
public void Values_can_be_correctly_converted_to_array_of_int_without_the_parser_specifying_a_custom_converter()
694696
=> GetValue(new Option<int[]>("-x"), "-x 1 -x 2 -x 3").Should().BeEquivalentTo(new[] { 1, 2, 3 });
695697

698+
public static IEnumerable<object[]> AritiesAndEnumerableTypes()
699+
{
700+
foreach (int minArity in new[] { 0, 1 })
701+
{
702+
foreach (int maxArity in new[] { 3, 100_000 })
703+
{
704+
yield return new object[] { minArity, maxArity, new Option<string[]>("--items") };
705+
yield return new object[] { minArity, maxArity, new Option<IEnumerable<string>>("--items") };
706+
yield return new object[] { minArity, maxArity, new Option<List<string>>("--items") };
707+
yield return new object[] { minArity, maxArity, new Option<IList<string>>("--items") };
708+
yield return new object[] { minArity, maxArity, new Option<ICollection<string>>("--items") };
709+
}
710+
}
711+
}
712+
696713
[Theory]
697-
[InlineData(0, 100_000, typeof(string[]))]
698-
[InlineData(0, 3, typeof(string[]))]
699-
[InlineData(0, 100_000, typeof(IEnumerable<string>))]
700-
[InlineData(0, 3, typeof(IEnumerable<string>))]
701-
[InlineData(0, 100_000, typeof(List<string>))]
702-
[InlineData(0, 3, typeof(List<string>))]
703-
[InlineData(0, 100_000, typeof(IList<string>))]
704-
[InlineData(0, 3, typeof(IList<string>))]
705-
[InlineData(0, 100_000, typeof(ICollection<string>))]
706-
[InlineData(0, 3, typeof(ICollection<string>))]
707-
708-
[InlineData(1, 100_000, typeof(string[]))]
709-
[InlineData(1, 3, typeof(string[]))]
710-
[InlineData(1, 100_000, typeof(IEnumerable<string>))]
711-
[InlineData(1, 3, typeof(IEnumerable<string>))]
712-
[InlineData(1, 100_000, typeof(List<string>))]
713-
[InlineData(1, 3, typeof(List<string>))]
714-
[InlineData(1, 100_000, typeof(IList<string>))]
715-
[InlineData(1, 3, typeof(IList<string>))]
716-
[InlineData(1, 100_000, typeof(ICollection<string>))]
717-
[InlineData(1, 3, typeof(ICollection<string>))]
714+
[MemberData(nameof(AritiesAndEnumerableTypes))]
718715
public void Max_arity_greater_than_1_converts_to_enumerable_types(
719716
int minArity,
720717
int maxArity,
721-
Type argumentType)
718+
Option option)
722719
{
723-
var option = OptionBuilder.CreateOption("--items", valueType: argumentType);
724720
option.Arity = new ArgumentArity(minArity, maxArity);
725721

726722
var command = new RootCommand
@@ -731,7 +727,7 @@ public void Max_arity_greater_than_1_converts_to_enumerable_types(
731727
var result = command.Parse("--items one --items two --items three");
732728

733729
result.Errors.Should().BeEmpty();
734-
result.GetResult(option).GetValueOrDefault<object>().Should().BeAssignableTo(argumentType);
730+
result.GetResult(option).GetValueOrDefault<object>().Should().BeAssignableTo(option.ValueType);
735731
}
736732

737733
[Fact]

‎src/System.CommandLine.Tests/System.CommandLine.Tests.csproj‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
</ItemGroup>
1414

1515
<ItemGroup>
16-
<Compile Include="..\Common\ArgumentBuilder.cs" Link="Utility\ArgumentBuilder.cs" />
17-
<Compile Include="..\Common\OptionBuilder.cs" Link="Utility\OptionBuilder.cs" />
1816
<Compile Include="..\System.CommandLine.Suggest\DotnetMuxer.cs" Link="Utility\DotnetMuxer.cs" />
1917
<Compile Include="..\System.CommandLine\LocalizationResources.cs" Link="LocalizationResources.cs" />
2018
<Compile Include="..\System.CommandLine\Properties\Resources.Designer.cs" Link="Properties\Resources.Designer.cs" />

‎src/System.CommandLine/RootCommand.cs‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.CommandLine.Completions;
66
using System.CommandLine.Help;
77
using System.IO;
8-
using System.Reflection;
98

109
namespace System.CommandLine
1110
{
@@ -19,10 +18,8 @@ namespace System.CommandLine
1918
/// </remarks>
2019
public class RootCommand : Command
2120
{
22-
private static Assembly? _assembly;
2321
private static string? _executablePath;
2422
private static string? _executableName;
25-
private static string? _executableVersion;
2623

2724
/// <param name="description">The description of the command, shown in help.</param>
2825
public RootCommand(string description = "") : base(ExecutableName, description)
@@ -45,9 +42,6 @@ public RootCommand(string description = "") : base(ExecutableName, description)
4542
/// </summary>
4643
public void Add(Directive directive) => Directives.Add(directive);
4744

48-
internal static Assembly GetAssembly()
49-
=> _assembly ??= (Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly());
50-
5145
/// <summary>
5246
/// The name of the currently running executable.
5347
/// </summary>
@@ -58,23 +52,5 @@ public static string ExecutableName
5852
/// The path to the currently running executable.
5953
/// </summary>
6054
public static string ExecutablePath => _executablePath ??= Environment.GetCommandLineArgs()[0];
61-
62-
internal static string ExecutableVersion => _executableVersion ??= GetExecutableVersion();
63-
64-
private static string GetExecutableVersion()
65-
{
66-
var assembly = GetAssembly();
67-
68-
var assemblyVersionAttribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>();
69-
70-
if (assemblyVersionAttribute is null)
71-
{
72-
return assembly.GetName().Version?.ToString() ?? "";
73-
}
74-
else
75-
{
76-
return assemblyVersionAttribute.InformationalVersion;
77-
}
78-
}
7955
}
8056
}

‎src/System.CommandLine/VersionOption.cs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.CommandLine.Invocation;
55
using System.CommandLine.Parsing;
66
using System.Linq;
7+
using System.Reflection;
78

89
namespace System.CommandLine
910
{
@@ -64,11 +65,27 @@ private sealed class VersionOptionAction : SynchronousCommandLineAction
6465
{
6566
public override int Invoke(ParseResult parseResult)
6667
{
67-
parseResult.InvocationConfiguration.Output.WriteLine(RootCommand.ExecutableVersion);
68+
parseResult.InvocationConfiguration.Output.WriteLine(GetExecutableVersion());
6869
return 0;
6970
}
7071

7172
public override bool ClearsParseErrors => true;
73+
74+
private static string GetExecutableVersion()
75+
{
76+
var assembly = Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly();
77+
78+
var assemblyVersionAttribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>();
79+
80+
if (assemblyVersionAttribute is null)
81+
{
82+
return assembly.GetName().Version?.ToString() ?? "";
83+
}
84+
else
85+
{
86+
return assemblyVersionAttribute.InformationalVersion;
87+
}
88+
}
7289
}
7390
}
7491
}

0 commit comments

Comments
 (0)