chore: update rust to 1.76.0 and fix new clippy lints #1522
Conversation
Some of the lints were... kind of 🙄, but it's easier to appease the animated paperclip than to argue with it.
| @@ -164,7 +164,7 @@ fn verification_and_weights_for_verified_and_unverified_deals() { | |||
| data: verified_deal_1.piece_cid, | |||
| size: verified_deal_1.piece_size, | |||
| }, | |||
| s_response.activated.get(0).unwrap() | |||
There was a problem hiding this comment.
Clippy does not like get(0).
There was a problem hiding this comment.
.first() or .front() is
easier to read and has the same result
but, that's just like your opinion, man
There was a problem hiding this comment.
I generally agree. I switched it to indexing ([0]) where it made sense.
| @@ -173,7 +173,7 @@ fn verification_and_weights_for_verified_and_unverified_deals() { | |||
| data: verified_deal_2.piece_cid, | |||
| size: verified_deal_2.piece_size, | |||
| }, | |||
| s_response.activated.get(1).unwrap() | |||
There was a problem hiding this comment.
I fixed the others because this is simpler anyways.
| @@ -4982,11 +4982,11 @@ fn notify_pledge_changed(rt: &impl Runtime, pledge_delta: &TokenAmount) -> Resul | |||
|
|
|||
| fn get_claims( | |||
| rt: &impl Runtime, | |||
| ids: &Vec<ext::verifreg::ClaimID>, | |||
| ids: &[ext::verifreg::ClaimID], | |||
There was a problem hiding this comment.
Clippy does not like &Vec<u8>.
| @@ -628,7 +628,7 @@ fn extend_expiration2_drop_claims() { | |||
| let extension = 42 * rt.policy().wpost_proving_period; | |||
| let new_expiration = old_sector.expiration + extension; | |||
|
|
|||
| let claim_ids = vec![400, 500]; | |||
| let claim_ids = [400, 500]; | |||
There was a problem hiding this comment.
Clippy does not like "useless vecs".
| match self.hamt.for_each(|k, v| { | ||
| let key = K::from_bytes(k).context_code(ExitCode::USR_ILLEGAL_STATE, "invalid key")?; | ||
| f(key, v).map_err(|e| anyhow!(e)) | ||
| }) { |
There was a problem hiding this comment.
Clippy was unhappy about all the code in the match part of the match statement, so I just got rid of the match entirely.
There was a problem hiding this comment.
this is probably a good call though, I find this kind of complex conditional construct puts unnecessary pressure on my mental stack
| .collect() | ||
| invocs.iter().enumerate().fold(String::new(), |mut s, (i, invoc)| { | ||
| use std::fmt::Write; | ||
| let _ = writeln!(s, "{}: [{}:{}],", i, invoc.to, invoc.method); |
There was a problem hiding this comment.
clippy did not like creating lots of small strings, this new version will build a single string.
There was a problem hiding this comment.
When Clippy is making you replace a map+collect with a fold and macros, I think it has gone rather out of its swim lane!
There was a problem hiding this comment.
Well, I mean, it's replacing one macro with another. It's point is valid: calling format! repeatedly will allocate a lot of strings while repeatedly calling write! on the same string won't.
In this case, it doesn't matter. But this could definitely have a performance impact if this were performance critical code.
There was a problem hiding this comment.
Yeah but it's a linter, not a performance coach. It's supposed (IMO) to make the code more readable, not faster.
There was a problem hiding this comment.
It's supposed to teach you when there may be a "better" way to do things. I do wish there were a more elegant way to write this particular code, but the suggestion is valid.
There was a problem hiding this comment.
Think of it this way: rust is big and complicated, you likely wouldn't learn half the language or about half the new features without something looking over your shoulder and telling you when there's a better way to do something.
There was a problem hiding this comment.
I do wish there were a more elegant way to write this particular code
there is, if you resist the functional siren song
use std::fmt::Write;
let mut s = String::new();
for (i, invoc) in invocs.iter().enumerate() {
let _ = write!(&mut s, "{}: [{}:{}],\n", i, invoc.to, invoc.method);
}
ssimple, clear, maintainable
| .collect() | ||
| invocs.iter().enumerate().fold(String::new(), |mut s, (i, invoc)| { | ||
| use std::fmt::Write; | ||
| let _ = writeln!(s, "{}: [{}:{}],", i, invoc.to, invoc.method); |
There was a problem hiding this comment.
When Clippy is making you replace a map+collect with a fold and macros, I think it has gone rather out of its swim lane!
Basically:
In terms of implications? Yeah, we shouldn't use this in v13. But that branch has been cut so it should be safe to merge this to master. |
Some of the lints were... kind of 🙄, but it's easier to appease the animated paperclip than to argue with it.