From a338be493c5ee427faee94c113062e190283553d Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Fri, 27 Feb 2026 22:44:49 +0900 Subject: [PATCH] Fix IndexOf upper-bound guard in ImmutableArray/List Change the startIndex upper-bound validation in IndexOf from '< Count' to '<= Count' for ImmutableArray, ImmutableArray.Builder, and add upper-bound validation in ImmutableList.Node. This aligns the behavior with Array.IndexOf, which permits index == Count provided count is 0. --- .../Immutable/ImmutableArray_1.Builder.cs | 2 +- .../Collections/Immutable/ImmutableArray_1.cs | 2 +- .../Immutable/ImmutableList_1.Node.cs | 10 +++-- .../tests/ImmutableArrayBuilderTest.cs | 20 ++++++++- .../tests/ImmutableArrayTest.cs | 20 ++++++++- .../tests/ImmutableListBuilderTest.cs | 3 +- .../tests/ImmutableListTest.cs | 23 +++++++++- .../tests/IndexOfTests.cs | 42 ++++++++++++++----- 8 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.Builder.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.Builder.cs index 9f596e8f7578a7..0012e1b5cf4d22 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.Builder.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.Builder.cs @@ -796,7 +796,7 @@ public int IndexOf(T item, int startIndex, int count, IEqualityComparer? equa return -1; } - Requires.Range(startIndex >= 0 && startIndex < this.Count, nameof(startIndex)); + Requires.Range(startIndex >= 0 && startIndex <= this.Count, nameof(startIndex)); Requires.Range(count >= 0 && startIndex + count <= this.Count, nameof(count)); equalityComparer ??= EqualityComparer.Default; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs index ec4d94dc62184e..e85d849ff1b5fd 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs @@ -173,7 +173,7 @@ public int IndexOf(T item, int startIndex, int count, IEqualityComparer? equa return -1; } - Requires.Range(startIndex >= 0 && startIndex < self.Length, nameof(startIndex)); + Requires.Range(startIndex >= 0 && startIndex <= self.Length, nameof(startIndex)); Requires.Range(count >= 0 && startIndex + count <= self.Length, nameof(count)); equalityComparer ??= EqualityComparer.Default; 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 973b03c1aab387..f7f0507b4077b0 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 @@ -759,9 +759,13 @@ internal int BinarySearch(int index, int count, T item, IComparer? comparer) /// internal int IndexOf(T item, int index, int count, IEqualityComparer? equalityComparer) { - Requires.Range(index >= 0, nameof(index)); - Requires.Range(count >= 0, nameof(count)); - Requires.Range(count <= this.Count, nameof(count)); + if (count == 0 && index == 0) + { + return -1; + } + + Requires.Range(index >= 0 && index <= this.Count, nameof(index)); + Requires.Range(count >= 0 && count <= this.Count, nameof(count)); Requires.Range(index + count <= this.Count, nameof(count)); equalityComparer ??= EqualityComparer.Default; diff --git a/src/libraries/System.Collections.Immutable/tests/ImmutableArrayBuilderTest.cs b/src/libraries/System.Collections.Immutable/tests/ImmutableArrayBuilderTest.cs index 841bfc451e332d..58da1330fd8701 100644 --- a/src/libraries/System.Collections.Immutable/tests/ImmutableArrayBuilderTest.cs +++ b/src/libraries/System.Collections.Immutable/tests/ImmutableArrayBuilderTest.cs @@ -255,7 +255,8 @@ public void IndexOf() (b, v) => b.IndexOf(v), (b, v, i) => b.IndexOf(v, i), (b, v, i, c) => b.IndexOf(v, i, c), - (b, v, i, c, eq) => b.IndexOf(v, i, c, eq)); + (b, v, i, c, eq) => b.IndexOf(v, i, c, eq), + "startIndex"); } [Fact] @@ -268,6 +269,23 @@ public void IndexOf_WithoutCountParam() Assert.Equal(-1, builder.IndexOf(-5, 2, absComparer)); } + [Fact] + public void IndexOfConsistentWithArray() + { + ImmutableArray.Builder builder = ImmutableArray.Create(1, 2, 3, 4).ToBuilder(); + int[] array = new[] { 1, 2, 3, 4 }; + + Assert.Equal(-1, builder.IndexOf(2, builder.Count, 0)); + Assert.Equal(-1, Array.IndexOf(array, 2, array.Length, 0)); + + for (int i = 0; i < array.Length; i++) + { + int builderResult = builder.IndexOf(builder[i], 0, builder.Count); + int arrayResult = Array.IndexOf(array, array[i], 0, array.Length); + Assert.Equal(builderResult, arrayResult); + } + } + [Fact] public void LastIndexOf() { diff --git a/src/libraries/System.Collections.Immutable/tests/ImmutableArrayTest.cs b/src/libraries/System.Collections.Immutable/tests/ImmutableArrayTest.cs index 80a3ec1a37e6f0..6787c977ed55d4 100644 --- a/src/libraries/System.Collections.Immutable/tests/ImmutableArrayTest.cs +++ b/src/libraries/System.Collections.Immutable/tests/ImmutableArrayTest.cs @@ -790,7 +790,25 @@ public void IndexOf() (b, v) => b.IndexOf(v), (b, v, i) => b.IndexOf(v, i), (b, v, i, c) => b.IndexOf(v, i, c), - (b, v, i, c, eq) => b.IndexOf(v, i, c, eq)); + (b, v, i, c, eq) => b.IndexOf(v, i, c, eq), + "startIndex"); + } + + [Fact] + public void IndexOfConsistentWithArray() + { + ImmutableArray immutableArray = ImmutableArray.Create(1, 2, 3, 4); + int[] array = new[] { 1, 2, 3, 4 }; + + Assert.Equal(-1, immutableArray.IndexOf(2, immutableArray.Length, 0)); + Assert.Equal(-1, Array.IndexOf(array, 2, array.Length, 0)); + + for (int i = 0; i < array.Length; i++) + { + int immutableArrayResult = immutableArray.IndexOf(immutableArray[i], 0, immutableArray.Length); + int arrayResult = Array.IndexOf(array, array[i], 0, array.Length); + Assert.Equal(immutableArrayResult, arrayResult); + } } [Fact] diff --git a/src/libraries/System.Collections.Immutable/tests/ImmutableListBuilderTest.cs b/src/libraries/System.Collections.Immutable/tests/ImmutableListBuilderTest.cs index 741dfbd085cde4..7d2b9fe483bf4a 100644 --- a/src/libraries/System.Collections.Immutable/tests/ImmutableListBuilderTest.cs +++ b/src/libraries/System.Collections.Immutable/tests/ImmutableListBuilderTest.cs @@ -333,7 +333,8 @@ public void IndexOf() (b, v) => b.IndexOf(v), (b, v, i) => b.IndexOf(v, i), (b, v, i, c) => b.IndexOf(v, i, c), - (b, v, i, c, eq) => b.IndexOf(v, i, c, eq)); + (b, v, i, c, eq) => b.IndexOf(v, i, c, eq), + "index"); } [Fact] diff --git a/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs b/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs index 429c6a01e19241..cc6fe537864641 100644 --- a/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs +++ b/src/libraries/System.Collections.Immutable/tests/ImmutableListTest.cs @@ -494,13 +494,32 @@ public void IndexOf() (b, v) => b.IndexOf(v), (b, v, i) => b.IndexOf(v, i), (b, v, i, c) => b.IndexOf(v, i, c), - (b, v, i, c, eq) => b.IndexOf(v, i, c, eq)); + (b, v, i, c, eq) => b.IndexOf(v, i, c, eq), + "index"); IndexOfTests.IndexOfTest( seq => (IImmutableList)ImmutableList.CreateRange(seq), (b, v) => b.IndexOf(v), (b, v, i) => b.IndexOf(v, i), (b, v, i, c) => b.IndexOf(v, i, c), - (b, v, i, c, eq) => b.IndexOf(v, i, c, eq)); + (b, v, i, c, eq) => b.IndexOf(v, i, c, eq), + "index"); + } + + [Fact] + public void IndexOfConsistentWithArray() + { + ImmutableList list = ImmutableList.Create(1, 2, 3, 4); + int[] array = new[] { 1, 2, 3, 4 }; + + Assert.Equal(-1, list.IndexOf(2, list.Count, 0)); + Assert.Equal(-1, Array.IndexOf(array, 2, array.Length, 0)); + + for (int i = 0; i < array.Length; i++) + { + int listResult = list.IndexOf(list[i], 0, list.Count); + int arrayResult = Array.IndexOf(array, array[i], 0, array.Length); + Assert.Equal(listResult, arrayResult); + } } [Fact] diff --git a/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs b/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs index b594bab93d042c..2ae643686b93b9 100644 --- a/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs +++ b/src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs @@ -14,26 +14,48 @@ public static void IndexOfTest( Func indexOfItem, Func indexOfItemIndex, Func indexOfItemIndexCount, - Func, int> indexOfItemIndexCountEQ) + Func, int> indexOfItemIndexCountEQ, + string indexParameterName) { TCollection emptyCollection = factory(new int[0]); + TCollection singleCollection = factory(new[] { 10 }); TCollection collection1256 = factory(new[] { 1, 2, 5, 6 }); - Assert.Throws(() => indexOfItemIndexCountEQ(emptyCollection, 100, 1, 1, EqualityComparer.Default)); - Assert.Throws(() => indexOfItemIndexCountEQ(emptyCollection, 100, -1, 1, EqualityComparer.Default)); - Assert.Throws(() => indexOfItemIndexCountEQ(collection1256, 100, 1, 20, EqualityComparer.Default)); - Assert.Throws(() => indexOfItemIndexCountEQ(collection1256, 100, 1, -1, EqualityComparer.Default)); - Assert.Throws(() => indexOfItemIndexCountEQ(emptyCollection, 100, 1, 1, new CustomComparer(50))); - Assert.Throws(() => indexOfItemIndexCountEQ(emptyCollection, 100, -1, 1, new CustomComparer(50))); - Assert.Throws(() => indexOfItemIndexCountEQ(collection1256, 100, 1, 20, new CustomComparer(1))); - Assert.Throws(() => indexOfItemIndexCountEQ(collection1256, 100, 1, -1, new CustomComparer(1))); + AssertExtensions.Throws(indexParameterName, () => indexOfItemIndexCountEQ(emptyCollection, 100, 1, 1, EqualityComparer.Default)); + AssertExtensions.Throws(indexParameterName, () => indexOfItemIndexCountEQ(emptyCollection, 100, -1, 1, EqualityComparer.Default)); + AssertExtensions.Throws("count", () => indexOfItemIndexCountEQ(singleCollection, 100, 1, 1, EqualityComparer.Default)); + AssertExtensions.Throws(indexParameterName, () => indexOfItemIndexCountEQ(singleCollection, 100, -1, 1, EqualityComparer.Default)); + AssertExtensions.Throws("count", () => indexOfItemIndexCountEQ(collection1256, 100, 1, 20, EqualityComparer.Default)); + AssertExtensions.Throws("count", () => indexOfItemIndexCountEQ(collection1256, 100, 1, -1, EqualityComparer.Default)); + AssertExtensions.Throws(indexParameterName, () => indexOfItemIndexCountEQ(emptyCollection, 100, 1, 1, new CustomComparer(50))); + AssertExtensions.Throws(indexParameterName, () => indexOfItemIndexCountEQ(emptyCollection, 100, -1, 1, new CustomComparer(50))); + AssertExtensions.Throws("count", () => indexOfItemIndexCountEQ(singleCollection, 100, 1, 1, new CustomComparer(50))); + AssertExtensions.Throws(indexParameterName, () => indexOfItemIndexCountEQ(singleCollection, 100, -1, 1, new CustomComparer(50))); + AssertExtensions.Throws("count", () => indexOfItemIndexCountEQ(collection1256, 100, 1, 20, new CustomComparer(1))); + AssertExtensions.Throws("count", () => indexOfItemIndexCountEQ(collection1256, 100, 1, -1, new CustomComparer(1))); + AssertExtensions.Throws(indexParameterName, () => indexOfItemIndex(collection1256, 2, 5)); + AssertExtensions.Throws("count", () => indexOfItemIndexCountEQ(collection1256, 6, 2, 4, EqualityComparer.Default)); Assert.Equal(-1, indexOfItem(emptyCollection, 5)); Assert.Equal(-1, indexOfItemIndex(emptyCollection, 5, 0)); - Assert.Equal(2, indexOfItemIndex(collection1256, 5, 1)); Assert.Equal(-1, indexOfItemIndexCount(emptyCollection, 5, 0, 0)); + Assert.Equal(-1, indexOfItemIndexCountEQ(emptyCollection, 5, 0, 0, EqualityComparer.Default)); + + Assert.Equal(0, indexOfItem(singleCollection, 10)); + Assert.Equal(0, indexOfItemIndex(singleCollection, 10, 0)); + Assert.Equal(0, indexOfItemIndexCount(singleCollection, 10, 0, 1)); + Assert.Equal(0, indexOfItemIndexCountEQ(singleCollection, 10, 0, 1, EqualityComparer.Default)); + Assert.Equal(-1, indexOfItemIndexCountEQ(singleCollection, 100, 0, 1, EqualityComparer.Default)); + Assert.Equal(-1, indexOfItemIndexCountEQ(singleCollection, 10, 1, 0, EqualityComparer.Default)); + + Assert.Equal(2, indexOfItem(collection1256, 5)); + Assert.Equal(2, indexOfItemIndex(collection1256, 5, 1)); Assert.Equal(-1, indexOfItemIndexCount(collection1256, 5, 1, 1)); Assert.Equal(2, indexOfItemIndexCount(collection1256, 5, 1, 2)); + Assert.Equal(2, indexOfItemIndexCountEQ(collection1256, 5, 0, 4, EqualityComparer.Default)); + Assert.Equal(3, indexOfItemIndexCountEQ(collection1256, 6, 0, 4, EqualityComparer.Default)); + Assert.Equal(-1, indexOfItemIndexCountEQ(collection1256, 100, 0, 4, EqualityComparer.Default)); + Assert.Equal(-1, indexOfItemIndexCountEQ(collection1256, 100, 4, 0, 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)));