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

Suggest usage of array and Vec .as_slice() and .as_mut_slice() methods #8405

Closed
wants to merge 5 commits into from
Closed
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3254,6 +3254,7 @@ Released 2018-09-13
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
[`manual_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice
[`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
[`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat
[`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ store.register_lints(&[
manual_map::MANUAL_MAP,
manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
manual_ok_or::MANUAL_OK_OR,
manual_slice::MANUAL_SLICE,
manual_strip::MANUAL_STRIP,
manual_unwrap_or::MANUAL_UNWRAP_OR,
map_clone::MAP_CLONE,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_restriction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(integer_division::INTEGER_DIVISION),
LintId::of(let_underscore::LET_UNDERSCORE_MUST_USE),
LintId::of(literal_representation::DECIMAL_LITERAL_REPRESENTATION),
LintId::of(manual_slice::MANUAL_SLICE),
LintId::of(map_err_ignore::MAP_ERR_IGNORE),
LintId::of(matches::REST_PAT_IN_FULLY_BOUND_STRUCTS),
LintId::of(matches::WILDCARD_ENUM_MATCH_ARM),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ mod manual_bits;
mod manual_map;
mod manual_non_exhaustive;
mod manual_ok_or;
mod manual_slice;
mod manual_strip;
mod manual_unwrap_or;
mod map_clone;
Expand Down Expand Up @@ -862,6 +863,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv)));
store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv)));
store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation));
store.register_late_pass(move || Box::new(manual_slice::ManualSlice::new(msrv)));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
86 changes: 86 additions & 0 deletions clippy_lints/src/manual_slice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use clippy_utils::{
diagnostics::span_lint_and_sugg, meets_msrv, msrvs, source::snippet_with_context, ty::is_type_lang_item,
};
use rustc_errors::Applicability;
use rustc_hir::{lang_items, BorrowKind, Expr, ExprKind, Mutability};
use rustc_lint::{LateContext, LateLintPass};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};

declare_clippy_lint! {
/// ### What it does
/// Finds arrays or vectors being converted to slices of the same length.
/// ### Why is this bad?
/// The methods `as_slice()` or `as_mut_slice()` could be used instead.
/// ### Example
/// ```rust
/// let mut arr: [u32; 3] = [1, 2, 3];
/// let arr_slice = &arr[..];
/// let mutable_arr_slice = &mut arr[..];
///
/// let mut vec = vec![1, 2, 3];
/// let vec_slice = &vec[..];
/// let mutable_vec_slice = &mut vec[..];
/// ```
/// Use instead:
/// ```rust
/// let mut arr: [u32; 3] = [1, 2, 3];
/// let arr_slice = arr.as_slice();
/// let mutable_arr_slice = arr.as_mut_slice();
///
/// let mut vec = vec![1, 2, 3];
/// let vec_slice = vec.as_slice();
/// let mutable_vec_slice = vec.as_mut_slice();
/// ```
#[clippy::version = "1.60.0"]
pub MANUAL_SLICE,
restriction,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this.

"Suggest use of array and Vec .as_slice() and .as_mut_slice() methods"
}

#[derive(Clone)]
pub struct ManualSlice {
msrv: Option<RustcVersion>,
}

impl ManualSlice {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self { msrv }
}
}

impl_lint_pass!(ManualSlice => [MANUAL_SLICE]);

impl<'tcx> LateLintPass<'tcx> for ManualSlice {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
if !meets_msrv(self.msrv.as_ref(), &msrvs::NON_EXHAUSTIVE) {
return;
}
let ctxt = expr.span.ctxt();
if_chain! {
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, inner) = &expr.kind;
if let ExprKind::Index(object, range) = &inner.kind;
if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), lang_items::LangItem::RangeFull);
then {
let mut app = Applicability::MachineApplicable;
let snip = snippet_with_context(cx, object.span, ctxt, "..", &mut app).0;
let suggested_method = match mutability {
Mutability::Not => "to_slice()",
Mutability::Mut => "to_mut_slice()",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be as_slice/as_mut_slice

};
span_lint_and_sugg(
cx,
MANUAL_SLICE,
expr.span,
"converting to a slice of the same length",
"use",
format!("{}.{}", snip, suggested_method),
app
);
}
}
}

