Merge #3368

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]>

Author: 3 people

clippy_lints/src/methods/mod.rs

@@ -568,7 +568,14 @@ declare_clippy_lint! {
 /// **Why is this bad?** Using the Index trait (`[]`) is more clear and more
 /// concise.
 ///
-/// **Known problems:** None.
+/// **Known problems:** Not a replacement for error handling: Using either
+/// `.unwrap()` or the Index trait (`[]`) carries the risk of causing a `panic`
+/// if the value being accessed is `None`. If the use of `.get().unwrap()` is a
+/// temporary placeholder for dealing with the `Option` type, then this does
+/// not mitigate the need for error handling. If there is a chance that `.get()`
+/// will be `None` in your program, then it is advisable that the `None` case
+/// is handled in a future refactor instead of using `.unwrap()` or the Index 
+/// trait.
 ///
 /// **Example:**
 /// ```rust