Skip to content

[useless_vec]: lint on vec![_] invocations that adjust to a slice #10933

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

Merged
merged 2 commits into from
Jun 12, 2023
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
97 changes: 42 additions & 55 deletions clippy_lints/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use clippy_utils::ty::is_copy;
use clippy_utils::visitors::for_each_local_use_after_expr;
use clippy_utils::{get_parent_expr, higher, is_trait_method};
use if_chain::if_chain;
use rustc_ast::BindingAnnotation;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Node, PatKind};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -54,11 +53,7 @@ declare_clippy_lint! {
impl_lint_pass!(UselessVec => [USELESS_VEC]);

fn adjusts_to_slice(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty_adjusted(e).kind() {
ty.is_slice()
} else {
false
}
matches!(cx.typeck_results().expr_ty_adjusted(e).kind(), ty::Ref(_, ty, _) if ty.is_slice())
}

/// Checks if the given expression is a method call to a `Vec` method
Expand All @@ -76,13 +71,21 @@ fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {

impl<'tcx> LateLintPass<'tcx> for UselessVec {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// search for `&vec![_]` expressions where the adjusted type is `&[_]`
// search for `&vec![_]` or `vec![_]` expressions where the adjusted type is `&[_]`
if_chain! {
if adjusts_to_slice(cx, expr);
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, addressee) = expr.kind;
if let Some(vec_args) = higher::VecArgs::hir(cx, addressee);
if let Some(vec_args) = higher::VecArgs::hir(cx, expr.peel_borrows());
then {
self.check_vec_macro(cx, &vec_args, mutability, expr.span, SuggestSlice::Yes);
let (suggest_slice, span) = if let ExprKind::AddrOf(BorrowKind::Ref, mutability, _) = expr.kind {
// `expr` is `&vec![_]`, so suggest `&[_]` (or `&mut[_]` resp.)
(SuggestedType::SliceRef(mutability), expr.span)
} else {
// `expr` is the `vec![_]` expansion, so suggest `[_]`
// and also use the span of the actual `vec![_]` expression
(SuggestedType::Array, expr.span.ctxt().outer_expn_data().call_site)
};

self.check_vec_macro(cx, &vec_args, span, suggest_slice);
}
}

Expand All @@ -93,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
// for now ignore locals with type annotations.
// this is to avoid compile errors when doing the suggestion here: let _: Vec<_> = vec![..];
&& local.ty.is_none()
&& let PatKind::Binding(BindingAnnotation(_, mutbl), id, ..) = local.pat.kind
&& let PatKind::Binding(_, id, ..) = local.pat.kind
&& is_copy(cx, vec_type(cx.typeck_results().expr_ty_adjusted(expr)))
{
let only_slice_uses = for_each_local_use_after_expr(cx, id, expr.hir_id, |expr| {
Expand All @@ -113,9 +116,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
self.check_vec_macro(
cx,
&vec_args,
mutbl,
expr.span.ctxt().outer_expn_data().call_site,
SuggestSlice::No
SuggestedType::Array
);
}
}
Expand All @@ -128,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
then {
// report the error around the `vec!` not inside `<std macros>:`
let span = arg.span.ctxt().outer_expn_data().call_site;
self.check_vec_macro(cx, &vec_args, Mutability::Not, span, SuggestSlice::No);
self.check_vec_macro(cx, &vec_args, span, SuggestedType::Array);
}
}
}
Expand All @@ -137,29 +139,23 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
}

