Format, migrate to 2024 Edition, and fix remaining clippy lints #31
Conversation
I was originally perplexed as to why the crate wasn't already formatted, but quickly found out why... Though the PR seems to have languished for some time, I've run a patched version of rustfmt that completes without crashing: rust-lang/rustfmt#6396
This is a strange case — I had to look this up myself! Because you have a `#[macro_use]` above the `params` module in `lib.rs`, all of the macros in `params.rs` are automatically made available crate-wide. This import of the macro in `scan_properties.rs` is therefore redundant! The same logic applies to `impl_param_described`. https://lukaswirth.dev/tlborm/decl-macros/minutiae/scoping.html
For `enum`s, you can add a tag and `#[derive(Default)]` instead of manually implementing the same code: https://doc.rust-lang.org/std/default/trait.Default.html#enums
NOTE: This method was added in a recent version of Rust (1.87.0), so changing to this code means that our minimum supported Rust version would become that! If you'd rather keep compatibility with older-than-stable versions of Rust, we should leave this unchanged!
Performs some automatic migration to the 2024 edition of Rust. Will be followed by a cleanup commit.
This is now a different, newer PR that supports the 2024 edition: rust-lang/rustfmt#6630
The 2024 edition changed `expr`s to accept `const` and `_` expressions, but because no macros in `mzdata` included any `const` or `_` keywords in their syntax, no change needs to be made.
None of the migrated code seemed to depend on any exact `Drop` timing, so the overly-cautious `if let` -> `match` migration was unnecessary.
In the 2024 edition, `gen` became a reserved Rust keyword. Using `r#gen` is still possible, but probably not best practice, so I've renamed the old `gen` variables.
|
How much of this is automation from Clippy and how much is manually fixing things it flagged? I'm not inclined to change the edition of the library as that impacts the minimal compatible Rust versions. |
|
@mobiusklein It's largely automated fixes, though I did manually review many of the edition changes and changed them back after determining the automated tool was overly cautious! If the MSRV is a concern, then I'll split off everything after I'll use |
|
Okay, running The MSRV is unchanged by everything up until Bumping to the 2025 edition would move the MSRV from 1.83.0 to 1.85.0, and fixing the Let me know how you'd like to proceed! Happy to stick with 1.83.0, bump to 1.85.0, or all the way to 1.87.0 at your preference! EDIT: And if older support is really important, then we can do something about that 1.82.0 failure and move to an even older version before locking that MSRV into the |
|
No, that's fine with me. I wasn't aware I was using recent features here. Yes, my attempts to do generic programming to the extreme with macros cause the code to run afoul of that formatter problem. |
|
I decided to revert this after seeing the havoc it caused for my tooling for what are effectively cosmetic changes. I will need to check this with other users to see what they think. |
|
Oh dear! What sort of havoc for which tools? That way I can split off something still that just silences warnings and we can keep the more disruptive stuff in waiting as a PR. |
2 participants
There is a lot here, and it's a massive diff on account of the
cargo fmt(which I needed to use this fork for: rust-lang/rustfmt#6630, after running into your same rustfmt issue), but I've broken things out into (hopefully) very self-contained commits.Everything up until the
style: clippy fix manual_is_multiple_ofcommit should be completely non-disruptive, but after that a minimum of Rust 1.87.0 is required. If that's an issue, we can easily save he rest of the changes for later.Let me know if you'd like me break this up into several PRs! I'm very glad you've put so much hard work into this project — it's intimidating how much goes on in these MS files!