Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve redundant_slicing #6975

Merged
merged 1 commit into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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