Skip to content

Commit 00c331e

Browse files
natemcmastergithub-actions[bot]claude
authored
fix: resolve nullable type lookup bypassing custom value parsers (#608)
* fix: resolve nullable type lookup bypassing custom value parsers Move the nullable type unwrap check before the TypeDescriptor fallback in GetParserImpl<T>(). Previously, when a custom parser was registered via AddOrReplace for a type like TimeSpan, looking up TimeSpan? would miss it because the TypeDescriptor fallback (which returns the built-in converter) ran before the nullable unwrap that checks the dictionary under the unwrapped type. Fixes #559 Co-authored-by: Nate McMaster <natemcmaster@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add release notes for nullable value parser fix (#559) Co-authored-by: Nate McMaster <natemcmaster@users.noreply.github.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Nate McMaster <natemcmaster@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 33b59d7 commit 00c331e

File tree

4 files changed

+74
-5
lines changed

4 files changed

+74
-5
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77

88
### Fixes
99
* [@claude]: Validate dotnet path exists before returning from `TryFindDotNetExePath` ([#600])
10+
* [@claude]: Fix nullable type lookup bypassing custom value parsers registered via `AddOrReplace` ([#559])
1011

12+
[#559]: https://github.com/natemcmaster/CommandLineUtils/issues/559
1113
[#560]: https://github.com/natemcmaster/CommandLineUtils/pull/560
1214
[#600]: https://github.com/natemcmaster/CommandLineUtils/issues/600
1315

src/CommandLineUtils/Abstractions/ValueParserProvider.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,6 @@ public IValueParser GetParser(Type type)
100100
return EnumParser.Create(type);
101101
}
102102

103-
if (_defaultValueParserFactory.TryGetParser<T>(out parser))
104-
{
105-
return parser;
106-
}
107-
108103
if (ReflectionHelper.IsNullableType(type, out var wrappedType))
109104
{
110105
if (wrappedType.IsEnum)
@@ -118,6 +113,11 @@ public IValueParser GetParser(Type type)
118113
}
119114
}
120115

116+
if (_defaultValueParserFactory.TryGetParser<T>(out parser))
117+
{
118+
return parser;
119+
}
120+
121121
if (ReflectionHelper.IsSpecialValueTupleType(type, out var wrappedType2))
122122
{
123123
var innerParser = GetParser(wrappedType2);

src/CommandLineUtils/releasenotes.props

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Features:
88

99
Fixes:
1010
* @claude: Validate dotnet path exists before returning from TryFindDotNetExePath (#600)
11+
* @claude: Fix nullable type lookup bypassing custom value parsers registered via AddOrReplace (#559)
1112
</PackageReleaseNotes>
1213
<PackageReleaseNotes Condition="$(VersionPrefix.StartsWith('5.0.'))">
1314
Changes since 4.1:

test/CommandLineUtils.Tests/ValueParserProviderCustomTests.cs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,5 +339,71 @@ public void AddOrReplaceThrowsIfNullparser()
339339

340340
Assert.Contains("parser", ex.Message);
341341
}
342+
343+
private class CustomTimeSpanParser : IValueParser<TimeSpan>
344+
{
345+
public Type TargetType => typeof(TimeSpan);
346+
347+
public TimeSpan Parse(string? argName, string? value, CultureInfo culture)
348+
{
349+
if (value != null && value.EndsWith("s"))
350+
{
351+
var seconds = int.Parse(value.Substring(0, value.Length - 1), culture);
352+
return TimeSpan.FromSeconds(seconds);
353+
}
354+
355+
return TimeSpan.Parse(value!, culture);
356+
}
357+
358+
object? IValueParser.Parse(string? argName, string? value, CultureInfo culture)
359+
=> Parse(argName, value, culture);
360+
}
361+
362+
private class NullableTimeSpanOptionProgram
363+
{
364+
[Option("--timeout", CommandOptionType.SingleValue)]
365+
public TimeSpan? Timeout { get; set; }
366+
}
367+
368+
private class NonNullableTimeSpanOptionProgram
369+
{
370+
[Option("--timeout", CommandOptionType.SingleValue)]
371+
public TimeSpan Timeout { get; set; }
372+
}
373+
374+
[Fact]
375+
public void CustomParserWorksForNullableBuiltInType()
376+
{
377+
var app = new CommandLineApplication<NullableTimeSpanOptionProgram>();
378+
app.ValueParsers.AddOrReplace(new CustomTimeSpanParser());
379+
app.Conventions.UseDefaultConventions();
380+
381+
app.Parse("--timeout", "15s");
382+
383+
Assert.Equal(TimeSpan.FromSeconds(15), app.Model.Timeout);
384+
}
385+
386+
[Fact]
387+
public void CustomParserWorksForNonNullableBuiltInType()
388+
{
389+
var app = new CommandLineApplication<NonNullableTimeSpanOptionProgram>();
390+
app.ValueParsers.AddOrReplace(new CustomTimeSpanParser());
391+
app.Conventions.UseDefaultConventions();
392+
393+
app.Parse("--timeout", "15s");
394+
395+
Assert.Equal(TimeSpan.FromSeconds(15), app.Model.Timeout);
396+
}
397+
398+
[Fact]
399+
public void NullableBuiltInTypeUsesDefaultParserWhenNoCustomParser()
400+
{
401+
var app = new CommandLineApplication<NullableTimeSpanOptionProgram>();
402+
app.Conventions.UseDefaultConventions();
403+
404+
app.Parse("--timeout", "00:00:15");
405+
406+
Assert.Equal(TimeSpan.FromSeconds(15), app.Model.Timeout);
407+
}
342408
}
343409
}

0 commit comments

Comments
 (0)