Skip to content

Comments

Use Requires.Range in LastIndexOf#124560

Merged
ViveliDuCh merged 5 commits intodotnet:mainfrom
prozolic:lastindexof
Feb 24, 2026
Merged

Use Requires.Range in LastIndexOf#124560
ViveliDuCh merged 5 commits intodotnet:mainfrom
prozolic:lastindexof

Conversation

@prozolic
Copy link
Contributor

@prozolic prozolic commented Feb 18, 2026

This PR is a follow-up to #124161.
Change Requires.Argument(index - count + 1 >= 0) to Requires.Range(index - count + 1 >= 0, nameof(count)) in ImmutableList<T>.Node.LastIndexOf to align with the validation used in ImmutableArray<T>.LastIndexOf.

Changes:

  • Use Requires.Range in ImmutableList<T>.Node.LastIndexOf for validating the index and count parameters.
  • Add a test case to verify that an ArgumentOutOfRangeException is thrown when validation of the index and count parameters in LastIndexOf and FindLastIndex fails.
  • Fix test case to also validate the parameter name.
  • Rename ImmutableList<T>.Node.LastIndexOf index param to startIndex

Change `Requires.Argument(index - count + 1 >= 0)` to
`Requires.Range(index - count + 1 >= 0, nameof(count))` in
`ImmutableList<T>.Node.LastIndexOf` to align with the validation
used in `ImmutableArray<T>.LastIndexOf`.
Copilot AI review requested due to automatic review settings February 18, 2026 16:40
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 18, 2026
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is a follow-up to #124161 that improves parameter validation in ImmutableList<T>.Node.LastIndexOf by changing from Requires.Argument to Requires.Range. This ensures that ArgumentOutOfRangeException (instead of ArgumentException) is thrown when the combination of index and count parameters results in an invalid range, aligning the behavior with ImmutableArray<T>.LastIndexOf.

Changes:

  • Updated LastIndexOf to use Requires.Range with nameof(count) for the validation check index - count + 1 >= 0
  • Added test cases to verify ArgumentOutOfRangeException is thrown for invalid index and count combinations in both LastIndexOf and FindLastIndex

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs Changed validation from Requires.Argument to Requires.Range(index - count + 1 >= 0, nameof(count)) to throw the appropriate exception type
src/libraries/System.Collections.Immutable/tests/IndexOfTests.cs Added test case to verify ArgumentOutOfRangeException is thrown when index - count + 1 < 0 in LastIndexOf
src/libraries/System.Collections.Immutable/tests/ImmutableListTestBase.cs Added test case to verify ArgumentOutOfRangeException is thrown when startIndex - count + 1 < 0 in FindLastIndex

@prozolic prozolic marked this pull request as ready for review February 18, 2026 16:50
Copilot AI review requested due to automatic review settings February 18, 2026 16:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Copilot AI review requested due to automatic review settings February 21, 2026 00:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@ViveliDuCh
Copy link
Member

/ba-g Failing consistently with Known Issue #118603 after retries. Unrelated to PR changes.

@ViveliDuCh ViveliDuCh merged commit df9f260 into dotnet:main Feb 24, 2026
90 of 92 checks passed
@prozolic prozolic deleted the lastindexof branch February 24, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Collections community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants