Skip to content

Commit 341d847

Browse files
jonsequiturViktorHofer
jonsequitur
and
ViktorHofer
authored
Fix some help and GetRequiredValue issues (#2601)
* include Microsoft.NET.Test.Sdk when Arcade is disabled * fix #2592, #2582, #2573 * remove dead code * model HelpOption and VersionOption without a generic type parameter * update API baseline * PR suggestions * Fix TFM and update content * Add extern alias workaround --------- Co-authored-by: Viktor Hofer <[email protected]>
1 parent 1d3c226 commit 341d847

29 files changed

+657
-136
lines changed

‎CONTRIBUTING.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
Contributing
22
============
33

4-
Please read [.NET Guidelines](https://github.com/dotnet/runtime/blob/master/CONTRIBUTING.md) for more general information about coding styles, source structure, making pull requests, and more.
4+
Please read [.NET Guidelines](https://github.com/dotnet/runtime/blob/main/CONTRIBUTING.md) for more general information about coding styles, source structure, making pull requests, and more.
55

66
## Developer guide
77

88
This project can be developed on any platform. To get started, follow instructions for your OS.
99

1010
### Prerequisites
1111

12-
This project depends on .NET 7. Before working on the project, check that the [.NET SDK](https://dotnet.microsoft.com/en-us/download) is installed.
12+
This project depends on the .NET 9 SDK. Before working on the project, check that the [.NET SDK](https://dotnet.microsoft.com/en-us/download) is installed.
1313

1414
### Visual Studio
1515

‎Directory.Packages.props‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
<ItemGroup>
1111
<!-- Roslyn dependencies -->
1212
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="4.0.1" />
13+
<!-- Runtime dependencies -->
14+
<PackageVersion Include="Microsoft.Bcl.Memory" Version="9.0.6" />
1315
<!-- external dependencies -->
1416
<PackageVersion Include="ApprovalTests" Version="7.0.0-beta.3" />
1517
<PackageVersion Include="BenchmarkDotNet" Version="0.13.1" />
@@ -20,9 +22,10 @@
2022
</ItemGroup>
2123

2224
<ItemGroup Condition="'$(DisableArcade)' == '1'">
25+
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
2326
<!-- The xunit version should be kept in sync with the one that Arcade promotes -->
2427
<PackageVersion Include="xunit" Version="2.9.2" />
2528
<PackageVersion Include="xunit.runner.visualstudio" Version="3.0.2" />
2629
</ItemGroup>
2730

28-
</Project>
31+
</Project>

‎src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,11 @@
153153
public System.Collections.Generic.IEnumerable<Symbol> Parents { get; }
154154
public System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem> GetCompletions(System.CommandLine.Completions.CompletionContext context)
155155
public System.String ToString()
156-
public class VersionOption : Option<System.Boolean>
156+
public class VersionOption : Option
157157
.ctor()
158158
.ctor(System.String name, System.String[] aliases)
159159
public System.CommandLine.Invocation.CommandLineAction Action { get; set; }
160+
public System.Type ValueType { get; }
160161
System.CommandLine.Completions
161162
public class CompletionContext
162163
public static CompletionContext Empty { get; }
@@ -185,10 +186,11 @@ System.CommandLine.Help
185186
public class HelpAction : System.CommandLine.Invocation.SynchronousCommandLineAction
186187
.ctor()
187188
public System.Int32 Invoke(System.CommandLine.ParseResult parseResult)
188-
public class HelpOption : System.CommandLine.Option<System.Boolean>
189+
public class HelpOption : System.CommandLine.Option
189190
.ctor()
190191
.ctor(System.String name, System.String[] aliases)
191192
public System.CommandLine.Invocation.CommandLineAction Action { get; set; }
193+
public System.Type ValueType { get; }
192194
System.CommandLine.Invocation
193195
public abstract class AsynchronousCommandLineAction : CommandLineAction
194196
public System.Threading.Tasks.Task<System.Int32> InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null)

‎src/System.CommandLine.Tests/ArgumentTests.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using FluentAssertions;
55
using System.Linq;
6+
using FluentAssertions.Execution;
67
using Xunit;
78

89
namespace System.CommandLine.Tests;
@@ -41,6 +42,53 @@ public void When_there_is_no_default_value_then_GetDefaultValue_throws()
4142
.Be("Argument \"the-arg\" does not have a default value");
4243
}
4344

45+
[Fact]
46+
public void GetRequiredValue_does_not_throw_when_help_is_requested_and_DefaultValueFactory_is_set()
47+
{
48+
var argument = new Argument<string>("the-arg")
49+
{
50+
DefaultValueFactory = _ => "default"
51+
};
52+
53+
var result = new RootCommand { argument }.Parse("-h");
54+
55+
using var _ = new AssertionScope();
56+
57+
result.Invoking(r => r.GetRequiredValue(argument)).Should().NotThrow();
58+
result.GetRequiredValue(argument).Should().Be("default");
59+
60+
result.Invoking(r => r.GetRequiredValue<string>("the-arg")).Should().NotThrow();
61+
result.GetRequiredValue<string>("the-arg").Should().Be("default");
62+
63+
result.Errors.Should().BeEmpty();
64+
}
65+
66+
[Fact]
67+
public void When_there_is_no_default_value_then_GetDefaultValue_does_not_throw_for_bool()
68+
{
69+
var argument = new Argument<bool>("the-arg");
70+
71+
argument.GetDefaultValue().Should().Be(false);
72+
}
73+
74+
[Fact]
75+
public void When_there_is_no_default_value_then_GetRequiredValue_does_not_throw_for_bool()
76+
{
77+
var argument = new Argument<bool>("the-arg");
78+
79+
var result = new RootCommand { argument }.Parse("");
80+
81+
using var _ = new AssertionScope();
82+
83+
result.Invoking(r => r.GetRequiredValue(argument)).Should().NotThrow();
84+
result.GetRequiredValue(argument).Should().BeFalse();
85+
86+
result.Invoking(r => r.GetRequiredValue<bool>("the-arg")).Should().NotThrow();
87+
result.GetRequiredValue<bool>("the-arg").Should().BeFalse();
88+
89+
result.Errors.Should().BeEmpty();
90+
}
91+
4492
[Fact]
4593
public void Argument_of_enum_can_limit_enum_members_as_valid_values()
4694
{

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.IO;
99
using System.Linq;
1010
using System.Net;
11+
using FluentAssertions.Execution;
1112
using Xunit;
1213

1314
namespace System.CommandLine.Tests.Binding
@@ -585,6 +586,7 @@ public void Values_can_be_correctly_converted_to_Uri_when_custom_parser_is_provi
585586
[Fact]
586587
public void Options_with_arguments_specified_can_be_correctly_converted_to_bool_without_the_parser_specifying_a_custom_converter()
587588
{
589+
using var _ = new AssertionScope();
588590
GetValue(new Option<bool>("-x"), "-x false").Should().BeFalse();
589591
GetValue(new Option<bool>("-x"), "-x true").Should().BeTrue();
590592
}

‎src/System.CommandLine.Tests/DirectiveTests.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4+
using System.CommandLine.Parsing;
45
using System.Linq;
56
using System.Threading.Tasks;
67
using FluentAssertions;
@@ -22,14 +23,14 @@ public void Directives_should_be_considered_as_unmatched_tokens_when_they_are_no
2223
}
2324

2425
[Fact]
25-
public void Raw_tokens_still_hold_directives()
26+
public void Tokens_still_hold_directives()
2627
{
2728
Directive directive = new ("parse");
2829

2930
ParseResult result = Parse(new Option<bool>("-y"), directive, "[parse] -y");
3031

3132
result.GetResult(directive).Should().NotBeNull();
32-
result.Tokens.Should().Contain(t => t.Value == "[parse]");
33+
result.Tokens.Should().Contain(t => t.Value == "[parse]" && t.Type == TokenType.Directive);
3334
}
3435

3536
[Fact]

‎src/System.CommandLine.Tests/GetValueByNameParserTests.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,4 +368,30 @@ public void Recursive_option_on_parent_command_can_be_looked_up_when_subcommand_
368368

369369
result.GetValue<string>("--opt").Should().Be("hello");
370370
}
371+
372+
[Fact]
373+
public void When_argument_type_is_unknown_then_named_lookup_can_be_used_to_get_value_as_supertype()
374+
{
375+
var command = new RootCommand
376+
{
377+
new Argument<string>("arg")
378+
};
379+
380+
var result = command.Parse("value");
381+
382+
result.GetValue<object>("arg").Should().Be("value");
383+
}
384+
385+
[Fact]
386+
public void When_option_type_is_unknown_then_named_lookup_can_be_used_to_get_value_as_supertype()
387+
{
388+
var command = new RootCommand
389+
{
390+
new Option<string>("-x")
391+
};
392+
393+
var result = command.Parse("-x value");
394+
395+
result.GetValue<object>("-x").Should().Be("value");
396+
}
371397
}

‎src/System.CommandLine.Tests/GlobalOptionTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ public void When_a_required_global_option_is_omitted_it_results_in_an_error()
2525
{
2626
var command = new Command("child");
2727
var rootCommand = new RootCommand { command };
28-
command.SetAction((_) => { });
29-
var requiredOption = new Option<bool>("--i-must-be-set")
28+
command.SetAction(_ => { });
29+
var requiredOption = new Option<string>("--i-must-be-set")
3030
{
3131
Required = true,
3232
Recursive = true
@@ -45,7 +45,7 @@ public void When_a_required_global_option_is_omitted_it_results_in_an_error()
4545
public void When_a_required_global_option_has_multiple_aliases_the_error_message_uses_the_name()
4646
{
4747
var rootCommand = new RootCommand();
48-
var requiredOption = new Option<bool>("-i", "--i-must-be-set")
48+
var requiredOption = new Option<string>("-i", "--i-must-be-set")
4949
{
5050
Required = true,
5151
Recursive = true

‎src/System.CommandLine.Tests/Help/ApprovalTests.Config.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1-
using ApprovalTests.Reporters;
2-
using ApprovalTests.Reporters.TestFrameworks;
1+
// Alias workaround for https://github.com/approvals/ApprovalTests.Net/issues/768
2+
extern alias ApprovalTests;
3+
4+
using ApprovalTests.ApprovalTests.Reporters;
5+
using ApprovalTests.ApprovalTests.Reporters.TestFrameworks;
36

47
// Use globally defined Reporter for ApprovalTests. Please see
58
// https://github.com/approvals/ApprovalTests.Net/blob/master/docs/ApprovalTests/Reporters.md
69

710
[assembly: UseReporter(typeof(FrameworkAssertReporter))]
811

9-
[assembly: ApprovalTests.Namers.UseApprovalSubdirectory("Approvals")]
12+
[assembly: ApprovalTests.ApprovalTests.Namers.UseApprovalSubdirectory("Approvals")]

‎src/System.CommandLine.Tests/Help/HelpBuilderTests.Approval.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4+
// Alias workaround for https://github.com/approvals/ApprovalTests.Net/issues/768
5+
extern alias ApprovalTests;
6+
47
using Xunit;
58
using System.IO;
6-
using ApprovalTests;
7-
using ApprovalTests.Reporters;
9+
using ApprovalTests.ApprovalTests;
10+
using ApprovalTests.ApprovalTests.Reporters;
811

912
namespace System.CommandLine.Tests.Help
1013
{

0 commit comments

Comments
 (0)