Skip to content

Add new lint might_panic #11889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

cocodery
Copy link
Contributor

@cocodery cocodery commented Nov 28, 2023

fixes #issue11808

This lint check accessing array or slice with index but don't know whether this index is legal.

For example, the code

let numbers = &[4, 7, 3];
let _my_number = numbers[5];

can be written as

let numbers = &[4, 7, 3];
let Some(_my_number) = numbers.get(5) else { panic!(); }

Besides, if you declare my_number with its type i32,

let numbers = &[4, 7, 3];
let _my_number : i32 = numbers[5];

it can be written as

let numbers = &[4, 7, 3];
let Some(_my_number): Option<&i32> = numbers.get(5) else { panic!() };

This lint can help programmer to locate where you access array out of boundary easilier, and can write their handler.

changelog: add new lint might_panic

@rustbot
Copy link
Collaborator

rustbot commented Nov 28, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 28, 2023
@cocodery cocodery force-pushed the might-panic branch 2 times, most recently from ce52300 to 6dfc169 Compare November 28, 2023 11:46
@bors
Copy link
Contributor

bors commented Dec 1, 2023

☔ The latest upstream changes (presumably #11597) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 2, 2023

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

fix tests which are affected by `might-panic`
@xFrednet
Copy link
Member

xFrednet commented Dec 2, 2023

Hey @cocodery, I think you can leave this PR until the discussion in the issue has been solved. If you rebase now, it's likely to be broken again, by the time we came to a conclusion. You can maybe pickup another issue, with the good-first-issue label :)

@cocodery cocodery reopened this Dec 2, 2023
@cocodery
Copy link
Contributor Author

cocodery commented Dec 2, 2023

Hey @cocodery, I think you can leave this PR until the discussion in the issue has been solved. If you rebase now, it's likely to be broken again, by the time we came to a conclusion. You can maybe pickup another issue, with the good-first-issue label :)

Hi, thanks for your advice! I'll waiting for you conclusion.
I'm fixing another issue currently. They help me a lot with learning Rust!

@xFrednet
Copy link
Member

I'm closing this PR due to the reasons stated in the issue. @cocodery Thank you for trying to implement this issue! If you have any questions, you're welcome to reach out.

@xFrednet xFrednet closed this Dec 17, 2023
@cocodery cocodery deleted the might-panic branch December 18, 2023 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) 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.

[Book]: Add new section for collections of lints, that focus on the same problem
4 participants