#[derive(Copy, Clone)]
enum SuggestSlice {
enum SuggestedType {
/// Suggest using a slice `&[..]` / `&mut [..]`
Yes,
SliceRef(Mutability),
/// Suggest using an array: `[..]`
No,
Array,
}

impl UselessVec {
fn check_vec_macro<'tcx>(
&mut self,
cx: &LateContext<'tcx>,
vec_args: &higher::VecArgs<'tcx>,
mutability: Mutability,
span: Span,
suggest_slice: SuggestSlice,
suggest_slice: SuggestedType,
) {
let mut applicability = Applicability::MachineApplicable;

let (borrow_prefix_mut, borrow_prefix) = match suggest_slice {
SuggestSlice::Yes => ("&mut ", "&"),
SuggestSlice::No => ("", ""),
};

let snippet = match *vec_args {
higher::VecArgs::Repeat(elem, len) => {
if let Some(Constant::Int(len_constant)) = constant(cx, cx.typeck_results(), len) {
Expand All @@ -168,21 +164,13 @@ impl UselessVec {
return;
}

match mutability {
Mutability::Mut => {
format!(
"{borrow_prefix_mut}[{}; {}]",
snippet_with_applicability(cx, elem.span, "elem", &mut applicability),
snippet_with_applicability(cx, len.span, "len", &mut applicability)
)
},
Mutability::Not => {
format!(
"{borrow_prefix}[{}; {}]",
snippet_with_applicability(cx, elem.span, "elem", &mut applicability),
snippet_with_applicability(cx, len.span, "len", &mut applicability)
)
},
let elem = snippet_with_applicability(cx, elem.span, "elem", &mut applicability);
let len = snippet_with_applicability(cx, len.span, "len", &mut applicability);

match suggest_slice {
SuggestedType::SliceRef(Mutability::Mut) => format!("&mut [{elem}; {len}]"),
SuggestedType::SliceRef(Mutability::Not) => format!("&[{elem}; {len}]"),
SuggestedType::Array => format!("[{elem}; {len}]"),
}
} else {
return;
Expand All @@ -194,25 +182,24 @@ impl UselessVec {
return;
}
let span = args[0].span.to(last.span);
let args = snippet_with_applicability(cx, span, "..", &mut applicability);

match mutability {
Mutability::Mut => {
format!(
"{borrow_prefix_mut}[{}]",
snippet_with_applicability(cx, span, "..", &mut applicability)
)
match suggest_slice {
SuggestedType::SliceRef(Mutability::Mut) => {
format!("&mut [{args}]")
},
SuggestedType::SliceRef(Mutability::Not) => {
format!("&[{args}]")
},
Mutability::Not => {
format!(
"{borrow_prefix}[{}]",
snippet_with_applicability(cx, span, "..", &mut applicability)
)
SuggestedType::Array => {
format!("[{args}]")
},
}
} else {
match mutability {
Mutability::Mut => format!("{borrow_prefix_mut}[]"),
Mutability::Not => format!("{borrow_prefix}[]"),
match suggest_slice {
SuggestedType::SliceRef(Mutability::Mut) => "&mut []".to_owned(),
SuggestedType::SliceRef(Mutability::Not) => "&[]".to_owned(),
SuggestedType::Array => "[]".to_owned(),
}
}
},
Expand All @@ -226,8 +213,8 @@ impl UselessVec {
&format!(
"you can use {} directly",
match suggest_slice {
SuggestSlice::Yes => "a slice",
SuggestSlice::No => "an array",
SuggestedType::SliceRef(_) => "a slice",
SuggestedType::Array => "an array",
}
),
snippet,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/cloned_instead_of_copied.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![warn(clippy::cloned_instead_of_copied)]
#![allow(unused)]
#![allow(clippy::useless_vec)]

fn main() {
// yay
Expand Down
1 change: 1 addition & 0 deletions tests/ui/cloned_instead_of_copied.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![warn(clippy::cloned_instead_of_copied)]
#![allow(unused)]
#![allow(clippy::useless_vec)]

fn main() {
// yay
Expand Down
16 changes: 8 additions & 8 deletions tests/ui/cloned_instead_of_copied.stderr
Original file line number Diff line number Diff line change
@@ -1,49 +1,49 @@
error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:8:24
--> $DIR/cloned_instead_of_copied.rs:9:24
|
LL | let _ = [1].iter().cloned();
| ^^^^^^ help: try: `copied`
|
= note: `-D clippy::cloned-instead-of-copied` implied by `-D warnings`

error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:9:31
--> $DIR/cloned_instead_of_copied.rs:10:31
|
LL | let _ = vec!["hi"].iter().cloned();
| ^^^^^^ help: try: `copied`

error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:10:22
--> $DIR/cloned_instead_of_copied.rs:11:22
|
LL | let _ = Some(&1).cloned();
| ^^^^^^ help: try: `copied`

error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:11:34
--> $DIR/cloned_instead_of_copied.rs:12:34
|
LL | let _ = Box::new([1].iter()).cloned();
| ^^^^^^ help: try: `copied`

error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:12:32
--> $DIR/cloned_instead_of_copied.rs:13:32
|
LL | let _ = Box::new(Some(&1)).cloned();
| ^^^^^^ help: try: `copied`

error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:28:22
--> $DIR/cloned_instead_of_copied.rs:29:22
|
LL | let _ = Some(&1).cloned(); // Option::copied needs 1.35
| ^^^^^^ help: try: `copied`

error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:33:24
--> $DIR/cloned_instead_of_copied.rs:34:24
|
LL | let _ = [1].iter().cloned(); // Iterator::copied needs 1.36
| ^^^^^^ help: try: `copied`

error: used `cloned` where `copied` could be used instead
--> $DIR/cloned_instead_of_copied.rs:34:22
--> $DIR/cloned_instead_of_copied.rs:35:22
|
LL | let _ = Some(&1).cloned();
| ^^^^^^ help: try: `copied`
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/eta.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
clippy::no_effect,
clippy::option_map_unit_fn,
clippy::redundant_closure_call,
clippy::uninlined_format_args
clippy::uninlined_format_args,
clippy::useless_vec
)]

use std::path::{Path, PathBuf};
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/eta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
clippy::no_effect,
clippy::option_map_unit_fn,
clippy::redundant_closure_call,
clippy::uninlined_format_args
clippy::uninlined_format_args,
clippy::useless_vec
)]

use std::path::{Path, PathBuf};
Expand Down
Loading