Clippy: Clean remaining low-hanging fruit

[hseg]

These are about previously-unaddressed lints, but are mostly straightforward. The last two commits might be more controversial:

• Documenting the need for collect() is obvious, but works around that lint being fragile and giving false positives. The alternative is to keep that disabling around, but that seems noisier. I've marked the exception with #[expect] so that it will be flagged for removal once the lint is fixed.
• Rust 2018 changed the idiom to disallow using types with lifetimes without mentioning their lifetime parameters. All use sites in the codebase can be inferred, so setting the lifetime parameters there to '_ did the trick, but that does make the code a little more noisy. Still, the reminder might be useful, so I'd argue following the idiom makes sense.

Closes: #86 (this PR is rebased on that one, assuming both will be merged, with #86 coming first)

[ltratt]

You are right: the last two are controversial :)

I think the collect one shows that warning is simply not useful, and we simply shouldn't try enabling it.

In terms of lifetime annotations, I don't know that there's any advantage to explicit <'_> in these cases, other than bloating the source code a bit? I am prepared to be convinced on the latter, though.

[hseg]

I think the *collect* one shows that warning is simply not useful, and we simply shouldn't try enabling it.

The issue is as previously that we generally have three options: - make rules opt-in rather than opt-out (would bloat Cargo.toml with all the rules we know we want to avoid) - opt out of the rule locally - opt out of the rule globally Given there is only one false positive, the cost difference between local and global opt outs is pretty much the same. Making it global misses out on the cases it is correct in, and makes explaining the opt out less salient. Plus, having the opt out spelled #[expect] as I did means you automatically will get a notification once the problem is fixed. Two supplementary solutions also present themselves: - figuring out a way to avoid the collect, eg by using more higher-order functions. A quick inspection suggests the filtering at the very least can be done in the iterator, and potentially the updates can as well. If so, we might sidestep the issue altogether. - yak-shaving and trying to fix the lint itself. It would be of independent interest to me, but I admit I'd rather not suspend progress here for it

In terms of lifetime annotations, I don't know that there's any advantage to explicit *<'_>* in these cases, other than bloating the source code a bit? I am prepared to be convinced on the latter, though.

Trying to weigh the pros and cons on this one is turning out to be quite the archeological rabbit hole, ran out of time to understand it for tonight. My reading list on this atm is https://users.rust-lang.org/t/implicit-lifetime-generic-arguments/98700/7 for historical context rust-lang/rust#91639 for why it is currently allow-by-default (but is implied by rist_2018_idioms, so we need to address it) rust-lang/rust#120808, and especially rust-lang/rust#91639 (comment) rust-lang/rust#120808 (comment) which if I'm reading them correctly might even imply that once this lint leaves PR hell, the changes I made might be judged unnecessary Overall, this suggests to me this is a tarball of a lint that _might_ be addressed if and when I get a better handle on it, but can be removed from this PR for the moment.

[ltratt]

Given there is only one false positive, the cost difference between local and global opt outs is pretty much the same.

Understood. I'm going to come down in favour of the global opt out, because if that same idiom crops up in the code, I don't think the author should be burdened by a false positive that they then have to investigate.

Overall, this suggests to me this is a tarball of a lint that might be addressed if and when I get a better handle on it, but can be removed from this PR for the moment.

FWIW, I agree.

OK, so then I suggest: let's remove the last two commits from this PR, add that global opt out, do a cargo fmt, then we're good to merge.

[hseg]

OK, done. FWIW, I believe the needless_collect lint can be reenabled once rust-lang/rust-clippy#6066 is fixed.

I also wrote up my understanding of the situation around elided_lifetimes_in_paths in the commit message and more briefly in Cargo.toml. It seems that once rust-lang/rust#120808 lands, we won't need to explicitly disable it anymore, since the cases it currently complains about will be allow-by-default under it.

[hseg]

I do still think I might rework the logic in the needless_collect case regardless, it does look like it could be written more iterator-heavily. But that's work for a future PR.

[hseg]

Was a bit busy last week, sorry for the delay.

[ltratt]

Thanks!