Skip to content
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

Suggest using Vec::extend() in same_item_push #13987

Merged
merged 1 commit into from
Jan 22, 2025
Merged
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
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
* [`ptr_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr)
* [`redundant_field_names`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names)
* [`redundant_static_lifetimes`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes)
* [`same_item_push`](https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push)
* [`seek_from_current`](https://rust-lang.github.io/rust-clippy/master/index.html#seek_from_current)
* [`seek_rewind`](https://rust-lang.github.io/rust-clippy/master/index.html#seek_rewind)
* [`transmute_ptr_to_ref`](https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref)
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ define_Conf! {
ptr_as_ptr,
redundant_field_names,
redundant_static_lifetimes,
same_item_push,
seek_from_current,
seek_rewind,
transmute_ptr_to_ref,
Expand Down
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
34 changes: 23 additions & 11 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::diagnostics::span_lint_and_then;
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;

span_lint_and_help(
let secondary_help = if msrv.meets(msrvs::REPEAT_N)
&& let Some(std_or_core) = std_or_core(cx)
{
format!("or `{vec_str}.extend({std_or_core}::iter::repeat_n({item_str}, SIZE))`")
} else {
format!("or `{vec_str}.resize(NEW_SIZE, {item_str})`")
};

span_lint_and_then(
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})"),
"it looks like the same item is being pushed into this `Vec`",
|diag| {
diag.help(format!("consider using `vec![{item_str};SIZE]`"))
.help(secondary_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
20 changes: 15 additions & 5 deletions tests/ui/same_item_push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,33 +21,43 @@ fn main() {
let item = 2;
for _ in 5..=20 {
vec.push(item);
//~^ ERROR: it looks like the same item is being pushed into this Vec
//~^ ERROR: it looks like the same item is being pushed into this `Vec`
}

let mut vec: Vec<u8> = Vec::new();
for _ in 0..15 {
let item = 2;
vec.push(item);
//~^ ERROR: it looks like the same item is being pushed into this Vec
//~^ ERROR: it looks like the same item is being pushed into this `Vec`
}

let mut vec: Vec<u8> = Vec::new();
for _ in 0..15 {
vec.push(13);
//~^ ERROR: it looks like the same item is being pushed into this Vec
//~^ ERROR: it looks like the same item is being pushed into this `Vec`
}

let mut vec = Vec::new();
for _ in 0..20 {
vec.push(VALUE);
//~^ ERROR: it looks like the same item is being pushed into this Vec
//~^ ERROR: it looks like the same item is being pushed into this `Vec`
}

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
//~^ ERROR: it looks like the same item is being pushed into this `Vec`
}

#[clippy::msrv = "1.81"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure to also add the lint to the configuration:

use_self,
)]
msrv: Msrv = Msrv::empty(),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always forget that users may configure the msrv in Clippy as well, thanks for the reminder!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want a project, you can try to find a way to do this automatically. This has been something like a holly grail for some time. ^^

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 **
Expand Down
36 changes: 25 additions & 11 deletions tests/ui/same_item_push.stderr
Original file line number Diff line number Diff line change
@@ -1,44 +1,58 @@
error: it looks like the same item is being pushed into this Vec
error: it looks like the same item is being pushed into this `Vec`
--> tests/ui/same_item_push.rs:23:9
|
LL | vec.push(item);
| ^^^
|
= help: consider using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
= help: consider using `vec![item;SIZE]`
= help: 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)]`

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

error: it looks like the same item is being pushed into this Vec
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]`
= help: or `vec.extend(std::iter::repeat_n(13, SIZE))`

error: it looks like the same item is being pushed into this Vec
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]`
= help: or `vec.extend(std::iter::repeat_n(VALUE, SIZE))`

error: it looks like the same item is being pushed into this Vec
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.resize(NEW_SIZE, item)
= help: consider using `vec![item;SIZE]`
= help: or `vec.extend(std::iter::repeat_n(item, SIZE))`

error: aborting due to 5 previous errors
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]`
= help: or `vec.resize(NEW_SIZE, item)`

error: aborting due to 6 previous errors

Loading