introduce ParseResult.GetValue<T>(string name)

[adamsitnik]

It's just a draft, as it contains #2067 and I need to ask some questions before I continue my work

fixes #2052

[adamsitnik]

@Keboo @jonsequitur @KalleOlaviNiemitalo while working on the new API I've reused some of the tests provided for ParseResult.GetValue<T>(Option<T>/Argument<T>) and realized that edge-case handling is not 100% intuitive.

The existing method throws InvalidOperationException when it's impossible to parse:

command-line-api/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs

Lines 914 to 923 in 1e15ab1

	public void When_getting_a_single_value_and_specifying_a_conversion_type_that_is_not_supported_then_it_throws()
	{
	var option = new Option<int>("-x");
	var result = new RootCommand { option }.Parse("-x not-an-int");
	Action getValue = () => result.GetValue(option);
	getValue.Should()
	.Throw<InvalidOperationException>()

but it does not throw for Required Options or Arguments with min arity >= 1. It returns default value:

command-line-api/src/System.CommandLine/Parsing/SymbolResult.cs

Lines 64 to 73 in 1e15ab1

	public T? GetValue<T>(Argument<T> argument)
	{
	if (FindResultFor(argument) is { } result &&
	result.GetValueOrDefault<T>() is { } t)
	{
	return t;
	}
	return (T)ArgumentConverter.GetDefaultValue(argument.ValueType)!;
	}

In my opinion both GetValue overloads should throw in such case (then the method does not need to be named GetValueOrDefault).

But I am not sure about what should happen when there was an error unrelated to given symbol/name. Should we just throw if there were any parse errors?

Please share your thoughts.

[jonsequitur]

The existing method throws InvalidOperationException when it's impossible to parse ... but it does not throw for Required Options or Arguments with min arity >= 1. It returns default value:

This is by design, if I'm understanding correctly. Here's the logic, and please let me know if I'm misunderstanding.

The default value provides a value that can be retrieved via GetValue even when there was no command line input to parse. When this happens, the tokens are effectively implicit, or assumed to exist, so we treat the IsRequired requirement as having been satisfied. The caller of GetValue shouldn't have to care whether the value is from actual parsed input tokens or the fallback default value.

But if the user provided unparseable input (e.g. "hello" for an Option<int>), we treat this as an error and we don't fall back to a default value. The caller of GetValue shouldn't be able to pretend there the user error didn't happen.

But I am not sure about what should happen when there was an error unrelated to given symbol/name. Should we just throw if there were any parse errors?

I don't think we should throw. I think this would make some scenarios unreachable. For example, custom parse delegates can use GetValue for other symbols during parsing. This is very useful for binding custom types and for validating one option/argument against another.