rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

new lint: `vec.drain(..)` instead of `vec.clear()`

Open philpax opened this issue 2 years ago • 3 comments

What it does

Checks for usage of vec.drain(..) (iterator dropped immediately) which can be replaced with vec.clear()

Lint Name

unnecessary_vec_drain

Category

style

Advantage

  • Makes intent clearer
  • Marginally more efficient, especially in debug builds

Drawbacks

No response

Example

vec.drain(..);

Could be written as:

vec.clear();

philpax avatar Aug 16 '22 02:08 philpax

@rustbot claim

carloosaf avatar Aug 18 '22 11:08 carloosaf

@rustbot claim

koka831 avatar Aug 22 '22 14:08 koka831

@rustbot claim

lana99 avatar Sep 07 '22 08:09 lana99

Hi @lana99, are you working on this or is this up for grabs?

czotomo avatar Oct 06 '22 21:10 czotomo

I’ve been busy but I will probably push code for the issue this weekend. @czotomo

lana99 avatar Oct 06 '22 23:10 lana99

Hi @lana99, I don't know if you are still working on this but as I was looking at #9623 I was thinking that the example in the documentation you provided might be improved from this:

let mut vec: Vec<i32> = Vec::new();
vec.drain(..);

to this:

let mut v = vec![0, 1, 2];
v.drain(..);

to make it conform to the examples of the Drain documentation and the Vec documentation.

Sorry if this isn't the place to discuss this, and I don't mean to be nosy, I was just trying to learn from other people's PR how to contribute and yours caught my attention :slightly_smiling_face:

bluthej avatar Mar 04 '23 10:03 bluthej

Ah great, im not working on it anymore , so its up for grabs :)

lana99 avatar Mar 04 '23 13:03 lana99

@flip1995 I would like to pick up where @lana99 left off, but first I would like suggest renaming the lint. I looked at #9623 and saw that you suggested renaming UNNECESSARY_VEC_DRAIN to NEEDLESS_VEC_DRAIN, but after thumbing through the naming guidelines and the existing lints, I came up with the following proposals (I prefer number 1 personally):

  1. clear_with_drain: It's nice because it's consistent with iter_with_drain (and also get_last_with_len) and it expresses well what the person is doing, i.e. clearing a vector with a call to drain
  2. drain_instead_of_clear: Also expresses well what the person is doing and consistent with lints like from_iter_instead_of_collect, however it's a bit less concise

My point is, while looking at existing lints it occurred to me that unnecessary_... lints tend to deal with something that should be removed entirely because it is not needed. Here, the call to drain is needed in order to clear the vector, it's just that it's not the best way to do it.

Any thoughts?

bluthej avatar Mar 11 '23 11:03 bluthej

@rustbot claim

bluthej avatar Mar 18 '23 12:03 bluthej

Hi @philpax, I started working on this but I'm a newbie and I have some questions :) (on the lint itself, not necessarily on how to contribute).

Do you know of anyone who might have some time to mentor me? The active members link in CONTRIBUTING.md seems to be pointing to a file that does not exist anymore, so I am to sure who I can contact.

Cheers

bluthej avatar Mar 18 '23 12:03 bluthej

I'm afraid not - I'm just a user myself! I'd suggest going to the #clippy channel of the Zulip and asking there.

philpax avatar Mar 18 '23 20:03 philpax

Oh I see! I'll do that, thank you 🙂

bluthej avatar Mar 19 '23 00:03 bluthej