From f5d7963bdb1af965a6cd257805a1b5004fbc5c18 Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Mon, 9 Feb 2026 11:16:41 +0900 Subject: [PATCH] Fix missing upper-bound guard in LastIndexOf and FindLastIndex Add the missing index < Count validation to LastIndexOf and FindLastIndex on ImmutableList.Node. Previously, passing index == Count did not throw ArgumentOutOfRangeException and instead returned an incorrect (off-by-one) result silently. This aligns the behavior with ImmutableArray. --- .../Immutable/ImmutableList_1.Node.cs | 17 ++++++++++--- .../tests/ImmutableListTest.cs | 25 +++++++++++++++++++ .../tests/ImmutableListTestBase.cs | 20 +++++++++++++++ .../tests/IndexOfTests.cs | 21 ++++++++++++++++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs index 03eae53582c15c..de18332bd01788 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs @@ -801,7 +801,12 @@ internal int IndexOf(T item, int index, int count, IEqualityComparer? equalit /// internal int LastIndexOf(T item, int index, int count, IEqualityComparer? equalityComparer) { - Requires.Range(index >= 0, nameof(index)); + if (index == 0 && count == 0) + { + return -1; + } + + Requires.Range(index >= 0 && index < this.Count, nameof(index)); Requires.Range(count >= 0 && count <= this.Count, nameof(count)); Requires.Argument(index - count + 1 >= 0); @@ -1233,8 +1238,14 @@ internal int FindLastIndex(int startIndex, Predicate match) internal int FindLastIndex(int startIndex, int count, Predicate match) { Requires.NotNull(match, nameof(match)); - Requires.Range(startIndex >= 0, nameof(startIndex)); - Requires.Range(count <= this.Count, nameof(count)); + + if (startIndex == 0 && count == 0) + { + return -1; + } + + Requires.Range(startIndex >= 0 && startIndex < this.Count, nameof(startIndex)); + Requires.Range(count >= 0 && count <= this.Count, nameof(count)); Requires.Range(startIndex - count + 1 >= 0, nameof(startIndex)); using (var enumerator = new Enumerator(this, startIndex: startIndex, count: count, reversed: true)) diff --git a/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs b/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs index 84fb7392466bef..429c6a01e19241 100644 --- a/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs +++ b/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs @@ -522,6 +522,31 @@ public void LastIndexOf() (b, v, i, c, eq) => b.LastIndexOf(v, i, c, eq)); } + [Fact] + public void LastIndexOfMultipleMatches() + { + ImmutableList list = ImmutableList.Create("NonMatch", "Match", "Match", "NonMatch"); + Assert.Equal(2, list.LastIndexOf("Match", index: 3, count: 4, equalityComparer: null)); + Assert.Throws(() => list.LastIndexOf("Match", index: 4, count: 4, equalityComparer: null)); + } + + [Fact] + public void LastIndexOfConsistentWithImmutableArray() + { + ImmutableList list = ImmutableList.Create(1, 2, 3); + ImmutableArray array = ImmutableArray.Create(1, 2, 3); + + Assert.Throws(() => list.LastIndexOf(1, list.Count, 1, equalityComparer: null)); + Assert.Throws(() => array.LastIndexOf(1, array.Length, 1, equalityComparer: null)); + + for (int i = 0; i < list.Count; i++) + { + int listResult = list.LastIndexOf(list[i], list.Count - 1, list.Count, equalityComparer: null); + int arrayResult = array.LastIndexOf(array[i], array.Length - 1, array.Length, equalityComparer: null); + Assert.Equal(arrayResult, listResult); + } + } + [Fact] public void ReplaceTest() { diff --git a/src/libraries/System.Collections.Immutable/tests/ImmutableListTestBase.cs b/src/libraries/System.Collections.Immutable/tests/ImmutableListTestBase.cs index 86640628603a78..e7237c9de74562 100644 --- a/src/libraries/System.Collections.Immutable/tests/ImmutableListTestBase.cs +++ b/src/libraries/System.Collections.Immutable/tests/ImmutableListTestBase.cs @@ -174,6 +174,18 @@ public void FindLastIndexTest() Assert.Equal(-1, this.GetListQuery(ImmutableList.Empty).FindLastIndex(0, n => true)); Assert.Equal(-1, this.GetListQuery(ImmutableList.Empty).FindLastIndex(0, 0, n => true)); + ImmutableList singleElementList = ImmutableList.Create(10); + Assert.Equal(0, this.GetListQuery(singleElementList).FindLastIndex(0, 1, n => n == 10)); + Assert.Equal(-1, this.GetListQuery(singleElementList).FindLastIndex(0, 1, n => n == 99)); + Assert.Throws(() => this.GetListQuery(singleElementList).FindLastIndex(1, n => n == 10)); + Assert.Throws(() => this.GetListQuery(singleElementList).FindLastIndex(1, 1, n => n == 10)); + + ImmutableList multipleElementList = ImmutableList.Create(1, 2, 3, 4); + Assert.Equal(2, this.GetListQuery(multipleElementList).FindLastIndex(3, 4, n => n == 3)); + Assert.Throws(() => this.GetListQuery(multipleElementList).FindLastIndex(4, n => n == 1)); + Assert.Throws(() => this.GetListQuery(multipleElementList).FindLastIndex(4, 1, n => n == 1)); + Assert.Throws(() => this.GetListQuery(multipleElementList).FindLastIndex(4, 4, n => n == 1)); + // Create a list with contents: 100,101,102,103,104,100,101,102,103,104 ImmutableList list = ImmutableList.Empty.AddRange(Enumerable.Range(100, 5).Concat(Enumerable.Range(100, 5))); List bclList = list.ToList(); @@ -221,6 +233,14 @@ public void FindLastIndexTest() } } + [Fact] + public void FindLastIndexTestMultipleMatches() + { + ImmutableList list = ImmutableList.Create("NonMatch", "Match", "Match", "NonMatch"); + Assert.Equal(2, this.GetListQuery(list).FindLastIndex(3, 4, n => n == "Match")); + Assert.Throws(() => this.GetListQuery(list).FindLastIndex(4, 4, n => n == "Match")); + } + [Fact] public void IList_IndexOf_NullArgument() { diff --git a/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs b/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs index a9597abd53e97d..ed15b40f361637 100644 --- a/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs +++ b/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs @@ -88,23 +88,44 @@ public static void LastIndexOfTest( Func, int> lastIndexOfItemIndexCountEQ) { TCollection emptyCollection = factory(new int[0]); + TCollection singleCollection = factory(new[] { 10 }); TCollection collection1256 = factory(new[] { 1, 2, 5, 6 }); Assert.Throws(() => lastIndexOfItemIndexCountEQ(emptyCollection, 100, 1, 1, EqualityComparer.Default)); Assert.Throws(() => lastIndexOfItemIndexCountEQ(emptyCollection, 100, -1, 1, EqualityComparer.Default)); + Assert.Throws(() => lastIndexOfItemIndexCountEQ(singleCollection, 100, 1, 1, EqualityComparer.Default)); + Assert.Throws(() => lastIndexOfItemIndexCountEQ(singleCollection, 100, -1, 1, EqualityComparer.Default)); Assert.Throws(() => lastIndexOfItemIndexCountEQ(collection1256, 100, 1, 20, EqualityComparer.Default)); Assert.Throws(() => lastIndexOfItemIndexCountEQ(collection1256, 100, 1, -1, EqualityComparer.Default)); Assert.Throws(() => lastIndexOfItemIndexCountEQ(emptyCollection, 100, 1, 1, new CustomComparer(50))); Assert.Throws(() => lastIndexOfItemIndexCountEQ(emptyCollection, 100, -1, 1, new CustomComparer(50))); + Assert.Throws(() => lastIndexOfItemIndexCountEQ(singleCollection, 100, 1, 1, new CustomComparer(50))); + Assert.Throws(() => lastIndexOfItemIndexCountEQ(singleCollection, 100, -1, 1, new CustomComparer(50))); Assert.Throws(() => lastIndexOfItemIndexCountEQ(collection1256, 100, 1, 20, new CustomComparer(1))); Assert.Throws(() => lastIndexOfItemIndexCountEQ(collection1256, 100, 1, -1, new CustomComparer(1))); Assert.Throws(() => lastIndexOfItemIndex(collection1256, 2, 5)); + Assert.Throws(() => lastIndexOfItemIndexCountEQ(collection1256, 6, 4, 4, EqualityComparer.Default)); Assert.Equal(-1, lastIndexOfItem(emptyCollection, 5)); Assert.Equal(-1, lastIndexOfItemEQ(emptyCollection, 5, EqualityComparer.Default)); Assert.Equal(-1, lastIndexOfItemIndex(emptyCollection, 5, 0)); Assert.Equal(-1, lastIndexOfItemIndexCount(emptyCollection, 5, 0, 0)); + Assert.Equal(0, lastIndexOfItem(singleCollection, 10)); + Assert.Equal(0, lastIndexOfItemEQ(singleCollection, 10, EqualityComparer.Default)); + Assert.Equal(0, lastIndexOfItemIndex(singleCollection, 10, 0)); + Assert.Equal(0, lastIndexOfItemIndexCount(singleCollection, 10, 0, 1)); + Assert.Equal(0, lastIndexOfItemIndexCountEQ(singleCollection, 10, 0, 1, EqualityComparer.Default)); + Assert.Equal(-1, lastIndexOfItemIndexCountEQ(singleCollection, 100, 0, 1, EqualityComparer.Default)); + + Assert.Equal(1, lastIndexOfItem(collection1256, 2)); + Assert.Equal(1, lastIndexOfItemEQ(collection1256, 2, EqualityComparer.Default)); + Assert.Equal(1, lastIndexOfItemIndex(collection1256, 2, 3)); + Assert.Equal(1, lastIndexOfItemIndexCount(collection1256, 2, 3, 4)); + Assert.Equal(2, lastIndexOfItemIndexCountEQ(collection1256, 5, 3, 4, EqualityComparer.Default)); + Assert.Equal(3, lastIndexOfItemIndexCountEQ(collection1256, 6, 3, 4, EqualityComparer.Default)); + Assert.Equal(-1, lastIndexOfItemIndexCountEQ(collection1256, 100, 3, 4, EqualityComparer.Default)); + // Create a list with contents: 100,101,102,103,104,100,101,102,103,104 ImmutableList list = ImmutableList.Empty.AddRange(Enumerable.Range(100, 5).Concat(Enumerable.Range(100, 5))); List bclList = list.ToList();