Skip to content

Commit 3122e3d

Browse files
y21y21
committed
use let chains, move assignments out of block, add tests
1 parent 6331c94 commit 3122e3d

File tree

4 files changed

+95
-69
lines changed

4 files changed

+95
-69
lines changed

‎clippy_lints/src/methods/get_unwrap.rs

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::get_parent_expr;
44
use clippy_utils::source::snippet_with_applicability;
55
use clippy_utils::ty::is_type_diagnostic_item;
6-
use if_chain::if_chain;
76
use rustc_errors::Applicability;
87
use rustc_hir as hir;
98
use rustc_lint::LateContext;
@@ -23,21 +22,16 @@ pub(super) fn check<'tcx>(
2322
let mut applicability = Applicability::MachineApplicable;
2423
let expr_ty = cx.typeck_results().expr_ty(recv);
2524
let get_args_str = snippet_with_applicability(cx, get_arg.span, "..", &mut applicability);
26-
let mut needs_ref;
25+
let mut needs_ref = true;
2726
let caller_type = if derefs_to_slice(cx, recv, expr_ty).is_some() {
28-
needs_ref = true;
2927
"slice"
3028
} else if is_type_diagnostic_item(cx, expr_ty, sym::Vec) {
31-
needs_ref = true;
3229
"Vec"
3330
} else if is_type_diagnostic_item(cx, expr_ty, sym::VecDeque) {
34-
needs_ref = true;
3531
"VecDeque"
3632
} else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym::HashMap) {
37-
needs_ref = true;
3833
"HashMap"
3934
} else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym::BTreeMap) {
40-
needs_ref = true;
4135
"BTreeMap"
4236
} else {
4337
return; // caller is not a type that we want to lint
@@ -48,21 +42,19 @@ pub(super) fn check<'tcx>(
4842
// Handle the case where the result is immediately dereferenced,
4943
// either directly be the user, or as a result of a method call or the like
5044
// by not requiring an explicit reference
51-
if_chain! {
52-
if needs_ref;
53-
if let Some(parent) = get_parent_expr(cx, expr);
54-
if let hir::ExprKind::Unary(hir::UnOp::Deref, _)
45+
if needs_ref
46+
&& let Some(parent) = get_parent_expr(cx, expr)
47+
&& let hir::ExprKind::Unary(hir::UnOp::Deref, _)
5548
| hir::ExprKind::MethodCall(..)
5649
| hir::ExprKind::Field(..)
57-
| hir::ExprKind::Index(..) = parent.kind;
58-
then {
59-
if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind {
60-
// if the user explicitly dereferences the result, we can adjust
61-
// the span to also include the deref part
62-
span = parent.span;
63-
}
64-
needs_ref = false;
50+
| hir::ExprKind::Index(..) = parent.kind
51+
{
52+
if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind {
53+
// if the user explicitly dereferences the result, we can adjust
54+
// the span to also include the deref part
55+
span = parent.span;
6556
}
57+
needs_ref = false;
6658
}
6759

6860
let mut_str = if is_mut { "_mut" } else { "" };

‎tests/ui/get_unwrap.fixed

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,43 @@ fn main() {
6969
let _ = some_vec[0..1].to_vec();
7070
let _ = some_vec[0..1].to_vec();
7171
}
72-
issue9909();
7372
}
74-
fn issue9909() {
75-
let f = &[1, 2, 3];
73+
mod issue9909 {
74+
#![allow(clippy::identity_op, clippy::unwrap_used, dead_code)]
7675

77-
let _x: &i32 = &f[1 + 2];
78-
// ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
76+
fn reduced() {
77+
let f = &[1, 2, 3];
7978

80-
let _x = f[1 + 2].to_string();
81-
// ^ don't include a borrow here
79+
let _x: &i32 = &f[1 + 2];
80+
// ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
8281

83-
let _x = f[1 + 2].abs();
84-
// ^ don't include a borrow here
82+
let _x = f[1 + 2].to_string();
83+
// ^ don't include a borrow here
84+
85+
let _x = f[1 + 2].abs();
86+
// ^ don't include a borrow here
87+
}
88+
89+
// original code:
90+
fn linidx(row: usize, col: usize) -> usize {
91+
row * 1 + col * 3
92+
}
93+
94+
fn main_() {
95+
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];
96+
97+
for i in 0..2 {
98+
for j in i + 1..3 {
99+
if mat[linidx(j, 3)] > mat[linidx(i, 3)] {
100+
for k in 0..4 {
101+
let (x, rest) = mat.split_at_mut(linidx(i, k) + 1);
102+
let a = x.last_mut().unwrap();
103+
let b = &mut rest[linidx(j, k) - linidx(i, k) - 1];
104+
::std::mem::swap(a, b);
105+
}
106+
}
107+
}
108+
}
109+
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);
110+
}
85111
}

‎tests/ui/get_unwrap.rs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,43 @@ fn main() {
6969
let _ = some_vec.get(0..1).unwrap().to_vec();
7070
let _ = some_vec.get_mut(0..1).unwrap().to_vec();
7171
}
72-
issue9909();
7372
}
74-
fn issue9909() {
75-
let f = &[1, 2, 3];
73+
mod issue9909 {
74+
#![allow(clippy::identity_op, clippy::unwrap_used, dead_code)]
7675

77-
let _x: &i32 = f.get(1 + 2).unwrap();
78-
// ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
76+
fn reduced() {
77+
let f = &[1, 2, 3];
7978

80-
let _x = f.get(1 + 2).unwrap().to_string();
81-
// ^ don't include a borrow here
79+
let _x: &i32 = f.get(1 + 2).unwrap();
80+
// ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
8281

83-
let _x = f.get(1 + 2).unwrap().abs();
84-
// ^ don't include a borrow here
82+
let _x = f.get(1 + 2).unwrap().to_string();
83+
// ^ don't include a borrow here
84+
85+
let _x = f.get(1 + 2).unwrap().abs();
86+
// ^ don't include a borrow here
87+
}
88+
89+
// original code:
90+
fn linidx(row: usize, col: usize) -> usize {
91+
row * 1 + col * 3
92+
}
93+
94+
fn main_() {
95+
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];
96+
97+
for i in 0..2 {
98+
for j in i + 1..3 {
99+
if mat[linidx(j, 3)] > mat[linidx(i, 3)] {
100+
for k in 0..4 {
101+
let (x, rest) = mat.split_at_mut(linidx(i, k) + 1);
102+
let a = x.last_mut().unwrap();
103+
let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap();
104+
::std::mem::swap(a, b);
105+
}
106+
}
107+
}
108+
}
109+
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);
110+
}
85111
}

