Skip to content

Commit a8a8e98

Browse files
ada4aada4a
committed
clean-up
- move `is_allowed_vec_method` (a stripped-down version of it, anyway, as the function doesn't make sense as is out of context) to utils, as it's shared between `useless_vec` and `ptr_arg` - add another test for non-standard macro brace case - rm unneeded `allow`s - rm duplicated tests - add comments to some tests
1 parent 7534814 commit a8a8e98

File tree

8 files changed

+85
-104
lines changed

8 files changed

+85
-104
lines changed

‎clippy_lints/src/ptr/ptr_arg.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::PTR_ARG;
22
use clippy_utils::diagnostics::span_lint_hir_and_then;
33
use clippy_utils::res::MaybeResPath;
44
use clippy_utils::source::SpanRangeExt;
5-
use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, sym};
5+
use clippy_utils::{VEC_METHODS_SHADOWING_SLICE_METHODS, get_expr_use_or_unification_node, is_lint_allowed, sym};
66
use hir::LifetimeKind;
77
use rustc_abi::ExternAbi;
88
use rustc_errors::Applicability;
@@ -23,8 +23,6 @@ use rustc_trait_selection::infer::InferCtxtExt as _;
2323
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
2424
use std::{fmt, iter};
2525

26-
use crate::useless_vec::is_allowed_vec_method;
27-
2826
pub(super) fn check_body<'tcx>(
2927
cx: &LateContext<'tcx>,
3028
body: &Body<'tcx>,
@@ -383,7 +381,7 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &Body<'tcx>, args: &[
383381
// Some methods exist on both `[T]` and `Vec<T>`, such as `len`, where the receiver type
384382
// doesn't coerce to a slice and our adjusted type check below isn't enough,
385383
// but it would still be valid to call with a slice
386-
if is_allowed_vec_method(use_expr) {
384+
if VEC_METHODS_SHADOWING_SLICE_METHODS.contains(&name) {
387385
return;
388386
}
389387
}

