Skip to content

Make single_range_in_vec_init ignore type annotations, fn arguments and ExprFields #12611

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
54 changes: 51 additions & 3 deletions clippy_lints/src/single_range_in_vec_init.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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]`
Expand All @@ -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())
Expand Down Expand Up @@ -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(_))
}
28 changes: 27 additions & 1 deletion tests/ui/single_range_in_vec_init.rs
Original file line number Diff line number Diff line change
@@ -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];
Expand All @@ -21,6 +23,8 @@ fn awa_vec<T: PartialOrd>(start: T, end: T) {
vec![start..end];
}

fn do_not_lint_as_argument(_: Vec<Range<usize>>) {}

fn main() {
// Lint
[0..200];
Expand All @@ -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<Range<_>> = vec![0..200];
// https://github.com/rust-lang/rust-clippy/issues/11086#issuecomment-1636996525
struct DoNotLintStructInitializersUnit(Vec<Range<usize>>);
struct DoNotLintStructInitializersNamed {
a: Vec<Range<usize>>,
};
enum DoNotLintEnums {
One(Vec<Range<usize>>),
Two(Vec<Range<usize>>),
}
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];
Expand Down
41 changes: 30 additions & 11 deletions tests/ui/single_range_in_vec_init.stderr
Original file line number Diff line number Diff line change
@@ -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];
| ^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^
Expand All @@ -132,7 +132,7 @@ LL | (0..200isize).collect::<std::vec::Vec<isize>>();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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];
| ^^^^^^^^^^^^^^^^^
Expand All @@ -142,5 +142,24 @@ help: if you wanted a `Vec` that contains the entire range, try
LL | (0..200isize).collect::<std::vec::Vec<isize>>();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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::<std::vec::Vec<i32>>();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
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<std::ops::Range<i32>> = vec![0..200];
| +++++++++++++++++++++++++++++++++++++

error: aborting due to 11 previous errors