‎tests/ui/get_unwrap.stderr

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -188,46 +188,28 @@ LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec();
188188
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
189189

190190
error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
191-
--> $DIR/get_unwrap.rs:77:20
191+
--> $DIR/get_unwrap.rs:79:24
192192
|
193-
LL | let _x: &i32 = f.get(1 + 2).unwrap();
194-
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `&f[1 + 2]`
195-
196-
error: used `unwrap()` on an `Option` value
197-
--> $DIR/get_unwrap.rs:77:20
198-
|
199-
LL | let _x: &i32 = f.get(1 + 2).unwrap();
200-
| ^^^^^^^^^^^^^^^^^^^^^
201-
|
202-
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
193+
LL | let _x: &i32 = f.get(1 + 2).unwrap();
194+
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `&f[1 + 2]`
203195

204196
error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
205-
--> $DIR/get_unwrap.rs:80:14
206-
|
207-
LL | let _x = f.get(1 + 2).unwrap().to_string();
208-
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
209-
210-
error: used `unwrap()` on an `Option` value
211-
--> $DIR/get_unwrap.rs:80:14
212-
|
213-
LL | let _x = f.get(1 + 2).unwrap().to_string();
214-
| ^^^^^^^^^^^^^^^^^^^^^
197+
--> $DIR/get_unwrap.rs:82:18
215198
|
216-
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
199+
LL | let _x = f.get(1 + 2).unwrap().to_string();
200+
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
217201

218202
error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
219-
--> $DIR/get_unwrap.rs:83:14
203+
--> $DIR/get_unwrap.rs:85:18
220204
|
221-
LL | let _x = f.get(1 + 2).unwrap().abs();
222-
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
205+
LL | let _x = f.get(1 + 2).unwrap().abs();
206+
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
223207

224-
error: used `unwrap()` on an `Option` value
225-
--> $DIR/get_unwrap.rs:83:14
226-
|
227-
LL | let _x = f.get(1 + 2).unwrap().abs();
228-
| ^^^^^^^^^^^^^^^^^^^^^
208+
error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
209+
--> $DIR/get_unwrap.rs:103:33
229210
|
230-
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
211+
LL | let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap();
212+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut rest[linidx(j, k) - linidx(i, k) - 1]`
231213

232-
error: aborting due to 32 previous errors
214+
error: aborting due to 30 previous errors
233215

0 commit comments

Comments
 (0)