Runtime: System.ArgumentOutOfRangeException message is incorrect when inserting into a list

[v-kydela]

The ArgumentOutOfRange_Index message for a System.ArgumentOutOfRangeException goes like this:

Index was out of range. Must be non-negative and less than the size of the collection.

The "less than the size of the collection" part is only true when reading from a list or using a lists's set indexer. When inserting into a list, the index can be less than or equal to the size of the list.

I'm unsure if this also applies to other types of collection types besides lists, but so far List<T> is the only type I've found with such a method that allows an index equal to the size of the collection.

To reproduce

```c#
var list = new List { "foo", "bar" };

// (The size of the list is 2)
list.Insert(2, "baz");
// No exception

// (The size of the list is 3)
list.Insert(4, "qux");
// ArgumentOutOfRangeException:
// Index was out of range. Must be non-negative and less than the size of the collection.
```

This also goes for List<T>.InsertRange.

To resolve

1. The message could be reworded to be ambiguous enough to apply to both cases, e.g. "Must be non-negative and within the size of the collection."
2. There could be a new message introduced that's used for this case, e.g. "Index was out of range. Must be non-negative and not greater than the size of the collection."

[danmosemsft]

The other similar messages suggest a form like "must refer to a location within".

  <data name="ArgumentOutOfRange_Index" xml:space="preserve">
    <value>Index was out of range. Must be non-negative and less than the size of the collection.</value>
  </data>
  <data name="ArgumentOutOfRange_IndexCount" xml:space="preserve">
    <value>Index and count must refer to a location within the string.</value>
  </data>
  <data name="ArgumentOutOfRange_IndexCountBuffer" xml:space="preserve">
    <value>Index and count must refer to a location within the buffer.</value>
  </data>

Incidentally I notice some cases we are using ArgumentOutOfRange_Index when it is not really a collection, eg OSEncoding.GetBytes.

[vaz-rodrigo]

What about changing it to:

"Index was out of range. Must refer to a valid location within the collection."

This leaves it similar to the other messages while also hinting that there is some criteria to be obeyed.

[danmosemsft]

It might be nice to include the index and collection size as well, for those "debugging" with only the help of the exception message.

Eg., "Index {0} was not within the collection of length {1}." or something like that? Perhaps you have a better suggestion.

This assumes that the cost of throwing the exception is significantly more than the cost of gathering the length (which for some collections is not free)

[vaz-rodrigo]

What if we add a different message for Insertions that are out of range? I read through a bit of the code here and I couldn't find anything else that allowed for a insertion by integer index. The closest that I could find was Collections.Generic.SortedList, which explicitly throws if you even try to do it.

Maybe have something like:
ArgumentOutOfRange_InsertIndex
"Index {0} was not within the collection's limits of insertion."
or
"Insertion index must be within the length of the collection"

[danmosemsft]

Not required to ship 5.0

[fagnercarvalho]

Hey @danmosemsft, I tried to reproduce this issue and it seems like this was fixed.

This is the message I'm getting if I try to reproduce this:

'Index must be within the bounds of the List. (Parameter 'index')'

I'm using .NET Core 3.1.

[danmosemsft]

You're right, the repro does not seem correct. However this does produce the arguably-wrong message

var list = new List<string> { "foo", "bar" };
list.InsertRange( 3, new [] {"x"});

[fagnercarvalho]

Hello @danmosemsft, can I work on this?

I went a little deeper on this issue and I'm seeing that this message is being returned incorrectly also in the following methods:

• System.Collections.Generic.List.IndexOf(item, index).
• System.ArraySegment.Slice(index).
• System.ArraySegment.Slice(index, count).
• System.Collections.ObjectModel.Collection.Insert(index, item).
• System.Globalization.StringInfo.GetNextTextElement(str, index).
• System.Globalization.StringInfo.GetTextElementEnumerator(str, index).

Here are the tests I did to reproduce these issues:

    public class ReproduceIssue1250
    {
        private static string ArgumentOutOfRangeExceptionMessage =
            @"Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')";

        [Fact]
        public void ReproduceIssue1250_List_InsertRange()
        {
            var list = new List<string> { "foo", "bar" };

            list.InsertRange(2, new[] { "x" }); // this works

            Assert.Equal(ArgumentOutOfRangeExceptionMessage,
                Assert.Throws<ArgumentOutOfRangeException>(
                    () =>
                    {
                        list.InsertRange(4, new[] { "x" });
                    }).Message);
        }

        [Fact]
        public void ReproduceIssue1250_List_IndexOf()
        {
            var list = new List<string> { "foo", "bar" };

            list.IndexOf("foo", 2); // this works

            Assert.Equal(ArgumentOutOfRangeExceptionMessage,
                Assert.Throws<ArgumentOutOfRangeException>(
                    () =>
                    {
                        list.IndexOf("foo", 3);
                    }).Message);
        }

        [Fact]
        public void ReproduceIssue1250_ArraySegment_Slice()
        {
            var arraySegment = new ArraySegment<int>(new int[] { 1, 2 });

            arraySegment.Slice(2); // this works

            Assert.Equal(ArgumentOutOfRangeExceptionMessage,
                Assert.Throws<ArgumentOutOfRangeException>(
                    () =>
                    {
                        arraySegment.Slice(3);
                    }).Message);
        }

        [Fact]
        public void ReproduceIssue1250_ArraySegment_Slice_Count()
        {
            var arraySegment = new ArraySegment<int>(new int[] { 1, 2 });

            arraySegment.Slice(2, 0); // this works

            Assert.Equal(ArgumentOutOfRangeExceptionMessage,
                Assert.Throws<ArgumentOutOfRangeException>(
                    () =>
                    {
                        arraySegment.Slice(3, 0);
                    }).Message);
        }

        [Fact]
        public void ReproduceIssue1250_Collection_Insert()
        {
            var collection = new Collection<string> { "foo", "bar" };

            collection.Insert(2, "x"); // this works

            Assert.Equal(ArgumentOutOfRangeExceptionMessage,
                Assert.Throws<ArgumentOutOfRangeException>(
                    () =>
                    {
                        collection.Insert(4, "x");
                    }).Message);
        }

        [Fact]
        public void ReproduceIssue1250_StringInfo_GetNextTextElement()
        {
            StringInfo.GetNextTextElement("test", 4); // this works

            Assert.Equal(ArgumentOutOfRangeExceptionMessage,
                Assert.Throws<ArgumentOutOfRangeException>(
                    () =>
                    {
                        StringInfo.GetNextTextElement("test", 5);
                    }).Message);
        }

        [Fact]
        public void ReproduceIssue1250_StringInfo_GetTextElementEnumerator()
        {
            StringInfo.GetTextElementEnumerator("test", 4); // this works

            Assert.Equal(ArgumentOutOfRangeExceptionMessage,
                Assert.Throws<ArgumentOutOfRangeException>(
                    () =>
                    {
                        StringInfo.GetTextElementEnumerator("test", 5);
                    }).Message);
        }
    }

I would fix this by creating appropriate resources for each of these methods. Some of these methods also have additional conditions that will need to be included in the error message.

[danmosemsft]

Sure. Assigned to you. We would not want to take changes that measurably affect performance of the non exceptional path.