Skip to content

Commit 4bdc97c

Browse files
JarchoJarcho
committed
Account for auto-borrows and precedence in redundant_slicing lint
1 parent faeeef3 commit 4bdc97c

File tree

4 files changed

+58
-18
lines changed

4 files changed

+58
-18
lines changed

‎clippy_lints/src/redundant_slicing.rs‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ use clippy_utils::get_parent_expr;
33
use clippy_utils::source::snippet_with_context;
44
use clippy_utils::ty::{is_type_lang_item, peel_mid_ty_refs};
55
use if_chain::if_chain;
6+
use rustc_ast::util::parser::PREC_PREFIX;
67
use rustc_errors::Applicability;
78
use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
89
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability};
911
use rustc_middle::ty::subst::GenericArg;
1012
use rustc_session::{declare_lint_pass, declare_tool_lint};
1113

@@ -57,44 +59,64 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
5759
then {
5860
let (expr_ty, expr_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(expr));
5961
let (indexed_ty, indexed_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(indexed));
62+
let parent_expr = get_parent_expr(cx, expr);
63+
let needs_parens_for_prefix = parent_expr.map_or(false, |parent| {
64+
parent.precedence().order() > PREC_PREFIX
65+
});
6066
let mut app = Applicability::MachineApplicable;
6167

6268
let (help, sugg) = if expr_ty == indexed_ty {
6369
if expr_ref_count > indexed_ref_count {
70+
// Indexing takes self by reference and can't return a reference to that
71+
// reference as it's a local variable. The only way this could happen is if
72+
// `self` contains a reference to the `Self` type. If this occurs then the
73+
// lint no longer applies as it's essentially a field access, which is not
74+
// redundant.
6475
return;
6576
}
77+
let deref_count = indexed_ref_count - expr_ref_count;
6678

6779
let (reborrow_str, help_str) = if mutability == Mutability::Mut {
6880
// The slice was used to reborrow the mutable reference.
6981
("&mut *", "reborrow the original value instead")
7082
} else if matches!(
71-
get_parent_expr(cx, expr),
83+
parent_expr,
7284
Some(Expr {
7385
kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _),
7486
..
7587
})
76-
) {
88+
) || cx.typeck_results().expr_adjustments(expr).first().map_or(false, |a| {
89+
matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. })))
90+
}) {
7791
// The slice was used to make a temporary reference.
7892
("&*", "reborrow the original value instead")
79-
} else if expr_ref_count != indexed_ref_count {
93+
} else if deref_count != 0 {
8094
("", "dereference the original value instead")
8195
} else {
8296
("", "use the original value instead")
8397
};
8498

8599
let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
86-
(help_str, format!("{}{}{}", reborrow_str, "*".repeat(indexed_ref_count - expr_ref_count), snip))
100+
let sugg = if (deref_count != 0 || !reborrow_str.is_empty()) && needs_parens_for_prefix {
101+
format!("({}{}{})", reborrow_str, "*".repeat(deref_count), snip)
102+
} else {
103+
format!("{}{}{}", reborrow_str, "*".repeat(deref_count), snip)
104+
};
105+
106+
(help_str, sugg)
87107
} else if let Some(target_id) = cx.tcx.lang_items().deref_target() {
88108
if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions(
89109
cx.param_env,
90110
cx.tcx.mk_projection(target_id, cx.tcx.mk_substs([GenericArg::from(indexed_ty)].into_iter())),
91111
) {
92112
if deref_ty == expr_ty {
93113
let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
94-
(
95-
"dereference the original value instead",
96-
format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip),
97-
)
114+
let sugg = if needs_parens_for_prefix {
115+
format!("(&{}{}*{})", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip)
116+
} else {
117+
format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip)
118+
};
119+
("dereference the original value instead", sugg)
98120
} else {
99121
return;
100122
}

‎tests/ui/redundant_slicing.fixed‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#![allow(unused)]
44
#![warn(clippy::redundant_slicing)]
55

