From 9d3fdf4cd3153a578df608e9b906b8514ce57799 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 11:35:26 +0000 Subject: [PATCH] [patch] Check for nulls during the one enumeration in Join and ToStringEnumerable In Throw mode both methods ran AnyNull() over the source and then enumerated it again, so a one-shot sequence joined to "", selectors ran twice, and a null added after ToStringEnumerable(Throw) returned slipped through. Each item is now checked as it is projected. Fixes #138 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GUKjHWJy9iwpdVJ1NKRU1M --- .gitignore | 18 ++++++ Extensions.Test/EnumerableExtensionsTests.cs | 63 ++++++++++++++++++++ Extensions/EnumerableExtensions.cs | 39 ++++++------ 3 files changed, 101 insertions(+), 19 deletions(-) diff --git a/.gitignore b/.gitignore index dc0470a..e043c9f 100644 --- a/.gitignore +++ b/.gitignore @@ -203,6 +203,11 @@ PublishScripts/ **/[Pp]ackages/* # except build/, which is used as an MSBuild target. !**/[Pp]ackages/build/ +# and except a Unity project's Packages/, which is source: Unity's package manifest and its +# resolved lock file are both meant to be committed, and a NuGet restore folder never contains +# a file by either name. +!**/[Pp]ackages/manifest.json +!**/[Pp]ackages/packages-lock.json # Uncomment if necessary however generally it will be regenerated when needed #!**/[Pp]ackages/repositories.config # NuGet v3's project.json files produces more ignorable files @@ -651,3 +656,16 @@ Temporary Items # ImGui.ini files imgui.ini + +# Game engine projects +# +# Godot: the import cache, and the mono/temp bin+obj a C# build writes. +.godot/ + +# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule +# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs +# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently +# breaks those references - including for a plug-in whose .dll is itself a build output. This +# negation has to come after that rule to win, and is scoped to the asset tree so the Visual +# Studio artifact stays ignored everywhere else. +!**/[Aa]ssets/**/*.meta diff --git a/Extensions.Test/EnumerableExtensionsTests.cs b/Extensions.Test/EnumerableExtensionsTests.cs index 6e186ce..8feac9d 100644 --- a/Extensions.Test/EnumerableExtensionsTests.cs +++ b/Extensions.Test/EnumerableExtensionsTests.cs @@ -4,6 +4,9 @@ namespace ktsu.Extensions.Tests; +using System.Collections.Concurrent; +using System.Globalization; + [TestClass] public class EnumerableExtensionsTests { @@ -393,4 +396,64 @@ public void JoinWithNullItemHandlingThrowThrowsInvalidOperationException() // Act & Assert Assert.ThrowsExactly(() => items.Join(separator, NullItemHandling.Throw)); } + + [TestMethod] + public void JoinWithNullItemHandlingThrowReadsAOneShotSequence() + { + using BlockingCollection items = ["x", "y"]; + items.CompleteAdding(); + + Assert.AreEqual("x,y", items.GetConsumingEnumerable().Join(",", NullItemHandling.Throw)); + } + + [TestMethod] + public void JoinWithNullItemHandlingThrowEnumeratesTheSourceOnce() + { + int calls = 0; + IEnumerable items = Enumerable.Range(0, 3).Select(i => + { + calls++; + return i.ToString(CultureInfo.InvariantCulture); + }); + + Assert.AreEqual("0,1,2", items.Join(",", NullItemHandling.Throw)); + Assert.AreEqual(3, calls); + } + + [TestMethod] + public void ToStringEnumerableWithNullItemHandlingThrowReadsAOneShotSequence() + { + using BlockingCollection items = ["x", "y"]; + items.CompleteAdding(); + + CollectionAssert.AreEqual( + new List { "x", "y" }, + items.GetConsumingEnumerable().ToStringEnumerable(NullItemHandling.Throw).ToList()); + } + + [TestMethod] + public void ToStringEnumerableWithNullItemHandlingThrowEnumeratesTheSourceOnce() + { + int calls = 0; + IEnumerable items = Enumerable.Range(0, 3).Select(i => + { + calls++; + return i; + }); + + List result = [.. items.ToStringEnumerable(NullItemHandling.Throw)]; + + Assert.HasCount(3, result); + Assert.AreEqual(3, calls); + } + + [TestMethod] + public void ToStringEnumerableWithNullItemHandlingThrowCatchesANullAddedBeforeEnumeration() + { + List items = ["a", "b"]; + IEnumerable result = items.ToStringEnumerable(NullItemHandling.Throw); + items.Add(null); + + Assert.ThrowsExactly(() => result.ToList()); + } } diff --git a/Extensions/EnumerableExtensions.cs b/Extensions/EnumerableExtensions.cs index 1e5c637..6e34cb6 100644 --- a/Extensions/EnumerableExtensions.cs +++ b/Extensions/EnumerableExtensions.cs @@ -191,7 +191,7 @@ public static IEnumerable ToStringEnumerable(this IEnumerable item /// The enumerable to convert. /// Specifies how to handle null items. /// An enumerable of strings. - /// Thrown if is set to and the enumerable contains null items. + /// Thrown while enumerating the result if is set to and the enumerable contains null items. public static IEnumerable ToStringEnumerable(this IEnumerable items, NullItemHandling nullItemHandling) { #pragma warning disable KTSU0004 // Use Ensure.NotNull instead of manual null check @@ -201,16 +201,10 @@ public static IEnumerable ToStringEnumerable(this IEnumerable item } #pragma warning restore KTSU0004 // Use Ensure.NotNull instead of manual null check - if (nullItemHandling is NullItemHandling.Throw) - { - if (items.AnyNull()) - { - throw new InvalidOperationException("The enumerable contains a null item."); - } - } - + // The argument check above stays eager; the null check happens during the one enumeration the + // caller makes, so a single-pass source is not consumed early and a null added later is caught. return items - .Select(item => item?.ToString()) + .Select(item => ThrowIfNullItem(item, nullItemHandling)?.ToString()) .Where(item => nullItemHandling is NullItemHandling.Include || item is not null); } @@ -266,14 +260,21 @@ public static string Join(this IEnumerable items, string separator, NullIt } #pragma warning restore KTSU0004 // Use Ensure.NotNull instead of manual null check - if (nullItemHandling is NullItemHandling.Throw) - { - if (items.AnyNull()) - { - throw new InvalidOperationException("The enumerable contains a null item."); - } - } - - return string.Join(separator, items.Where(item => nullItemHandling is NullItemHandling.Include || item is not null).Select(i => i?.ToString())); + return string.Join(separator, items + .Select(item => ThrowIfNullItem(item, nullItemHandling)) + .Where(item => nullItemHandling is NullItemHandling.Include || item is not null) + .Select(i => i?.ToString())); } + + /// + /// Returns unchanged, or throws if it is null and is . + /// + /// + /// Checking each item as it is projected keeps the source to a single enumeration, unlike a separate + /// pass, which consumes a one-shot sequence before it can be read. + /// + private static T ThrowIfNullItem(T item, NullItemHandling nullItemHandling) => + item is null && nullItemHandling is NullItemHandling.Throw + ? throw new InvalidOperationException("The enumerable contains a null item.") + : item; }