Add #[must_use] to {HashMap,HashSet,BTreeMap,BTreeSet}::insert

[safinaskar]

I propose adding #[must_use] to methods insert of HashMap, HashSet, BTreeMap, BTreeSet.

Because in my code I always want to check whether such key existed before. And it is easy to forget such check. My current workaround is so: I forbid direct calls to HashMap::insert and similar functions using clippy::disallowed_methods and wrap this functions using wrappers with #[must_use]. I propose simply to add #[must_use] to std.

Yes, I am aware about try_insert ( #82766 ). But this functions is misnamed: prefix try_ is usually used for functions, which can report allocation error, such as HashMap::try_reserve. Also, this doesn't fix the problem for HashSet

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information.

[veber-alex]

I think this is going to be very annoying for a lot of people, I personally never needed the return value of insert.
A lot of code will need to start doing:

let _ = map.insert(1, 2);

or disable must_use all together.

[ChrisJefferson]

Have you considered seeing if something like this could added as an (optional) clippy?

I agree with @veber-alex -- I just looked and I have many inserts, and have never checked the return value of any of them. I personally feel [must_use] should be reserved for cases where without using the return value the function simply doesn't do anything useful, which doesn't apply in this case.

[joshtriplett]

I agree as well: we use #[must_use] in cases that very nearly can't have false positives, by the nature of the method. There's no point in calling len if you don't save the length; there's absolutely a valid use case for calling insert and not caring if the item was already present.

An optional (allow-by-default) clippy lint might be a good idea, for projects that want to enforce this.