Skip to content
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.

Sign up for GitHub

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

Jump to bottom

New lint match_vec_item #5522

Merged
merged 5 commits into from
Apr 27, 2020
Merged

New lint match_vec_item #5522

merged 5 commits into from
Apr 27, 2020

Conversation

CrazyRoka
Copy link
Contributor

@CrazyRoka CrazyRoka commented Apr 24, 2020 •
edited by phansch
Loading

Added new lint to warn a match on index item which can panic. It's always better to use get(..) instead.
Closes #5500
changelog: New lint match_on_vec_items

flip1995
flip1995 requested changes Apr 24, 2020
View reviewed changes
clippy_lints/src/match_vec_item.rs Outdated Show resolved Hide resolved
clippy_lints/src/match_vec_item.rs Outdated Show resolved Hide resolved
clippy_lints/src/match_vec_item.rs Outdated Show resolved Hide resolved
clippy_lints/src/match_vec_item.rs Outdated Show resolved Hide resolved
clippy_lints/src/match_vec_item.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Apr 24, 2020
@CrazyRoka
Copy link
Contributor Author

I have updated the code according to your suggestions. And thanks for reviewing 😃

phansch
phansch reviewed Apr 25, 2020
View reviewed changes
clippy_lints/src/match_vec_item.rs Outdated Show resolved Hide resolved
clippy_lints/src/match_vec_item.rs Outdated Show resolved Hide resolved
flip1995
flip1995 reviewed Apr 25, 2020
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM overall. Some minor comments.

clippy_lints/src/match_on_vec_items.rs Outdated Show resolved Hide resolved
clippy_lints/src/match_on_vec_items.rs Outdated Show resolved Hide resolved
tests/ui/match_on_vec_items.rs Outdated Show resolved Hide resolved
clippy_lints/src/match_on_vec_items.rs Outdated Show resolved Hide resolved
@CrazyRoka
Small lint update
940c662 
- Changed lint category to `correctness`
- Moved main function to bottom in test file
- Added `FIXME` comment to `span_lint_and_sugg` to improve later
@flip1995
Copy link
Member

cargo dev update_lints required, because of the move to correctness. Everything else LGTM.

@CrazyRoka
Copy link
Contributor Author

cargo dev update_lints required, because of the move to correctness. Everything else LGTM.

Yep, forgot about this

@phansch
Copy link
Member

phansch commented Apr 27, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Apr 27, 2020

📌 Commit b574941 has been approved by phansch

@bors
Copy link
Contributor

bors commented Apr 27, 2020

⌛ Testing commit b574941 with merge f46d5af...

bors added a commit that referenced this pull request Apr 27, 2020
@bors
Auto merge of #5522 - CrazyRoka:match_vec_item, r=phansch
f46d5af 
New  lint `match_vec_item`

Added new lint to warn a match on index item which can panic. It's always better to use `get(..)` instead.
Closes #5500
changelog: New lint `match_on_vec_items`
@bors
Copy link
Contributor

bors commented Apr 27, 2020

💔 Test failed - checks-action_test

@phansch
Copy link
Member

phansch commented Apr 27, 2020

@bors retry

@bors
Copy link
Contributor

bors commented Apr 27, 2020

⌛ Testing commit b574941 with merge d13ffbe...

@bors
Copy link
Contributor

bors commented Apr 27, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: phansch
Pushing d13ffbe to master...

@bors bors merged commit d13ffbe into rust-lang:master Apr 27, 2020
@bors bors mentioned this pull request Apr 27, 2020
Merged
@CrazyRoka CrazyRoka deleted the match_vec_item branch April 27, 2020 07:21
@CrazyRoka CrazyRoka restored the match_vec_item branch April 27, 2020 07:22
@CrazyRoka CrazyRoka deleted the match_vec_item branch April 29, 2020 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phansch phansch phansch left review comments

@flip1995 flip1995 flip1995 approved these changes

Assignees
No one assigned
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

new lint: warn if we match on index item and use wildcard pattern
4 participants
@CrazyRoka @flip1995 @phansch @bors