‎clippy_lints/src/useless_vec.rs‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use clippy_utils::msrvs::{self, Msrv};
1010
use clippy_utils::source::SpanRangeExt;
1111
use clippy_utils::ty::is_copy;
1212
use clippy_utils::visitors::for_each_local_use_after_expr;
13-
use clippy_utils::{get_parent_expr, higher, is_in_test, span_contains_comment, sym};
13+
use clippy_utils::{VEC_METHODS_SHADOWING_SLICE_METHODS, get_parent_expr, higher, is_in_test, span_contains_comment};
1414
use rustc_errors::Applicability;
1515
use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, LetStmt, Mutability, Node, Pat, PatKind};
1616
use rustc_lint::{LateContext, LateLintPass};
@@ -123,8 +123,16 @@ impl UselessVec {
123123
// allow indexing into a vec and some set of allowed method calls that exist on slices, too
124124
if let Some(parent) = get_parent_expr(cx, expr)
125125
&& (adjusts_to_slice(cx, expr)
126-
|| matches!(parent.kind, ExprKind::Index(..))
127-
|| is_allowed_vec_method(parent))
126+
|| match parent.kind {
127+
ExprKind::Index(..) => true,
128+
ExprKind::MethodCall(path, _, [], _) => {
129+
// If the given expression is a method call to a `Vec` method that also exists on
130+
// slices, it means that this expression does not actually require a `Vec` and could
131+
// just work with an array.
132+
VEC_METHODS_SHADOWING_SLICE_METHODS.contains(&path.ident.name)
133+
},
134+
_ => false,
135+
})
128136
{
129137
ControlFlow::Continue(())
130138
} else {
@@ -144,8 +152,9 @@ impl UselessVec {
144152
VecToArray::Impossible
145153
},
146154
// search for `for _ in vec![...]`
147-
Node::Expr(Expr { span, .. })
148-
if span.is_desugaring(DesugaringKind::ForLoop) && self.msrv.meets(cx, msrvs::ARRAY_INTO_ITERATOR) =>
155+
Node::Expr(expr)
156+
if expr.span.is_desugaring(DesugaringKind::ForLoop)
157+
&& self.msrv.meets(cx, msrvs::ARRAY_INTO_ITERATOR) =>
149158
{
150159
VecToArray::Possible
151160
},
@@ -276,9 +285,8 @@ impl SuggestedType {
276285
assert!(args_span.is_none_or(|s| !s.from_expansion()));
277286
assert!(len_span.is_none_or(|s| !s.from_expansion()));
278287

279-
let maybe_args = args_span
280-
.map(|sp| sp.get_source_text(cx).expect("spans are always crate-local"))
281-
.map_or(String::new(), |x| x.to_owned());
288+
let maybe_args = args_span.map(|sp| sp.get_source_text(cx).expect("spans are always crate-local"));
289+
let maybe_args = maybe_args.as_deref().unwrap_or_default();
282290
let maybe_len = len_span
283291
.map(|sp| sp.get_source_text(cx).expect("spans are always crate-local"))
284292
.map(|st| format!("; {st}"))
@@ -301,17 +309,6 @@ fn adjusts_to_slice(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
301309
matches!(cx.typeck_results().expr_ty_adjusted(e).kind(), ty::Ref(_, ty, _) if ty.is_slice())
302310
}
303311

304-
/// Checks if the given expression is a method call to a `Vec` method
305-
/// that also exists on slices. If this returns true, it means that
306-
/// this expression does not actually require a `Vec` and could just work with an array.
307-
pub fn is_allowed_vec_method(e: &Expr<'_>) -> bool {
308-
if let ExprKind::MethodCall(path, _, [], _) = e.kind {
309-
matches!(path.ident.name, sym::as_ptr | sym::is_empty | sym::len)
310-
} else {
311-
false
312-
}
313-
}
314-
315312
fn suggest_type(expr: &Expr<'_>) -> SuggestedType {
316313
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, _) = expr.kind {
317314
// `expr` is `&vec![_]`, so suggest `&[_]` (or `&mut[_]` resp.)

‎clippy_utils/src/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ use crate::res::{MaybeDef, MaybeQPath, MaybeResPath};
136136
use crate::ty::{adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type};
137137
use crate::visitors::for_each_expr_without_closures;
138138

139+
/// Methods on `Vec` that also exists on slices.
140+
pub const VEC_METHODS_SHADOWING_SLICE_METHODS: [Symbol; 3] = [sym::as_ptr, sym::is_empty, sym::len];
141+
139142
#[macro_export]
140143
macro_rules! extract_msrv_attr {
141144
() => {

‎tests/ui/useless_vec.fixed‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#![warn(clippy::useless_vec)]
2-
#![allow(clippy::nonstandard_macro_braces, clippy::uninlined_format_args, unused)]
32

43
use std::rc::Rc;
54

@@ -39,17 +38,14 @@ fn main() {
3938
on_mut_slice(&mut [1, 2]);
4039
//~^ useless_vec
4140

42-
on_slice(&[1, 2]);
43-
//~^ useless_vec
44-
on_slice(&[1, 2]);
45-
on_mut_slice(&mut [1, 2]);
46-
//~^ useless_vec
4741
#[rustfmt::skip]
48-
on_slice(&[1, 2]);
49-
//~^ useless_vec
50-
on_slice(&[1, 2]);
51-
on_mut_slice(&mut [1, 2]);
52-
//~^ useless_vec
42+
#[allow(clippy::nonstandard_macro_braces)] // not an `expect` as it will only lint _before_ the fix
43+
{
44+
on_slice(&[1, 2]);
45+
//~^ useless_vec
46+
on_mut_slice(&mut [1, 2]);
47+
//~^ useless_vec
48+
};
5349

5450
on_slice(&[1; 2]);
5551
//~^ useless_vec
@@ -75,22 +71,24 @@ fn main() {
7571
on_vec(&vec![1; 201]); // Ok, size of `vec` higher than `too_large_for_stack`
7672
on_mut_vec(&mut vec![1; 201]); // Ok, size of `vec` higher than `too_large_for_stack`
7773

78-
// Ok
74+
// Ok, size of `vec` higher than `too_large_for_stack`
7975
for a in vec![1; 201] {
80-
println!("{:?}", a);
76+
println!("{a:?}");
8177
}
8278

8379
// https://github.com/rust-lang/rust-clippy/issues/2262#issuecomment-783979246
8480
let _x: i32 = [1, 2, 3].iter().sum();
8581
//~^ useless_vec
8682

87-
// Do lint
88-
let mut x = [1, 2, 3];
89-
//~^ useless_vec
90-
x.fill(123);
91-
dbg!(x[0]);
92-
dbg!(x.len());
93-
dbg!(x.iter().sum::<i32>());
83+
// Do lint, only used as slice
84+
{
85+
let mut x = [1, 2, 3];
86+
//~^ useless_vec
87+
x.fill(123);
88+
dbg!(x[0]);
89+
dbg!(x.len());
90+
dbg!(x.iter().sum::<i32>());
91+
}
9492

9593
let _x: &[i32] = &[1, 2, 3];
9694
//~^ useless_vec

‎tests/ui/useless_vec.rs‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#![warn(clippy::useless_vec)]
2-
#![allow(clippy::nonstandard_macro_braces, clippy::uninlined_format_args, unused)]
32

43
use std::rc::Rc;
54

@@ -39,17 +38,14 @@ fn main() {
3938
on_mut_slice(&mut vec![1, 2]);
4039
//~^ useless_vec
4140

42-
on_slice(&vec![1, 2]);
43-
//~^ useless_vec
44-
on_slice(&[1, 2]);
45-
on_mut_slice(&mut vec![1, 2]);
46-
//~^ useless_vec
4741
#[rustfmt::skip]
48-
on_slice(&vec!(1, 2));
49-
//~^ useless_vec
50-
on_slice(&[1, 2]);
51-
on_mut_slice(&mut vec![1, 2]);
52-
//~^ useless_vec
42+
#[allow(clippy::nonstandard_macro_braces)] // not an `expect` as it will only lint _before_ the fix
43+
{
44+
on_slice(&vec!(1, 2));
45+
//~^ useless_vec
46+
on_mut_slice(&mut vec!(1, 2));
47+
//~^ useless_vec
48+
};
5349

5450
on_slice(&vec![1; 2]);
5551
//~^ useless_vec
@@ -75,22 +71,24 @@ fn main() {
7571
on_vec(&vec![1; 201]); // Ok, size of `vec` higher than `too_large_for_stack`
7672
on_mut_vec(&mut vec![1; 201]); // Ok, size of `vec` higher than `too_large_for_stack`
7773

78-
// Ok
74+
// Ok, size of `vec` higher than `too_large_for_stack`
7975
for a in vec![1; 201] {
80-
println!("{:?}", a);
76+
println!("{a:?}");
8177
}
8278

8379
// https://github.com/rust-lang/rust-clippy/issues/2262#issuecomment-783979246
8480
let _x: i32 = vec![1, 2, 3].iter().sum();
8581
//~^ useless_vec
8682

87-
// Do lint
88-
let mut x = vec![1, 2, 3];
89-
//~^ useless_vec
90-
x.fill(123);
91-
dbg!(x[0]);
92-
dbg!(x.len());
93-
dbg!(x.iter().sum::<i32>());
83+
// Do lint, only used as slice
84+
{
85+
let mut x = vec![1, 2, 3];
86+
//~^ useless_vec
87+
x.fill(123);
88+
dbg!(x[0]);
89+
dbg!(x.len());
90+
dbg!(x.iter().sum::<i32>());
91+
}
9492

9593
let _x: &[i32] = &vec![1, 2, 3];
9694
//~^ useless_vec

0 commit comments

Comments
 (0)