diff --git a/clippy_lints/src/methods/get_unwrap.rs b/clippy_lints/src/methods/get_unwrap.rs index 209d42ebabd..ba5aae28bd6 100644 --- a/clippy_lints/src/methods/get_unwrap.rs +++ b/clippy_lints/src/methods/get_unwrap.rs @@ -3,7 +3,6 @@ use clippy_utils::get_parent_expr; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -23,21 +22,16 @@ pub(super) fn check<'tcx>( let mut applicability = Applicability::MachineApplicable; let expr_ty = cx.typeck_results().expr_ty(recv); let get_args_str = snippet_with_applicability(cx, get_arg.span, "..", &mut applicability); - let mut needs_ref; + let mut needs_ref = true; let caller_type = if derefs_to_slice(cx, recv, expr_ty).is_some() { - needs_ref = true; "slice" } else if is_type_diagnostic_item(cx, expr_ty, sym::Vec) { - needs_ref = true; "Vec" } else if is_type_diagnostic_item(cx, expr_ty, sym::VecDeque) { - needs_ref = true; "VecDeque" } else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym::HashMap) { - needs_ref = true; "HashMap" } else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym::BTreeMap) { - needs_ref = true; "BTreeMap" } else { return; // caller is not a type that we want to lint @@ -48,21 +42,19 @@ pub(super) fn check<'tcx>( // Handle the case where the result is immediately dereferenced, // either directly be the user, or as a result of a method call or the like // by not requiring an explicit reference - if_chain! { - if needs_ref; - if let Some(parent) = get_parent_expr(cx, expr); - if let hir::ExprKind::Unary(hir::UnOp::Deref, _) + if needs_ref + && let Some(parent) = get_parent_expr(cx, expr) + && let hir::ExprKind::Unary(hir::UnOp::Deref, _) | hir::ExprKind::MethodCall(..) | hir::ExprKind::Field(..) - | hir::ExprKind::Index(..) = parent.kind; - then { - if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind { - // if the user explicitly dereferences the result, we can adjust - // the span to also include the deref part - span = parent.span; - } - needs_ref = false; + | hir::ExprKind::Index(..) = parent.kind + { + if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind { + // if the user explicitly dereferences the result, we can adjust + // the span to also include the deref part + span = parent.span; } + needs_ref = false; } let mut_str = if is_mut { "_mut" } else { "" }; diff --git a/tests/ui/get_unwrap.fixed b/tests/ui/get_unwrap.fixed index 3e4c2b499e4..f0d28dd9349 100644 --- a/tests/ui/get_unwrap.fixed +++ b/tests/ui/get_unwrap.fixed @@ -69,17 +69,43 @@ fn main() { let _ = some_vec[0..1].to_vec(); let _ = some_vec[0..1].to_vec(); } - issue9909(); } -fn issue9909() { - let f = &[1, 2, 3]; +mod issue9909 { + #![allow(clippy::identity_op, clippy::unwrap_used, dead_code)] - let _x: &i32 = &f[1 + 2]; - // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal + fn reduced() { + let f = &[1, 2, 3]; - let _x = f[1 + 2].to_string(); - // ^ don't include a borrow here + let _x: &i32 = &f[1 + 2]; + // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal - let _x = f[1 + 2].abs(); - // ^ don't include a borrow here + let _x = f[1 + 2].to_string(); + // ^ don't include a borrow here + + let _x = f[1 + 2].abs(); + // ^ don't include a borrow here + } + + // original code: + fn linidx(row: usize, col: usize) -> usize { + row * 1 + col * 3 + } + + fn main_() { + let mut mat = [1.0f32, 5.0, 9.0, 2.0, 6.0, 10.0, 3.0, 7.0, 11.0, 4.0, 8.0, 12.0]; + + for i in 0..2 { + for j in i + 1..3 { + if mat[linidx(j, 3)] > mat[linidx(i, 3)] { + for k in 0..4 { + let (x, rest) = mat.split_at_mut(linidx(i, k) + 1); + let a = x.last_mut().unwrap(); + let b = &mut rest[linidx(j, k) - linidx(i, k) - 1]; + ::std::mem::swap(a, b); + } + } + } + } + assert_eq!([9.0, 5.0, 1.0, 10.0, 6.0, 2.0, 11.0, 7.0, 3.0, 12.0, 8.0, 4.0], mat); + } } diff --git a/tests/ui/get_unwrap.rs b/tests/ui/get_unwrap.rs index a967971a7dc..21c1ecb0af2 100644 --- a/tests/ui/get_unwrap.rs +++ b/tests/ui/get_unwrap.rs @@ -69,17 +69,43 @@ fn main() { let _ = some_vec.get(0..1).unwrap().to_vec(); let _ = some_vec.get_mut(0..1).unwrap().to_vec(); } - issue9909(); } -fn issue9909() { - let f = &[1, 2, 3]; +mod issue9909 { + #![allow(clippy::identity_op, clippy::unwrap_used, dead_code)] - let _x: &i32 = f.get(1 + 2).unwrap(); - // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal + fn reduced() { + let f = &[1, 2, 3]; - let _x = f.get(1 + 2).unwrap().to_string(); - // ^ don't include a borrow here + let _x: &i32 = f.get(1 + 2).unwrap(); + // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal - let _x = f.get(1 + 2).unwrap().abs(); - // ^ don't include a borrow here + let _x = f.get(1 + 2).unwrap().to_string(); + // ^ don't include a borrow here + + let _x = f.get(1 + 2).unwrap().abs(); + // ^ don't include a borrow here + } + + // original code: + fn linidx(row: usize, col: usize) -> usize { + row * 1 + col * 3 + } + + fn main_() { + let mut mat = [1.0f32, 5.0, 9.0, 2.0, 6.0, 10.0, 3.0, 7.0, 11.0, 4.0, 8.0, 12.0]; + + for i in 0..2 { + for j in i + 1..3 { + if mat[linidx(j, 3)] > mat[linidx(i, 3)] { + for k in 0..4 { + let (x, rest) = mat.split_at_mut(linidx(i, k) + 1); + let a = x.last_mut().unwrap(); + let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap(); + ::std::mem::swap(a, b); + } + } + } + } + assert_eq!([9.0, 5.0, 1.0, 10.0, 6.0, 2.0, 11.0, 7.0, 3.0, 12.0, 8.0, 4.0], mat); + } } diff --git a/tests/ui/get_unwrap.stderr b/tests/ui/get_unwrap.stderr index 6221c236830..154083ea654 100644 --- a/tests/ui/get_unwrap.stderr +++ b/tests/ui/get_unwrap.stderr @@ -188,46 +188,28 @@ LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec(); = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:77:20 + --> $DIR/get_unwrap.rs:79:24 | -LL | let _x: &i32 = f.get(1 + 2).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `&f[1 + 2]` - -error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:77:20 - | -LL | let _x: &i32 = f.get(1 + 2).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^ - | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message +LL | let _x: &i32 = f.get(1 + 2).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `&f[1 + 2]` error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:80:14 + --> $DIR/get_unwrap.rs:82:18 | -LL | let _x = f.get(1 + 2).unwrap().to_string(); - | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]` - -error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:80:14 - | -LL | let _x = f.get(1 + 2).unwrap().to_string(); - | ^^^^^^^^^^^^^^^^^^^^^ - | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message +LL | let _x = f.get(1 + 2).unwrap().to_string(); + | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]` error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:83:14 + --> $DIR/get_unwrap.rs:85:18 | -LL | let _x = f.get(1 + 2).unwrap().abs(); - | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]` +LL | let _x = f.get(1 + 2).unwrap().abs(); + | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]` -error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:83:14 +error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise + --> $DIR/get_unwrap.rs:103:33 | -LL | let _x = f.get(1 + 2).unwrap().abs(); - | ^^^^^^^^^^^^^^^^^^^^^ - | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message +LL | let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut rest[linidx(j, k) - linidx(i, k) - 1]` -error: aborting due to 32 previous errors +error: aborting due to 30 previous errors