extract_msrv_attr!(LateContext);
}
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ macro_rules! msrv_aliases {

// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,58,0 { MANUAL_SLICE }
1,53,0 { OR_PATTERNS, MANUAL_BITS }
1,52,0 { STR_SPLIT_ONCE }
1,51,0 { BORROW_AS_PTR }
Expand Down
33 changes: 33 additions & 0 deletions tests/ui/manual_slice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#![warn(clippy::manual_slice)]

fn main() {
let mut arr: [u32; 3] = [1, 2, 3];
let arr_slice = &arr[..];
let mutable_arr_slice = &mut arr[..];

let mut vec = vec![1, 2, 3];
let vec_slice = &vec[..];
let mutable_vec_slice = &mut vec[..];

let vec_slice = &vec![1, 2, 3][..];
let mutable_vec_slice = &mut vec![1, 2, 3][..];

let vec_slice: &[i32] = &Vec::new()[..];
let mutable_vec_slice: &mut [i32] = &mut Vec::new()[..];

// Will not fire on any of these

let partial_slice_1 = &arr[1..];
let partial_slice_2 = &arr[..3];
let partial_slice_3 = &arr[1..3];
let partial_mut_slice_1 = &mut arr[1..];
let partial_mut_slice_2 = &mut arr[..3];
let partial_mut_slice_3 = &mut arr[1..3];

let partial_slice_1 = &vec[1..];
let partial_slice_2 = &vec[..3];
let partial_slice_3 = &vec[1..3];
let partial_mut_slice_1 = &mut vec[1..];
let partial_mut_slice_2 = &mut vec[..3];
let partial_mut_slice_3 = &mut vec[1..3];
}
52 changes: 52 additions & 0 deletions tests/ui/manual_slice.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
error: converting to a slice of the same length
--> $DIR/manual_slice.rs:5:21
|
LL | let arr_slice = &arr[..];
| ^^^^^^^^ help: use: `arr.to_slice()`
|
= note: `-D clippy::manual-slice` implied by `-D warnings`

error: converting to a slice of the same length
--> $DIR/manual_slice.rs:6:29
|
LL | let mutable_arr_slice = &mut arr[..];
| ^^^^^^^^^^^^ help: use: `arr.to_mut_slice()`

error: converting to a slice of the same length
--> $DIR/manual_slice.rs:9:21
|
LL | let vec_slice = &vec[..];
| ^^^^^^^^ help: use: `vec.to_slice()`

error: converting to a slice of the same length
--> $DIR/manual_slice.rs:10:29
|
LL | let mutable_vec_slice = &mut vec[..];
| ^^^^^^^^^^^^ help: use: `vec.to_mut_slice()`

error: converting to a slice of the same length
--> $DIR/manual_slice.rs:12:21
|
LL | let vec_slice = &vec![1, 2, 3][..];
| ^^^^^^^^^^^^^^^^^^ help: use: `vec![1, 2, 3].to_slice()`

error: converting to a slice of the same length
--> $DIR/manual_slice.rs:13:29
|
LL | let mutable_vec_slice = &mut vec![1, 2, 3][..];
| ^^^^^^^^^^^^^^^^^^^^^^ help: use: `vec![1, 2, 3].to_mut_slice()`

error: converting to a slice of the same length
--> $DIR/manual_slice.rs:15:29
|
LL | let vec_slice: &[i32] = &Vec::new()[..];
| ^^^^^^^^^^^^^^^ help: use: `Vec::new().to_slice()`

error: converting to a slice of the same length
--> $DIR/manual_slice.rs:16:41
|
LL | let mutable_vec_slice: &mut [i32] = &mut Vec::new()[..];
| ^^^^^^^^^^^^^^^^^^^ help: use: `Vec::new().to_mut_slice()`

error: aborting due to 8 previous errors