Skip to content

Commit

Permalink
Improve redundant_slicing
Browse files Browse the repository at this point in the history
Fix bad suggestion when a reborrow might be required
Fix bad suggestion when the value being sliced is a macro call
Don't lint inside of a macro due to the previous context sensitive changes
  • Loading branch information
Jarcho committed Mar 26, 2021
1 parent f41d68f commit 0ff68bb
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 20 deletions.
37 changes: 28 additions & 9 deletions clippy_lints/src/redundant_slicing.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::is_type_lang_item;
use clippy_utils::{get_parent_expr, in_macro};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, LangItem};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::{lint::in_external_macro, ty::TyS};
use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::TyS;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
Expand Down Expand Up @@ -40,26 +41,44 @@ declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING]);

impl LateLintPass<'_> for RedundantSlicing {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if in_external_macro(cx.sess(), expr.span) {
if in_macro(expr.span) {
return;
}

let ctxt = expr.span.ctxt();
if_chain! {
if let ExprKind::AddrOf(_, _, addressee) = expr.kind;
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, addressee) = expr.kind;
if addressee.span.ctxt() == ctxt;
if let ExprKind::Index(indexed, range) = addressee.kind;
if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), LangItem::RangeFull);
if TyS::same_type(cx.typeck_results().expr_ty(expr), cx.typeck_results().expr_ty(indexed));
then {
let mut app = Applicability::MachineApplicable;
let hint = snippet_with_applicability(cx, indexed.span, "..", &mut app).into_owned();
let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;

let (reborrow_str, help_str) = if mutability == Mutability::Mut {
// The slice was used to reborrow the mutable reference.
("&mut *", "reborrow the original value instead")
} else if matches!(
get_parent_expr(cx, expr),
Some(Expr {
kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _),
..
})
) {
// The slice was used to make a temporary reference.
("&*", "reborrow the original value instead")
} else {
("", "use the original value instead")
};

span_lint_and_sugg(
cx,
REDUNDANT_SLICING,
expr.span,
"redundant slicing of the whole range",
"use the original slice instead",
hint,
help_str,
format!("{}{}", reborrow_str, snip),
app,
);
}
Expand Down
29 changes: 25 additions & 4 deletions tests/ui/redundant_slicing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,31 @@
#![warn(clippy::redundant_slicing)]

fn main() {
let x: &[u32] = &[0];
let err = &x[..];
let slice: &[u32] = &[0];
let _ = &slice[..];

let v = vec![0];
let ok = &v[..];
let err = &(&v[..])[..];
let _ = &v[..]; // Changes the type
let _ = &(&v[..])[..]; // Outer borrow is redundant

static S: &[u8] = &[0, 1, 2];
let err = &mut &S[..]; // Should reborrow instead of slice

let mut vec = vec![0];
let mut_slice = &mut *vec;
let _ = &mut mut_slice[..]; // Should reborrow instead of slice

macro_rules! m {
($e:expr) => {
$e
};
}
let _ = &m!(slice)[..];

macro_rules! m2 {
($e:expr) => {
&$e[..]
};
}
let _ = m2!(slice); // Don't lint in a macro
}
32 changes: 25 additions & 7 deletions tests/ui/redundant_slicing.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,34 @@
error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:6:15
--> $DIR/redundant_slicing.rs:6:13
|
LL | let err = &x[..];
| ^^^^^^ help: use the original slice instead: `x`
LL | let _ = &slice[..];
| ^^^^^^^^^^ help: use the original value instead: `slice`
|
= note: `-D clippy::redundant-slicing` implied by `-D warnings`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:10:15
--> $DIR/redundant_slicing.rs:10:13
|
LL | let err = &(&v[..])[..];
| ^^^^^^^^^^^^^ help: use the original slice instead: `(&v[..])`
LL | let _ = &(&v[..])[..]; // Outer borrow is redundant
| ^^^^^^^^^^^^^ help: use the original value instead: `(&v[..])`

error: aborting due to 2 previous errors
error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:13:20
|
LL | let err = &mut &S[..]; // Should reborrow instead of slice
| ^^^^^^ help: reborrow the original value instead: `&*S`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:17:13
|
LL | let _ = &mut mut_slice[..]; // Should reborrow instead of slice
| ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:24:13
|
LL | let _ = &m!(slice)[..];
| ^^^^^^^^^^^^^^ help: use the original value instead: `slice`

error: aborting due to 5 previous errors

0 comments on commit 0ff68bb

Please sign in to comment.