Skip to content

Commit

Permalink
Make single_range_in_vec_init ignore type annotations and fn arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Centri3
Centri3 committed Jul 3, 2023
1 parent a959061 commit 6bd8d00
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 15 deletions.
52 changes: 48 additions & 4 deletions clippy_lints/src/single_range_in_vec_init.rs
View file
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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]`
Expand All @@ -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())
Expand Down Expand Up @@ -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(..))
}
10 changes: 10 additions & 0 deletions tests/ui/single_range_in_vec_init.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#[macro_use]
extern crate proc_macros;

use std::ops::Range;

macro_rules! a {
() => {
vec![0..200];
Expand All @@ -20,6 +22,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 @@ -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<Range<_>> = 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];
Expand Down
41 changes: 30 additions & 11 deletions tests/ui/single_range_in_vec_init.stderr
View file
Original file line number Diff line number Diff line change
@@ -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];
| ^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^
Expand All @@ -131,7 +131,7 @@ LL | (0..200isize).collect::<std::vec::Vec<isize>>();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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];
| ^^^^^^^^^^^^^^^^^
Expand All @@ -141,5 +141,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
--> $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::<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

0 comments on commit 6bd8d00

Please sign in to comment.