False positive: Changing struct with no pub fields to another kind of type is not breaking by itself

[obi1kenobi]

Consider the following code change, from:

pub struct Foo {
    x: i64
}

impl Foo {
  pub fn new(x: i64) -> Self {
    Self { x }
  }
}

to:

pub enum Foo {
    Bar(i64),
}

impl Foo {
  pub fn new(x: i64) -> Self {
    Self::Bar(x)
  }
}

This isn't a breaking change. The struct's field wasn't visible, the struct wasn't externally constructible with a literal (due to the private field), and its pub methods are preserved across the change.

Instead, this is semver-minor: the new code publicly allows Foo::Bar(x) whereas the old code had no such ability. I believe changing the struct to a union would also be semver-minor for similar reasons.

[obi1kenobi]

Here's a decision tree to decide whether a struct deletion / change to a different type is breaking:

deleted entirely? -> breaking

not deleted but changed type ->
    any pub fields -> breaking (field access)
        enums don't have fields
        union fields require `unsafe` to access

    no fields at all, but exhaustive -> breaking (constructible)
        enums require a variant to construct
        unions cannot have zero fields

The best way forward is:

• [x] alter struct_missing to handle the "deleted entirely" case only: #306
• [x] add a lint to cover the "pub field" case: #312
  • this is different than struct pub field missing since that looks for fields that were removed, and will report once per removed field
  • the new lint looks for situations where fields
• [x] add a lint to cover the "constructible fieldless struct" case: #308

[obi1kenobi]

@epage is changing a struct into a trait worth covering off as an edge case, or is it too rare to worry about? With the plan above, structs changed into traits would be reported as "removed" in the same way as the earlier struct -> enum case you brought up. The fix is doable but a bit more work, and I'm not sure if it's the most pressing thing at the moment.

[epage]

Replacing a struct with a trait is likely rare enough to be ok but it feels a bit hacky. Personally, I would structure it around namespaces / universes for what is removed and then have other lints for what happens within a namespace

[obi1kenobi]

Makes sense. I'll open a couple of separate issues for the "replacing struct with trait" and similar edge cases.

[CAD97]

No, replacing a struct with an enum is breaking:

fn breaks(value: Chameleon) {
    let Chameleon { .. } = value;
    // works if Chameleon is a struct, variant, or union type
    // error if Chameleon is an enum type
}

Example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=885f6f4afed657e9d9bfa5ab58b2afb7

This works cross-crate even with #[non_exhaustive] to state that it's a type capable of having fields to pattern match on, irrespective of whether there are any fields actually available to pattern match on.

[obi1kenobi]

Sorry, I was sloppy with terminology. Thanks for bringing this up.

The change is breaking but is semver-minor instead of semver-major because of this part of the API evolution RFC: https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#minor-change-going-from-a-tuple-struct-with-all-private-fields-with-at-least-one-field-to-a-normal-struct-or-vice-versa

While that section refers to struct kind changes and not struct -> enum changes, the reasoning is identical in this case — there was nothing useful that could be learned from let Chameleon { .. } = value.

I've also updated my blog post to this effect, and cleaned up my use of terminology. Sadly it seems like it's too late for Reddit's algorithm because the post is stuck at 0 votes forever now :)