added downsides to "known problems" for get_unwrap lint #3368
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
phansch
added
the
S-waiting-on-review
|
bors try |
tryBuild succeeded |
|
Thanks! That's definitely good advise for beginners. bors r+ |
bors bot
added a commit
that referenced
this pull request
Oct 31, 2018
3368: added downsides to "known problems" for get_unwrap lint r=flip1995 a=humean As a beginner I found this lint to be confusing because I was not sure how the `Option` type disappeared as conceptually I know that my `.get()` and Index could fail. Initially I thought maybe the compiler or clippy was smart enough to understand that it was impossible for my `.get()` to fail in this particular case, but it was explained to me that using the Index syntax is just shorthand for directly unwrapping the value: https://doc.rust-lang.org/src/std/collections/hash/map.rs.html#1547 For beginners or users trying to iterate quickly it seems common to litter your code with `unwrap` or `except` as placeholders for where some explicit error handling might need to take place. I think it should be warned that using Index is merely more concise, but doesn't at all reduce the risk of panics and might in fact cause you to miss handling them in a future refactor. Co-authored-by: Michael Rutter <[email protected]> Co-authored-by: Michael Rutter <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a beginner I found this lint to be confusing because I was not sure how the
Optiontype disappeared as conceptually I know that my.get()and Index could fail. Initially I thought maybe the compiler or clippy was smart enough to understand that it was impossible for my.get()to fail in this particular case, but it was explained to me that using the Index syntax is just shorthand for directly unwrapping the value:https://doc.rust-lang.org/src/std/collections/hash/map.rs.html#1547
For beginners or users trying to iterate quickly it seems common to litter your code with
unwraporexceptas placeholders for where some explicit error handling might need to take place. I think it should be warned that using Index is merely more concise, but doesn't at all reduce the risk of panics and might in fact cause you to miss handling them in a future refactor.