Skip to content

Commit

Permalink
Auto merge of #6528 - Jarcho:redundant_slicing, r=flip1995
Browse files Browse the repository at this point in the history
New lint: redundant_slicing

changelog: Added lint: `redundant_slicing`
fixes #6519

This will trigger on any type which implements `Index<RangeFull>` that returns the input type. This would be a false positive if the implementation does something other than return itself, but I'm not sure why you would ever want to do that.
  • Loading branch information
bors committed Jan 17, 2021
2 parents e0d331f + 9146a77 commit 990e2b3
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2162,6 +2162,7 @@ Released 2018-09-13
[`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
[`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
[`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
[`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing
[`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
[`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref
Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ mod redundant_closure_call;
mod redundant_else;
mod redundant_field_names;
mod redundant_pub_crate;
mod redundant_slicing;
mod redundant_static_lifetimes;
mod ref_option_ref;
mod reference;
Expand Down Expand Up @@ -849,6 +850,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&redundant_else::REDUNDANT_ELSE,
&redundant_field_names::REDUNDANT_FIELD_NAMES,
&redundant_pub_crate::REDUNDANT_PUB_CRATE,
&redundant_slicing::REDUNDANT_SLICING,
&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
&ref_option_ref::REF_OPTION_REF,
&reference::DEREF_ADDROF,
Expand Down Expand Up @@ -1229,6 +1231,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default());
store.register_late_pass(move || box types::PtrAsPtr::new(msrv));
store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons);
store.register_late_pass(|| box redundant_slicing::RedundantSlicing);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1591,6 +1594,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&redundant_clone::REDUNDANT_CLONE),
LintId::of(&redundant_closure_call::REDUNDANT_CLOSURE_CALL),
LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
LintId::of(&redundant_slicing::REDUNDANT_SLICING),
LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
LintId::of(&reference::DEREF_ADDROF),
LintId::of(&reference::REF_IN_DEREF),
Expand Down Expand Up @@ -1835,6 +1839,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
LintId::of(&redundant_closure_call::REDUNDANT_CLOSURE_CALL),
LintId::of(&redundant_slicing::REDUNDANT_SLICING),
LintId::of(&reference::DEREF_ADDROF),
LintId::of(&reference::REF_IN_DEREF),
LintId::of(&repeat_once::REPEAT_ONCE),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/map_unit_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ fn reduce_unit_expression<'a>(cx: &LateContext<'_>, expr: &'a hir::Expr<'_>) ->
Some(expr.span)
},
hir::ExprKind::Block(ref block, _) => {
match (&block.stmts[..], block.expr.as_ref()) {
match (block.stmts, block.expr.as_ref()) {
(&[], Some(inner_expr)) => {
// If block only contains an expression,
// reduce `{ X }` to `X`
Expand Down
67 changes: 67 additions & 0 deletions clippy_lints/src/redundant_slicing.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
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_session::{declare_lint_pass, declare_tool_lint};

use crate::utils::{is_type_lang_item, snippet_with_applicability, span_lint_and_sugg};

declare_clippy_lint! {
/// **What it does:** Checks for redundant slicing expressions which use the full range, and
/// do not change the type.
///
/// **Why is this bad?** It unnecessarily adds complexity to the expression.
///
/// **Known problems:** If the type being sliced has an implementation of `Index<RangeFull>`
/// that actually changes anything then it can't be removed. However, this would be surprising
/// to people reading the code and should have a note with it.
///
/// **Example:**
///
/// ```ignore
/// fn get_slice(x: &[u32]) -> &[u32] {
/// &x[..]
/// }
/// ```
/// Use instead:
/// ```ignore
/// fn get_slice(x: &[u32]) -> &[u32] {
/// x
/// }
/// ```
pub REDUNDANT_SLICING,
complexity,
"redundant slicing of the whole range of a type"
}

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) {
return;
}

if_chain! {
if let ExprKind::AddrOf(_, _, addressee) = expr.kind;
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();

span_lint_and_sugg(
cx,
REDUNDANT_SLICING,
expr.span,
"redundant slicing of the whole range",
"use the original slice instead",
hint,
app,
);
}
}
}
}
11 changes: 11 additions & 0 deletions tests/ui/redundant_slicing.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![allow(unused)]
#![warn(clippy::redundant_slicing)]

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

let v = vec![0];
let ok = &v[..];
let err = &(&v[..])[..];
}
16 changes: 16 additions & 0 deletions tests/ui/redundant_slicing.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:6:15
|
LL | let err = &x[..];
| ^^^^^^ help: use the original slice instead: `x`
|
= note: `-D clippy::redundant-slicing` implied by `-D warnings`

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

error: aborting due to 2 previous errors

0 comments on commit 990e2b3

Please sign in to comment.