Return null from StartsWithValueConverter on empty input - #196
Merged
TylerBrinks merged 1 commit intoJul 23, 2026
Merged
Conversation
Transform advances past leading whitespace with
while (enumerator.MoveNext() && enumerator.Current.Type == ...)
and then reads enumerator.Current without checking whether MoveNext had
actually returned false. On an empty or whitespace-only sequence the
enumerator is already exhausted, so Current either is null or throws:
Converters.IntegerConverter.StartsWithDelimiter().ConvertDefault();
// NullReferenceException
converter.Convert(new[] { Token.Whitespace });
// InvalidOperationException: Enumeration already finished.
Empty input is not exotic here - ConvertDefault() passes
Enumerable.Empty<Token>() by definition, and VaryStart falls back to it
once the token list is exhausted. Every other converter answers "no
match" for it. Capture the MoveNext result and do the same.
jhaygood86
marked this pull request as ready for review
July 22, 2026 21:30
jhaygood86
marked this pull request as draft
July 22, 2026 21:40
jhaygood86
marked this pull request as ready for review
July 22, 2026 21:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
StartsWithValueConverter.Transformskips leading whitespace and then readsCurrentwithout checking whether the enumerator was exhausted:When the sequence is empty or whitespace-only, the loop exits because
MoveNext()returnedfalse, andCurrentis then eithernullor throws depending on the enumerator implementation:Empty input isn't an exotic case here:
ConvertDefault()passesEnumerable.Empty<Token>()by definition, andVaryStartfalls back toConvertDefault()once its token list is exhausted. Every other converter treats empty input as "no match" —DictionaryValueConverter, for instance, getsnullback fromToIdentifier()and returnsnull.Fix
Capture the
MoveNext()result and returnnullwhen the sequence was exhausted.This also unblocks composing
StartsWithValueConverterinto converters that get probed with empty input — for example afont-style: oblique <angle>converter, which is where I hit it.Tests
5 tests in
Property.cs:ConvertDefault()on aStartsWithDelimiterconverter returns null (currentlyNullReferenceException)InvalidOperationException)RatioConverter— the one composition that putsStartsWithDelimiterbehind an ordered converter — parsing incompleteaspect-ratiomedia featuresThe first 2 fail on
master; the 3 guards pass either way and are there to keep the real-world path covered. The full suite (1263 existing tests) stays green, and all seven target frameworks build with no new warnings.