Skip to content

Commit 01a0a36

Browse files
committed
Auto merge of #11744 - matthri:get-first-deque-fix, r=Jarcho
Fix get_first false negative for VecDeque fixes #11695 Also run the lint on `VecDeque` and suggest using `.front()` instead of `.get(0)` when trying to access the first element. PS: At first I implemented the VecDeque Lint in a separate `if_chain` (see the previous commit). Let me know if thats the preferred way, then I will remove the refactoring into one block. changelog: [`get_first`]: fix false negative: Also lint `VecDeque` and suggest using `front()`
2 parents 919f698 + 61c76dd commit 01a0a36

File tree

4 files changed

+44
-16
lines changed

4 files changed

+44
-16
lines changed

Diff for: clippy_lints/src/methods/get_first.rs

+28-12
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_applicability;
3+
use clippy_utils::ty::is_type_diagnostic_item;
34
use if_chain::if_chain;
45
use rustc_ast::LitKind;
56
use rustc_errors::Applicability;
67
use rustc_hir as hir;
78
use rustc_lint::LateContext;
89
use rustc_span::source_map::Spanned;
10+
use rustc_span::sym;
911

1012
use super::GET_FIRST;
1113

@@ -18,20 +20,34 @@ pub(super) fn check<'tcx>(
1820
if_chain! {
1921
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
2022
if let Some(impl_id) = cx.tcx.impl_of_method(method_id);
21-
if cx.tcx.type_of(impl_id).instantiate_identity().is_slice();
23+
let identity = cx.tcx.type_of(impl_id).instantiate_identity();
2224
if let hir::ExprKind::Lit(Spanned { node: LitKind::Int(0, _), .. }) = arg.kind;
2325
then {
24-
let mut app = Applicability::MachineApplicable;
25-
let slice_name = snippet_with_applicability(cx, recv.span, "..", &mut app);
26-
span_lint_and_sugg(
27-
cx,
28-
GET_FIRST,
29-
expr.span,
30-
&format!("accessing first element with `{slice_name}.get(0)`"),
31-
"try",
32-
format!("{slice_name}.first()"),
33-
app,
34-
);
26+
if identity.is_slice() {
27+
let mut app = Applicability::MachineApplicable;
28+
let slice_name = snippet_with_applicability(cx, recv.span, "..", &mut app);
29+
span_lint_and_sugg(
30+
cx,
31+
GET_FIRST,
32+
expr.span,
33+
&format!("accessing first element with `{slice_name}.get(0)`"),
34+
"try",
35+
format!("{slice_name}.first()"),
36+
app,
37+
);
38+
} else if is_type_diagnostic_item(cx, identity, sym::VecDeque){
39+
let mut app = Applicability::MachineApplicable;
40+
let slice_name = snippet_with_applicability(cx, recv.span, "..", &mut app);
41+
span_lint_and_sugg(
42+
cx,
43+
GET_FIRST,
44+
expr.span,
45+
&format!("accessing first element with `{slice_name}.get(0)`"),
46+
"try",
47+
format!("{slice_name}.front()"),
48+
app,
49+
);
50+
}
3551
}
3652
}
3753
}

Diff for: tests/ui/get_first.fixed

+4-1
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,12 @@ fn main() {
3232
let _ = z[0];
3333

3434
let vecdeque: VecDeque<_> = x.iter().cloned().collect();
35+
let _ = vecdeque.front();
36+
//~^ ERROR: accessing first element with `vecdeque.get(0)`
37+
let _ = vecdeque.get(1);
38+
3539
let hashmap: HashMap<u8, char> = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]);
3640
let btreemap: BTreeMap<u8, char> = BTreeMap::from_iter(vec![(0, 'a'), (1, 'b')]);
37-
let _ = vecdeque.get(0); // Do not lint, because VecDeque is not slice.
3841
let _ = hashmap.get(&0); // Do not lint, because HashMap is not slice.
3942
let _ = btreemap.get(&0); // Do not lint, because BTreeMap is not slice.
4043

Diff for: tests/ui/get_first.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,12 @@ fn main() {
3232
let _ = z[0];
3333

3434
let vecdeque: VecDeque<_> = x.iter().cloned().collect();
35+
let _ = vecdeque.get(0);
36+
//~^ ERROR: accessing first element with `vecdeque.get(0)`
37+
let _ = vecdeque.get(1);
38+
3539
let hashmap: HashMap<u8, char> = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]);
3640
let btreemap: BTreeMap<u8, char> = BTreeMap::from_iter(vec![(0, 'a'), (1, 'b')]);
37-
let _ = vecdeque.get(0); // Do not lint, because VecDeque is not slice.
3841
let _ = hashmap.get(&0); // Do not lint, because HashMap is not slice.
3942
let _ = btreemap.get(&0); // Do not lint, because BTreeMap is not slice.
4043

Diff for: tests/ui/get_first.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,17 @@ error: accessing first element with `z.get(0)`
1919
LL | let _ = z.get(0);
2020
| ^^^^^^^^ help: try: `z.first()`
2121

22+
error: accessing first element with `vecdeque.get(0)`
23+
--> $DIR/get_first.rs:35:13
24+
|
25+
LL | let _ = vecdeque.get(0);
26+
| ^^^^^^^^^^^^^^^ help: try: `vecdeque.front()`
27+
2228
error: accessing first element with `non_primitives.get(0)`
23-
--> $DIR/get_first.rs:45:13
29+
--> $DIR/get_first.rs:48:13
2430
|
2531
LL | let _ = non_primitives.get(0);
2632
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `non_primitives.first()`
2733

28-
error: aborting due to 4 previous errors
34+
error: aborting due to 5 previous errors
2935

0 commit comments

Comments
 (0)