Skip to content

Commit

Permalink
Suggest using Vec::extend() in same_item_push
Browse files Browse the repository at this point in the history
Using `Vec::extend(std::iter::repeat_n(item, N))` allows to use the more
natural number of elements to add `N`, as is probably done in the original
loop, instead of computing the difference between the existing number of
elements and the wanted one.

Before MSRV 1.82, the older suggestion to use `Vec::resize()` is still
issued.
  • Loading branch information
samueltardieu committed Jan 11, 2025
1 parent ab55d3f commit b90338f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 13 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ impl Loops {
for_kv_map::check(cx, pat, arg, body);
mut_range_bound::check(cx, arg, body);
single_element_loop::check(cx, pat, arg, body, expr);
same_item_push::check(cx, pat, arg, body, expr);
same_item_push::check(cx, pat, arg, body, expr, &self.msrv);
manual_flatten::check(cx, pat, arg, body, span);
manual_find::check(cx, pat, arg, body, span, expr);
unused_enumerate_index::check(cx, pat, arg, body);
Expand Down
26 changes: 19 additions & 7 deletions clippy_lints/src/loops/same_item_push.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use super::SAME_ITEM_PUSH;
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::path_to_local;
use clippy_utils::msrvs::Msrv;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{msrvs, path_to_local, std_or_core};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
Expand All @@ -19,19 +20,30 @@ pub(super) fn check<'tcx>(
_: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
_: &'tcx Expr<'_>,
msrv: &Msrv,
) {
fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>, ctxt: SyntaxContext) {
fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>, ctxt: SyntaxContext, msrv: &Msrv) {
let mut app = Applicability::Unspecified;
let vec_str = snippet_with_context(cx, vec.span, ctxt, "", &mut app).0;
let item_str = snippet_with_context(cx, pushed_item.span, ctxt, "", &mut app).0;

let help = if msrv.meets(msrvs::REPEAT_N)
&& let Some(std_or_core) = std_or_core(cx)
{
format!(
"consider using vec![{item_str};SIZE] or {vec_str}.extend({std_or_core}::iter::repeat_n({item_str}, SIZE))"
)
} else {
format!("consider using vec![{item_str};SIZE] or {vec_str}.resize(NEW_SIZE, {item_str})")
};

span_lint_and_help(
cx,
SAME_ITEM_PUSH,
vec.span,
"it looks like the same item is being pushed into this Vec",
None,
format!("consider using vec![{item_str};SIZE] or {vec_str}.resize(NEW_SIZE, {item_str})"),
help,
);
}

Expand Down Expand Up @@ -67,23 +79,23 @@ pub(super) fn check<'tcx>(
{
match init.kind {
// immutable bindings that are initialized with literal
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt),
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt, msrv),
// immutable bindings that are initialized with constant
ExprKind::Path(ref path) => {
if let Res::Def(DefKind::Const, ..) = cx.qpath_res(path, init.hir_id) {
emit_lint(cx, vec, pushed_item, ctxt);
emit_lint(cx, vec, pushed_item, ctxt, msrv);
}
},
_ => {},
}
}
},
// constant
Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item, ctxt),
Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item, ctxt, msrv),
_ => {},
}
},
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt),
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt, msrv),
_ => {},
}
}
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/same_item_push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ fn main() {
//~^ ERROR: it looks like the same item is being pushed into this Vec
}

#[clippy::msrv = "1.81"]
fn older_msrv() {
let mut vec = Vec::new();
let item = VALUE;
for _ in 0..20 {
vec.push(item);
//~^ ERROR: it looks like the same item is being pushed into this Vec
}
}

// ** non-linted cases **
let mut spaces = Vec::with_capacity(10);
for _ in 0..10 {
Expand Down
18 changes: 13 additions & 5 deletions tests/ui/same_item_push.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: it looks like the same item is being pushed into this Vec
LL | vec.push(item);
| ^^^
|
= help: consider using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
= help: consider using vec![item;SIZE] or vec.extend(std::iter::repeat_n(item, SIZE))
= note: `-D clippy::same-item-push` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::same_item_push)]`

Expand All @@ -14,31 +14,39 @@ error: it looks like the same item is being pushed into this Vec
LL | vec.push(item);
| ^^^
|
= help: consider using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
= help: consider using vec![item;SIZE] or vec.extend(std::iter::repeat_n(item, SIZE))

error: it looks like the same item is being pushed into this Vec
--> tests/ui/same_item_push.rs:36:9
|
LL | vec.push(13);
| ^^^
|
= help: consider using vec![13;SIZE] or vec.resize(NEW_SIZE, 13)
= help: consider using vec![13;SIZE] or vec.extend(std::iter::repeat_n(13, SIZE))

error: it looks like the same item is being pushed into this Vec
--> tests/ui/same_item_push.rs:42:9
|
LL | vec.push(VALUE);
| ^^^
|
= help: consider using vec![VALUE;SIZE] or vec.resize(NEW_SIZE, VALUE)
= help: consider using vec![VALUE;SIZE] or vec.extend(std::iter::repeat_n(VALUE, SIZE))

error: it looks like the same item is being pushed into this Vec
--> tests/ui/same_item_push.rs:49:9
|
LL | vec.push(item);
| ^^^
|
= help: consider using vec![item;SIZE] or vec.extend(std::iter::repeat_n(item, SIZE))

error: it looks like the same item is being pushed into this Vec
--> tests/ui/same_item_push.rs:58:13
|
LL | vec.push(item);
| ^^^
|
= help: consider using vec![item;SIZE] or vec.resize(NEW_SIZE, item)

error: aborting due to 5 previous errors
error: aborting due to 6 previous errors

0 comments on commit b90338f

Please sign in to comment.