Auto merge of rust-lang#10901 - y21:smarter-useless-vec, r=Manishearth,Centri3

[`useless_vec`]: lint `vec!` invocations when a slice or an array would do

First off, sorry for that large diff in tests. *A lot* of tests seem to trigger the lint with this new change, so I decided to `#![allow()]` the lint in the affected tests to make reviewing this easier, and also split the commits up so that the first commit is the actual logic of the lint and the second commit contains all the test changes. The stuff that changed in the tests is mostly just line numbers now. So, as large as the diff looks, it's not actually that bad. 😅
I manually went through all of these to find out about edge cases and decided to put them in `tests/ui/vec.rs`.

For more context, I wrote about the idea of this PR here: rust-lang/rust-clippy#2262 (comment) (that explains the logic)

Basically, it now also considers the case where a `Vec` is put in a local variable and the user only ever does things with it that one could also do with a slice or an array. This should catch a lot more cases, and (at least from looking at the tests) it does.

changelog: [`useless_vec`]: lint `vec!` invocations when a slice or an array would do (also considering local variables now)

Author: bors

clippy_lints/src/vec.rs

@@ -1,16 +1,21 @@
+use std::ops::ControlFlow;
+
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::higher;
 use clippy_utils::source::snippet_with_applicability;
 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};
+use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Node, PatKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::ty::{self, Ty};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
+use rustc_span::sym;
 
 #[expect(clippy::module_name_repetitions)]
 #[derive(Copy, Clone)]
@@ -20,7 +25,7 @@ pub struct UselessVec {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for usage of `&vec![..]` when using `&[..]` would
+    /// Checks for usage of `vec![..]` when using `[..]` would
     /// be possible.
     ///
     /// ### Why is this bad?
@@ -46,16 +51,70 @@ 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
+    }
+}
+
+/// Checks if the given expression is a method call to a `Vec` method
+/// that also exists on slices. If this returns true, it means that
+/// this expression does not actually require a `Vec` and could just work with an array.
+fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
+    const ALLOWED_METHOD_NAMES: &[&str] = &["len", "as_ptr", "is_empty"];
+
+    if let ExprKind::MethodCall(path, ..) = e.kind {
+        ALLOWED_METHOD_NAMES.contains(&path.ident.name.as_str())
+    } else {
+        is_trait_method(cx, e, sym::IntoIterator)
+    }
+}
+
 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 `&[_]`
         if_chain! {
-            if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty_adjusted(expr).kind();
-            if let ty::Slice(..) = ty.kind();
+            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);
             then {
-                self.check_vec_macro(cx, &vec_args, mutability, expr.span);
+                self.check_vec_macro(cx, &vec_args, mutability, expr.span, SuggestSlice::Yes);
+            }
+        }
+
+        // search for `let foo = vec![_]` expressions where all uses of `foo`
+        // adjust to slices or call a method that exist on slices (e.g. len)
+        if let Some(vec_args) = higher::VecArgs::hir(cx, expr)
+            && let Node::Local(local) = cx.tcx.hir().get_parent(expr.hir_id)
+            // 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
+            && 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| {
+                // allow indexing into a vec and some set of allowed method calls that exist on slices, too
+                if let Some(parent) = get_parent_expr(cx, expr)
+                    && (adjusts_to_slice(cx, expr)
+                        || matches!(parent.kind, ExprKind::Index(..))
+                        || is_allowed_vec_method(cx, parent))
+                {
+                    ControlFlow::Continue(())
+                } else {
+                    ControlFlow::Break(())
+                }
+            }).is_continue();
+
+            if only_slice_uses {
+                self.check_vec_macro(
+                    cx,
+                    &vec_args,
+                    mutbl,
+                    expr.span.ctxt().outer_expn_data().call_site,
+                    SuggestSlice::No
+                );
             }
         }
 
@@ -67,21 +126,36 @@ 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);
+                self.check_vec_macro(cx, &vec_args, Mutability::Not, span, SuggestSlice::Yes);
             }
         }
     }
 }
 
+#[derive(Copy, Clone)]
+enum SuggestSlice {
+    /// Suggest using a slice `&[..]` / `&mut [..]`
+    Yes,
+    /// Suggest using an array: `[..]`
+    No,
+}
+
 impl UselessVec {
     fn check_vec_macro<'tcx>(
         self,
         cx: &LateContext<'tcx>,
         vec_args: &higher::VecArgs<'tcx>,
         mutability: Mutability,
         span: Span,
+        suggest_slice: SuggestSlice,
     ) {
         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) {
@@ -93,14 +167,14 @@ impl UselessVec {
                     match mutability {
                         Mutability::Mut => {
                             format!(
-                                "&mut [{}; {}]",
+                                "{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)
                             )
@@ -120,18 +194,21 @@ impl UselessVec {
                     match mutability {
                         Mutability::Mut => {
                             format!(
-                                "&mut [{}]",
+                                "{borrow_prefix_mut}[{}]",
                                 snippet_with_applicability(cx, span, "..", &mut applicability)
                             )
                         },
                         Mutability::Not => {
-                            format!("&[{}]", snippet_with_applicability(cx, span, "..", &mut applicability))
+                            format!(
+                                "{borrow_prefix}[{}]",
+                                snippet_with_applicability(cx, span, "..", &mut applicability)
+                            )
                         },
                     }
                 } else {
                     match mutability {
-                        Mutability::Mut => "&mut []".into(),
-                        Mutability::Not => "&[]".into(),
+                        Mutability::Mut => format!("{borrow_prefix_mut}[]"),
+                        Mutability::Not => format!("{borrow_prefix}[]"),
                     }
                 }
             },
@@ -142,7 +219,13 @@ impl UselessVec {
             USELESS_VEC,
             span,
             "useless use of `vec!`",
-            "you can use a slice directly",
+            &format!(
+                "you can use {} directly",
+                match suggest_slice {
+                    SuggestSlice::Yes => "a slice",
+                    SuggestSlice::No => "an array",
+                }
+            ),
             snippet,
             applicability,
         );

tests/ui-toml/suppress_lint_in_const/test.rs

@@ -3,7 +3,12 @@
 // We also check the out_of_bounds_indexing lint here, because it lints similar things and
 // we want to avoid false positives.
 #![warn(clippy::out_of_bounds_indexing)]
-#![allow(unconditional_panic, clippy::no_effect, clippy::unnecessary_operation)]
+#![allow(
+    unconditional_panic,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::useless_vec
+)]
 
 const ARR: [i32; 2] = [1, 2];
 const REF: &i32 = &ARR[idx()]; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.

tests/ui-toml/suppress_lint_in_const/test.stderr

@@ -1,17 +1,17 @@
 error[E0080]: evaluation of `main::{constant#3}` failed
-  --> $DIR/test.rs:31:14
+  --> $DIR/test.rs:36:14
    |
 LL |     const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
    |              ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4
 
 note: erroneous constant used
-  --> $DIR/test.rs:31:5
+  --> $DIR/test.rs:36:5
    |
 LL |     const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
    |     ^^^^^^^^^^^^^^^^^^^^^^
 
 error: indexing may panic
-  --> $DIR/test.rs:22:5
+  --> $DIR/test.rs:27:5
    |
 LL |     x[index];
    |     ^^^^^^^^
@@ -20,47 +20,47 @@ LL |     x[index];
    = note: `-D clippy::indexing-slicing` implied by `-D warnings`
 
 error: indexing may panic
-  --> $DIR/test.rs:38:5
+  --> $DIR/test.rs:43:5
    |
 LL |     v[0];
    |     ^^^^
    |
    = help: consider using `.get(n)` or `.get_mut(n)` instead
 
 error: indexing may panic
-  --> $DIR/test.rs:39:5
+  --> $DIR/test.rs:44:5
    |
 LL |     v[10];
    |     ^^^^^
    |
    = help: consider using `.get(n)` or `.get_mut(n)` instead
 
 error: indexing may panic
-  --> $DIR/test.rs:40:5
+  --> $DIR/test.rs:45:5
    |
 LL |     v[1 << 3];
    |     ^^^^^^^^^
    |
    = help: consider using `.get(n)` or `.get_mut(n)` instead
 
 error: indexing may panic
-  --> $DIR/test.rs:46:5
+  --> $DIR/test.rs:51:5
    |
 LL |     v[N];
    |     ^^^^
    |
    = help: consider using `.get(n)` or `.get_mut(n)` instead
 
 error: indexing may panic
-  --> $DIR/test.rs:47:5
+  --> $DIR/test.rs:52:5
    |
 LL |     v[M];
    |     ^^^^
    |
    = help: consider using `.get(n)` or `.get_mut(n)` instead
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/test.rs:10:24
+  --> $DIR/test.rs:15:24
    |
 LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.
    |                        ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs

@@ -1,6 +1,7 @@
 //@compile-flags: --crate-name conf_disallowed_methods
 
 #![warn(clippy::disallowed_methods)]
+#![allow(clippy::useless_vec)]
 
 extern crate futures;
 extern crate regex;