Skip to content

Conversation

@samueltardieu
Copy link
Member

@samueltardieu samueltardieu commented Feb 14, 2025

This lint does more harm than good: in its description, it proposes to rewrite match on Vec<_> indexes or slices by a version which cannot panic but masks the failure by choosing the default variant.

The clippy::indexing_slicing restriction lint covers those cases more safely, by suggesting to use a non-panicking version to retrieve the value from the container, without suggesting to fallback to the default success variant in case of failure.

This PR is an (opposite) alternative to #14208 (which will add a suggestion to the lint matching the lint description). Discussion on both PRs can be found on Zulip.

changelog: [match_on_vec_items]: deprecate lint

@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2025

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 14, 2025
@samueltardieu samueltardieu force-pushed the push-outvnnuossmq branch 2 times, most recently from 609321b to 7caf0c9 Compare February 15, 2025 15:33
This lint does more harm than good: in its description, it proposes to
rewrite `match` on `Vec<_>` indexes or slices by a version which cannot
panic but masks the failure by choosing the default variant.

The `clippy::indexing_slicing` restriction lint covers those cases more
safely, by suggesting to use a non-panicking version to retrieve the
value from the container, without suggesting to fallback to the default
success variant in case of failure.
@samueltardieu
Copy link
Member Author

Rebased

@samueltardieu
Copy link
Member Author

@llogiq Any objection to merge? Cf. discussion on Zulip.

@samueltardieu
Copy link
Member Author

Merging since @llogiq didn't object and there is one approval.

@samueltardieu samueltardieu added this pull request to the merge queue Apr 13, 2025
Merged via the queue into rust-lang:master with commit b90c80a Apr 13, 2025
11 checks passed
@samueltardieu samueltardieu deleted the push-outvnnuossmq branch April 13, 2025 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants