From 6bd8d0041944fde7fbc67d04d6e251bffb8f7613 Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Mon, 3 Jul 2023 08:57:05 -0500 Subject: [PATCH] Make `single_range_in_vec_init` ignore type annotations and fn arguments --- clippy_lints/src/single_range_in_vec_init.rs | 52 ++++++++++++++++++-- tests/ui/single_range_in_vec_init.rs | 10 ++++ tests/ui/single_range_in_vec_init.stderr | 41 ++++++++++----- 3 files changed, 88 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/single_range_in_vec_init.rs b/clippy_lints/src/single_range_in_vec_init.rs index dfe8be7a6a61..ab06f95b8c4b 100644 --- a/clippy_lints/src/single_range_in_vec_init.rs +++ b/clippy_lints/src/single_range_in_vec_init.rs @@ -1,10 +1,10 @@ use clippy_utils::{ - diagnostics::span_lint_and_then, get_trait_def_id, higher::VecArgs, macros::root_macro_call_first_node, - source::snippet_opt, ty::implements_trait, + diagnostics::span_lint_and_then, get_parent_expr, get_trait_def_id, higher::VecArgs, + macros::root_macro_call_first_node, peel_blocks, source::snippet_opt, ty::implements_trait, }; 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, declare_tool_lint}; use std::fmt::{self, Display, Formatter}; @@ -66,7 +66,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]` @@ -90,6 +90,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()) @@ -139,9 +140,52 @@ 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. We should not lint this. +fn is_argument(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let Some(parent) = get_parent_expr(cx, expr) else { + return false; + }; + + matches!(parent.kind, ExprKind::Call(_, _) | ExprKind::MethodCall(..)) +} diff --git a/tests/ui/single_range_in_vec_init.rs b/tests/ui/single_range_in_vec_init.rs index 833e1c43bfc0..96aaedb69bc3 100644 --- a/tests/ui/single_range_in_vec_init.rs +++ b/tests/ui/single_range_in_vec_init.rs @@ -6,6 +6,8 @@ #[macro_use] extern crate proc_macros; +use std::ops::Range; + macro_rules! a { () => { vec![0..200]; @@ -20,6 +22,8 @@ fn awa_vec(start: T, end: T) { vec![start..end]; } +fn do_not_lint_as_argument(_: Vec>) {} + fn main() { // Lint [0..200]; @@ -33,11 +37,17 @@ 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]; + 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 3e3d521f4a50..a0771b159db3 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 - --> $DIR/single_range_in_vec_init.rs:25:5 + --> $DIR/single_range_in_vec_init.rs:29:5 | LL | [0..200]; | ^^^^^^^^ @@ -15,7 +15,7 @@ LL | [0; 200]; | ~~~~~~ error: a `Vec` of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:26:5 + --> $DIR/single_range_in_vec_init.rs:30:5 | LL | vec![0..200]; | ^^^^^^^^^^^^ @@ -30,7 +30,7 @@ LL | vec![0; 200]; | ~~~~~~ error: an array of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:27:5 + --> $DIR/single_range_in_vec_init.rs:31:5 | LL | [0u8..200]; | ^^^^^^^^^^ @@ -45,7 +45,7 @@ LL | [0u8; 200]; | ~~~~~~~~ error: an array of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:28:5 + --> $DIR/single_range_in_vec_init.rs:32:5 | LL | [0usize..200]; | ^^^^^^^^^^^^^ @@ -60,7 +60,7 @@ LL | [0usize; 200]; | ~~~~~~~~~~~ error: an array of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:29:5 + --> $DIR/single_range_in_vec_init.rs:33:5 | LL | [0..200usize]; | ^^^^^^^^^^^^^ @@ -75,7 +75,7 @@ LL | [0; 200usize]; | ~~~~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:30:5 + --> $DIR/single_range_in_vec_init.rs:34:5 | LL | vec![0u8..200]; | ^^^^^^^^^^^^^^ @@ -90,7 +90,7 @@ LL | vec![0u8; 200]; | ~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:31:5 + --> $DIR/single_range_in_vec_init.rs:35:5 | LL | vec![0usize..200]; | ^^^^^^^^^^^^^^^^^ @@ -105,7 +105,7 @@ LL | vec![0usize; 200]; | ~~~~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:32:5 + --> $DIR/single_range_in_vec_init.rs:36:5 | LL | vec![0..200usize]; | ^^^^^^^^^^^^^^^^^ @@ -120,7 +120,7 @@ LL | vec![0; 200usize]; | ~~~~~~~~~~~ error: an array of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:34:5 + --> $DIR/single_range_in_vec_init.rs:38:5 | LL | [0..200isize]; | ^^^^^^^^^^^^^ @@ -131,7 +131,7 @@ LL | (0..200isize).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: a `Vec` of `Range` that is only one element - --> $DIR/single_range_in_vec_init.rs:35:5 + --> $DIR/single_range_in_vec_init.rs:39:5 | LL | vec![0..200isize]; | ^^^^^^^^^^^^^^^^^ @@ -141,5 +141,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 + --> $DIR/single_range_in_vec_init.rs:41: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