List<T>.IndexOf outputs incorrect error message

[HeinziAT]

Description

The error message returned by List<T>.IndexOf if an invalid index is used is incorrect. As it is currently implemented, the size of the collection (2 in the repro example below) is a valid input for index.

Reproduction Steps

// list of length 2
var list = new List<string>() { "a", "b" };

Console.WriteLine(list.IndexOf("a", 2)); // yields -1
Console.WriteLine(list.IndexOf("a", 3)); // yields an error

(fiddle)

Expected behavior

An ArgumentException with the following error text is thrown:

Index was out of range. Must be non-negative and less than or equal to the size of the collection. (Parameter 'index')

(Alternatively: Must be non-negative and not greater than the size of the collection.)

Actual behavior

An ArgumentException with the following error text is thrown:

Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')

Regression?

No response

Known Workarounds

No response

Configuration

Tested with dotnetfiddle, with all currently available compilers (.NET 4.7.2, Roslyn 4.0, .NET 6).

Other information

I do not suggest to change the behavior of IndexOf - the current behavior is the only sane and consistent one. However, the error message (and the documentation of List<T>.IndexOf) should be updated to reflect the actual behavior.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

The error message returned by List<T>.IndexOf if an invalid index is used is incorrect. As it is currently implemented, the size of the collection (2 in the repro example below) is a valid input for index.

Reproduction Steps

// list of length 2
var list = new List<string>() { "a", "b" };

Console.WriteLine(list.IndexOf("a", 2)); // yields -1
Console.WriteLine(list.IndexOf("a", 3)); // yields an error

(fiddle)

Expected behavior

An ArgumentException with the following error text is thrown:

Index was out of range. Must be non-negative and less than or equal to the size of the collection. (Parameter 'index')

(Alternatively: Must be non-negative and not greater than the size of the collection.)

Actual behavior

An ArgumentException with the following error text is thrown:

Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')

Regression?

No response

Known Workarounds

No response

Configuration

Tested with dotnetfiddle, with all currently available compilers (.NET 4.7.2, Roslyn 4.0, .NET 6).

Other information

I do not suggest to change the behavior of IndexOf - the current behavior is the only sane and consistent one. However, the error message (and the documentation of List<T>.IndexOf) should be updated to reflect the actual behavior.

Author:	HeinziAT
Assignees:	-
Labels:	area-System.Collections, untriaged
Milestone:	-

[B1Z0N]

From what I see we are pretty much inconsistent in usage of ThrowHelper.ThrowArgumentOutOfRange_IndexException in corelib and other libraries. For example just on List in the same file see current IndexOf example and this indexer here.

ThrowHelper.ThrowArgumentOutOfRange_IndexException uses SR.ArgumentOutOfRange_Index internally. And that one in turn belongs to resource files like this one here. I think that perhaps we should just split it into two:

• SR.ArgumentOutOfRange_IndexMustBeLessThan for errors like that in List indexer and other throw if index >= size cases
• SR.ArgumentOutOfRange_IndexMustBeLessThanOrEqual for errors like current IndexOf and other throw if index > size cases

Of course we also should not forget to update docs.

I would be grateful if someone from area owners, perhaps @jeffhandley, answered:

1. How is the idea, namings, backward compat problems, etc?
2. I'd like to take the job as a contributor, may I?

[Clockwork-Muse]

Naive guess: it's implemented this way to support striding/slicing:

int previousIndex = 0;
int index = commaDelimitedString.IndexOf(",", 0);

while(index > -1)
{
    handleElement(commaDelimitedString.Substring(previousIndex, index));
    previousIndex = index;
    index = commaDelimitedString.IndexOf(",", index + 1);
}

...also, consider the case when the string is empty.

[eiriktsarpalis]

@B1Z0N happy to accept a PR that improves this.