Skip to content

Conversation

@GlitchlessCode
Copy link
Contributor

Adds a witness hint for the unit_struct_changed_kind lint.

Example

unit_struct_changed_kind outputs the following witness hint for the ./test_crates/unit_struct_changed_kind/ test.

let witness = unit_struct_changed_kind::UnitStructToPlain;

This compiles on the old version, but not on the new as the struct can no longer be constructed as a unit struct;

Commit Notes

  • Added witness hint for unit_struct_changed_kind

Comments

I'm using this just to make sure my workspace is all working well, which it seems to be. Excited to start working on the witness system soon!

+ Added witness hint for unit_struct_changed_kind
@obi1kenobi
Copy link
Owner

I haven't looked at this lint in a while, and I'm not sure I'm convinced anymore that this lint is correct as it currently stands.

This witness fails to work if we define a const/static with the same name:

// Imagine this is the crate we're checking:
pub mod inner {
    // Imagine this is the struct that used to be a unit.
    pub struct SomeStruct {}
    
    pub const SomeStruct: SomeStruct = SomeStruct {};
}

fn witness() {
    // This works fine!
    let _x = inner::SomeStruct;
}

Try to take a look at pattern-matching the struct as the source of breakage, maybe? Make sure to consider #[non_exhaustive] structs, noting that #[non_exhaustive] structs can still be matched exhaustively from inside their own crate. A true #[non_exhaustive] test requires two crates — or a test and its doctests (which are compiled as a separate crate).

Depending on what you find, we may need to fix the lint to consider const/static values with the same name, or we may just need a different witness.

A similar close analysis like this might allow us to write a new lint for unit-to-tuple struct changes too, since we don't currently cover that case at all.

@GlitchlessCode
Copy link
Contributor Author

GlitchlessCode commented May 31, 2025

Good point! Some of these are trickier than one expects XD
I'll need to look at that, I'll try to get another commit in. Admittedly, I was mostly just looking for something theoretically simple to test my setup (had some changes since I last worked on this). I guess this needs more work than I expected.

Note: This may just need to backseated for now, since I'll be more focused on the witness system

@GlitchlessCode
Copy link
Contributor Author

Also I noticed that I accidentally commented out a line, which was not intentional.

@obi1kenobi
Copy link
Owner

I guess this needs more work than I expected.

Story of my life in cargo-semver-checks lately 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants