diff --git a/clippy_lints/src/single_range_in_vec_init.rs b/clippy_lints/src/single_range_in_vec_init.rs index 0a9a3c6307a7..b5b21c287779 100644 --- a/clippy_lints/src/single_range_in_vec_init.rs +++ b/clippy_lints/src/single_range_in_vec_init.rs @@ -1,12 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::get_trait_def_id; use clippy_utils::higher::VecArgs; use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; +use clippy_utils::{get_trait_def_id, peel_blocks}; use rustc_ast::{LitIntType, LitKind, UintTy}; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, LangItem, QPath}; +use rustc_hir::{Expr, ExprKind, LangItem, Local, Node, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; use std::fmt::{self, Display, Formatter}; @@ -68,7 +68,7 @@ impl Display for SuggestedType { } impl LateLintPass<'_> for SingleRangeInVecInit { - fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { // inner_expr: `vec![0..200]` or `[0..200]` // ^^^^^^ ^^^^^^^ // span: `vec![0..200]` or `[0..200]` @@ -92,6 +92,7 @@ impl LateLintPass<'_> for SingleRangeInVecInit { if matches!(lang_item, LangItem::Range) && let ty = cx.typeck_results().expr_ty(start.expr) + && !(has_type_annotations(cx, expr) || is_argument(cx, expr)) && let Some(snippet) = snippet_opt(cx, span) // `is_from_proc_macro` will skip any `vec![]`. Let's not! && snippet.starts_with(suggested_type.starts_with()) @@ -141,9 +142,56 @@ impl LateLintPass<'_> for SingleRangeInVecInit { Applicability::MaybeIncorrect, ); } + + if let Some(local) = get_local(cx, expr) { + diag.span_suggestion( + local.pat.span.shrink_to_hi(), + "if this is intended, give it type annotations", + format!(": {}", cx.typeck_results().expr_ty(expr)), + Applicability::HasPlaceholders, + ); + } }, ); } } } } + +fn get_local<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Local<'tcx>> { + cx.tcx + .hir() + .parent_iter(expr.hir_id) + .find(|p| matches!(p.1, Node::Local(_))) + .map(|local| local.1.expect_local()) +} + +/// Returns whether `expr` has type annotations if it's a local binding. We should not lint this. +fn has_type_annotations(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let Some((_, Node::Local(local))) = cx + .tcx + .hir() + .parent_iter(expr.hir_id) + .find(|p| matches!(p.1, Node::Local(_))) + else { + return false; + }; + let Some(init) = local.init else { + return false; + }; + + peel_blocks(init).hir_id == expr.hir_id && local.ty.is_some() +} + +/// Returns whether `expr` is used as an argument to a function or initializing a struct/enum in any +/// way. We should not lint this. +fn is_argument(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let parent = cx.tcx.parent_hir_node(expr.hir_id); + + matches!( + parent, + Node::Expr(e) if matches!(e.kind, ExprKind::Call(_, _) | ExprKind::MethodCall(..) | ExprKind::Assign(_, _, _))) + // NOTE: This will ignore anything akin to `Vec<&dyn Any>` as well, which could ideally be + // linted as well, but this should be fine due to the rarity of those circumstances + || matches!(parent, Node::ExprField(_)) +} diff --git a/tests/ui/single_range_in_vec_init.rs b/tests/ui/single_range_in_vec_init.rs index 7887cfc61750..c77528b245e9 100644 --- a/tests/ui/single_range_in_vec_init.rs +++ b/tests/ui/single_range_in_vec_init.rs @@ -1,12 +1,14 @@ //@aux-build:proc_macros.rs //@no-rustfix: overlapping suggestions -#![allow(clippy::no_effect, clippy::useless_vec, unused)] +#![allow(clippy::temporary_assignment, clippy::no_effect, clippy::useless_vec, unused)] #![warn(clippy::single_range_in_vec_init)] #![feature(generic_arg_infer)] #[macro_use] extern crate proc_macros; +use std::ops::Range; + macro_rules! a { () => { vec![0..200]; @@ -21,6 +23,8 @@ fn awa_vec(start: T, end: T) { vec![start..end]; } +fn do_not_lint_as_argument(_: Vec>) {} + fn main() { // Lint [0..200]; @@ -34,11 +38,33 @@ fn main() { // Only suggest collect [0..200isize]; vec![0..200isize]; + // Lints, but suggests adding type annotations + let x = vec![0..200]; + do_not_lint_as_argument(vec![0..200]); // Do not lint [0..200, 0..100]; vec![0..200, 0..100]; [0.0..200.0]; vec![0.0..200.0]; + // Issue #11086 + let do_not_lint_if_has_type_annotations: Vec> = vec![0..200]; + // https://github.com/rust-lang/rust-clippy/issues/11086#issuecomment-1636996525 + struct DoNotLintStructInitializersUnit(Vec>); + struct DoNotLintStructInitializersNamed { + a: Vec>, + }; + enum DoNotLintEnums { + One(Vec>), + Two(Vec>), + } + DoNotLintStructInitializersUnit(vec![0..200]).0 = vec![0..200]; + DoNotLintStructInitializersNamed { a: vec![0..200] }.a = vec![0..200]; + let o = DoNotLintEnums::One(vec![0..200]); + match o { + DoNotLintEnums::One(mut e) => e = vec![0..200], + _ => todo!(), + } + do_not_lint_as_argument(vec![0..200]); // `Copy` is not implemented for `Range`, so this doesn't matter // FIXME: [0..200; 2]; // FIXME: [vec!0..200; 2]; diff --git a/tests/ui/single_range_in_vec_init.stderr b/tests/ui/single_range_in_vec_init.stderr index 9c125adb51a7..f233d59de5ac 100644 --- a/tests/ui/single_range_in_vec_init.stderr +++ b/tests/ui/single_range_in_vec_init.stderr @@ -1,5 +1,5 @@ error: an array of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:26:5 + --> tests/ui/single_range_in_vec_init.rs:30:5 | LL | [0..200]; | ^^^^^^^^ @@ -16,7 +16,7 @@ LL | [0; 200]; | ~~~~~~ error: a `Vec` of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:27:5 + --> tests/ui/single_range_in_vec_init.rs:31:5 | LL | vec![0..200]; | ^^^^^^^^^^^^ @@ -31,7 +31,7 @@ LL | vec![0; 200]; | ~~~~~~ error: an array of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:28:5 + --> tests/ui/single_range_in_vec_init.rs:32:5 | LL | [0u8..200]; | ^^^^^^^^^^ @@ -46,7 +46,7 @@ LL | [0u8; 200]; | ~~~~~~~~ error: an array of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:29:5 + --> tests/ui/single_range_in_vec_init.rs:33:5 | LL | [0usize..200]; | ^^^^^^^^^^^^^ @@ -61,7 +61,7 @@ LL | [0usize; 200]; | ~~~~~~~~~~~ error: an array of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:30:5 + --> tests/ui/single_range_in_vec_init.rs:34:5 | LL | [0..200usize]; | ^^^^^^^^^^^^^ @@ -76,7 +76,7 @@ LL | [0; 200usize]; | ~~~~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:31:5 + --> tests/ui/single_range_in_vec_init.rs:35:5 | LL | vec![0u8..200]; | ^^^^^^^^^^^^^^ @@ -91,7 +91,7 @@ LL | vec![0u8; 200]; | ~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:32:5 + --> tests/ui/single_range_in_vec_init.rs:36:5 | LL | vec![0usize..200]; | ^^^^^^^^^^^^^^^^^ @@ -106,7 +106,7 @@ LL | vec![0usize; 200]; | ~~~~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:33:5 + --> tests/ui/single_range_in_vec_init.rs:37:5 | LL | vec![0..200usize]; | ^^^^^^^^^^^^^^^^^ @@ -121,7 +121,7 @@ LL | vec![0; 200usize]; | ~~~~~~~~~~~ error: an array of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:35:5 + --> tests/ui/single_range_in_vec_init.rs:39:5 | LL | [0..200isize]; | ^^^^^^^^^^^^^ @@ -132,7 +132,7 @@ LL | (0..200isize).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> tests/ui/single_range_in_vec_init.rs:36:5 + --> tests/ui/single_range_in_vec_init.rs:40:5 | LL | vec![0..200isize]; | ^^^^^^^^^^^^^^^^^ @@ -142,5 +142,24 @@ help: if you wanted a `Vec` that contains the entire range, try LL | (0..200isize).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 10 previous errors +error: a `Vec` of `Range` that is only one element + --> tests/ui/single_range_in_vec_init.rs:42:13 + | +LL | let x = vec![0..200]; + | ^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains the entire range, try + | +LL | let x = (0..200).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted a `Vec` of len 200, try + | +LL | let x = vec![0; 200]; + | ~~~~~~ +help: if this is intended, give it type annotations + | +LL | let x: std::vec::Vec> = vec![0..200]; + | +++++++++++++++++++++++++++++++++++++ + +error: aborting due to 11 previous errors