6+
use std::io::Read;
7+
68
fn main() {
79
let slice: &[u32] = &[0];
810
let _ = slice; // Redundant slice
@@ -37,4 +39,8 @@ fn main() {
3739

3840
let slice_ref = &slice;
3941
let _ = *slice_ref; // Deref instead of slice
42+
43+
// Issue #7972
44+
let bytes: &[u8] = &[];
45+
let _ = (&*bytes).read_to_end(&mut vec![]).unwrap();
4046
}

‎tests/ui/redundant_slicing.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#![allow(unused)]
44
#![warn(clippy::redundant_slicing)]
55

6+
use std::io::Read;
7+
68
fn main() {
79
let slice: &[u32] = &[0];
810
let _ = &slice[..]; // Redundant slice
@@ -37,4 +39,8 @@ fn main() {
3739

3840
let slice_ref = &slice;
3941
let _ = &slice_ref[..]; // Deref instead of slice
42+
43+
// Issue #7972
44+
let bytes: &[u8] = &[];
45+
let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap();
4046
}

‎tests/ui/redundant_slicing.stderr‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,64 @@
11
error: redundant slicing of the whole range
2-
--> $DIR/redundant_slicing.rs:8:13
2+
--> $DIR/redundant_slicing.rs:10:13
33
|
44
LL | let _ = &slice[..]; // Redundant slice
55
| ^^^^^^^^^^ help: use the original value instead: `slice`
66
|
77
= note: `-D clippy::redundant-slicing` implied by `-D warnings`
88

99
error: redundant slicing of the whole range
10-
--> $DIR/redundant_slicing.rs:11:13
10+
--> $DIR/redundant_slicing.rs:13:13
1111
|
1212
LL | let _ = &v[..]; // Deref instead of slice
1313
| ^^^^^^ help: dereference the original value instead: `&*v`
1414

1515
error: redundant slicing of the whole range
16-
--> $DIR/redundant_slicing.rs:12:13
16+
--> $DIR/redundant_slicing.rs:14:13
1717
|
1818
LL | let _ = &(&*v)[..]; // Outer borrow is redundant
1919
| ^^^^^^^^^^ help: use the original value instead: `(&*v)`
2020

2121
error: redundant slicing of the whole range
22-
--> $DIR/redundant_slicing.rs:15:20
22+
--> $DIR/redundant_slicing.rs:17:20
2323
|
2424
LL | let err = &mut &S[..]; // Should reborrow instead of slice
2525
| ^^^^^^ help: reborrow the original value instead: `&*S`
2626

2727
error: redundant slicing of the whole range
28-
--> $DIR/redundant_slicing.rs:18:21
28+
--> $DIR/redundant_slicing.rs:20:21
2929
|
3030
LL | let mut_slice = &mut vec[..]; // Deref instead of slice
3131
| ^^^^^^^^^^^^ help: dereference the original value instead: `&mut *vec`
3232

3333
error: redundant slicing of the whole range
34-
--> $DIR/redundant_slicing.rs:19:13
34+
--> $DIR/redundant_slicing.rs:21:13
3535
|
3636
LL | let _ = &mut mut_slice[..]; // Should reborrow instead of slice
3737
| ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice`
3838

3939
error: redundant slicing of the whole range
40-
--> $DIR/redundant_slicing.rs:22:13
40+
--> $DIR/redundant_slicing.rs:24:13
4141
|
4242
LL | let _ = &ref_vec[..]; // Deref instead of slice
4343
| ^^^^^^^^^^^^ help: dereference the original value instead: `&**ref_vec`
4444

4545
error: redundant slicing of the whole range
46-
--> $DIR/redundant_slicing.rs:29:13
46+
--> $DIR/redundant_slicing.rs:31:13
4747
|
4848
LL | let _ = &m!(slice)[..];
4949
| ^^^^^^^^^^^^^^ help: use the original value instead: `slice`
5050

5151
error: redundant slicing of the whole range
52-
--> $DIR/redundant_slicing.rs:39:13
52+
--> $DIR/redundant_slicing.rs:41:13
5353
|
5454
LL | let _ = &slice_ref[..]; // Deref instead of slice
5555
| ^^^^^^^^^^^^^^ help: dereference the original value instead: `*slice_ref`
5656

57-
error: aborting due to 9 previous errors
57+
error: redundant slicing of the whole range
58+
--> $DIR/redundant_slicing.rs:45:13
59+
|
60+
LL | let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap();
61+
| ^^^^^^^^^^^^ help: reborrow the original value instead: `(&*bytes)`
62+
63+
error: aborting due to 10 previous errors
5864

0 commit comments

Comments
 (0)