feat: add lint struct_with_no_pub_fields_changed (#962)

* intial draft of struct_without_pub_fields_changed

* Added two more testcases

* align the naming of testcases

* added a testcase for `#[doc(hidden)]`

* added a (not currently fulfilled) testcase showing a false negative via `PubStructWithNonPubDocFieldChangedToUnion`

* Removed `witness_template`

Co-authored-by: Max Carr <m@mcarr.one>

* fixed a typo noticed during code review

* fixed a typo and reworded the description

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

* made the lint name consistent by adding the `_type` suffix

* made sure that clippy is happy with the `test_crates`

* apply the suggestions from the code review

* removed a misunderstanding about `#[non_exhaustive]` usage

* made sure that line lengths of 100 is enforced

* reformatted the test_crate with consistent whitespace

* removed an accidentally incorrect comment

* Fixed a typo in the whitness

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

* updated the witness tests with the typo fix

---------

Co-authored-by: Max Carr <m@mcarr.one>
Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

src/lints/struct_with_no_pub_fields_changed_type.ron

@@ -0,0 +1,76 @@
+SemverQuery(
+    id: "struct_with_no_pub_fields_changed_type",
+    human_readable_name: "public API struct with no public fields is no longer a struct",
+    description: "A struct without pub fields was converted into an enum or union, breaking pattern matching.",
+    required_update: Major,
+    lint_level: Deny,
+    reference_link: Some("https://internals.rust-lang.org/t/rest-patterns-foo-should-match-non-struct-types/21607"),
+    reference: Some(
+        r#"\
+Even if a struct does not expose pub fields, pattern matching like `matches!(value, Example { .. })` is allowed outside \
+the struct's own crate. Changing such a struct into an enum or union will break such pattern matching.
+
+More info: https://github.com/obi1kenobi/cargo-semver-checks/issues/954
+"#
+    ),
+    query: r#"
+    {
+        CrateDiff {
+            baseline {
+                item {
+                    ... on Struct {
+                        struct_typename: __typename @tag @output
+                        visibility_limit @filter(op: "=", value: ["$public"]) @output
+                        struct_type @output
+
+                        # Ensure the struct does not have pub fields
+                        # This prevents overlap with struct_with_pub_fields_changed_type
+                        field @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
+                            visibility_limit @filter(op: "=", value: ["$public"])
+                            public_api_eligible @filter(op: "=", value: ["$true"])
+                        }
+
+                        # Ensure the struct does have non-pub fields
+                        # This prevents overlap with constructible_struct_changed_type
+                        field @fold @transform(op: "count") @filter(op: ">", value: ["$zero"])
+
+                        importable_path {
+                            path @output @tag
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+                    }
+                }
+            }
+            current {
+                item {
+                    ... on ImplOwner {
+                        current_typename: __typename @filter(op: "!=", value: ["%struct_typename"])
+                                                     @output
+                        visibility_limit @filter(op: "=", value: ["$public"])
+                        name @output
+
+                        importable_path {
+                            path @filter(op: "=", value: ["%path"])
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+
+                        span_: span @optional {
+                            filename @output
+                            begin_line @output
+                        }
+                    }
+                }
+            }
+        }
+    }"#,
+    arguments: {
+        "public": "public",
+        "true": true,
+        "zero": 0,
+    },
+    error_message: "A struct without pub fields became an enum or union, breaking pattern matching.",
+    per_result_error_template: Some("struct {{join \"::\" path}} became {{lowercase current_typename}} in file {{span_filename}}:{{span_begin_line}}"),
+    witness: (
+        hint_template: r#"matches!(value, {{join "::" path}} {..});"#,
+    ),
+)

src/lints/struct_with_pub_fields_changed_type.ron

@@ -1,7 +1,7 @@
 SemverQuery(
     id: "struct_with_pub_fields_changed_type",
     human_readable_name: "struct with pub fields became an enum or union",
-    description: "A struct was converted into an enum or union, breaking accesses to its fields.",
+    description: "A struct with pub fields was converted into an enum or union, breaking accesses to its public fields.",
     required_update: Major,
     lint_level: Deny,
     reference_link: Some("https://github.com/obi1kenobi/cargo-semver-checks/issues/297#issuecomment-1399099659"),
@@ -64,6 +64,6 @@ More info: https://github.com/obi1kenobi/cargo-semver-checks/issues/297
         "true": true,
         "zero": 0,
     },
-    error_message: "A struct became an enum or union, breaking accesses to its public fields.",
+    error_message: "A struct with pub fields became an enum or union, breaking accesses to its public fields.",
     per_result_error_template: Some("struct {{join \"::\" path}} became {{lowercase current_typename}} in file {{span_filename}}:{{span_begin_line}}"),
 )

src/query.rs

@@ -1065,6 +1065,7 @@ add_lints!(
     struct_pub_field_now_doc_hidden,
     struct_repr_transparent_removed,
     struct_with_pub_fields_changed_type,
+    struct_with_no_pub_fields_changed_type,
     trait_added_supertrait,
     trait_associated_const_added,
     trait_associated_const_default_removed,

test_crates/struct_with_no_pub_fields_changed_type/new/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+publish = false
+name = "struct_with_no_pub_fields_changed_type"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]

test_crates/struct_with_no_pub_fields_changed_type/new/src/lib.rs

@@ -0,0 +1,112 @@
+pub enum PubStructChangedToEnum {
+    Foo(usize),
+}
+
+pub union PubStructChangedToUnion {
+    foo: usize
+}
+
+#[non_exhaustive]
+pub enum PubNonExhaustiveStructChangedToEnum {
+    Foo(usize),
+}
+
+pub union PubNonExhaustiveStructChangedToUnion {
+    foo: usize,
+}
+
+pub enum PubStructWithNonPubDocFieldChangedToEnum {
+    Foo(usize),
+}
+
+pub union PubStructWithNonPubDocFieldChangedToUnion {
+    /// Despite this field being pub, hiding it makes this not be `public_api_eligible` anymore
+    /// This struct should trigger `struct_with_no_pub_fields_changed_type` instead of
+    /// `struct_with_pub_fields_changed_type`
+    #[doc(hidden)]
+    pub foo: usize,
+}
+
+pub enum PubStructWithNonPubDocFieldAndNonPubFieldChangedToEnum {
+    Foo(usize),
+    Bar(usize),
+}
+
+pub union PubStructWithNonPubDocFieldAndNonPubFieldChangedToUnion {
+    foo: usize,
+    /// Despite this field being pub, hiding it makes this not be `public_api_eligible` anymore
+    /// This struct should trigger `struct_with_no_pub_fields_changed_type` instead of
+    /// `struct_with_pub_fields_changed_type`
+    #[doc(hidden)]
+    pub bar: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// The struct is not `public_api_eligible`.
+/// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+#[doc(hidden)]
+pub enum NonPubDocStructChangedToEnum {
+    Foo(usize),
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// The struct is not `public_api_eligible`.
+/// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+#[doc(hidden)]
+pub union NonPubDocStructChangedToUnion {
+    foo: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `struct_with_pub_fields_changed_type` instead
+pub enum PubStructWithPubFieldChangedToEnum {
+    Foo(usize),
+    Bar(usize),
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `struct_with_pub_fields_changed_type` instead
+pub union PubStructWithPubFieldChangedToUnion {
+    foo: usize,
+    pub bar: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `struct_missing` instead
+pub type PubStructChangedToType = u8;
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `constructible_struct_changed_type` instead
+pub enum PubStructChangedToNoFieldsEnum {}
+
+/// This struct should not be reported by the `struct_pub_field_missing` rule:
+/// since the struct is not pub in the first place, changing it does not change the API
+enum NonPubStructChangedToEnum {
+    Foo(usize),
+}
+
+/// This struct should not be reported by the `struct_pub_field_missing` rule:
+/// since the struct is not pub in the first place, changing it does not change the API
+union NonPubStructChangedToUnion {
+    foo: usize
+}
+
+type NonPubStructChangedToType = u8;
+
+mod not_pub_visible {
+    /// This struct should not be reported by the `struct_pub_field_missing` rule:
+    /// since the struct is not in a pub module, changing it does not change the API
+    pub enum NonReachableStructChangedToEnum {
+        Foo(usize),
+    }
+
+    /// This struct should not be reported by the `struct_pub_field_missing` rule:
+    /// since the struct is not in a pub module, changing it does not change the API
+    pub union NonReachableStructChangedToUnion {
+        foo: usize
+    }
+
+    /// This struct should not be reported by the `struct_pub_field_missing` rule:
+    /// since the struct is not in a pub module, changing it does not change the API
+    pub type NonReachableStructChangedToType = u8;
+}

test_crates/struct_with_no_pub_fields_changed_type/old/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+publish = false
+name = "struct_with_no_pub_fields_changed_type"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]

test_crates/struct_with_no_pub_fields_changed_type/old/src/lib.rs

@@ -0,0 +1,127 @@
+pub struct PubStructChangedToEnum {
+    foo: usize,
+}
+
+pub struct PubStructChangedToUnion {
+    foo: usize,
+}
+
+#[non_exhaustive]
+pub struct PubNonExhaustiveStructChangedToEnum {
+    foo: usize,
+}
+
+#[non_exhaustive]
+pub struct PubNonExhaustiveStructChangedToUnion {
+    foo: usize,
+}
+
+pub struct PubStructWithNonPubDocFieldChangedToEnum {
+    /// Despite this field being pub, hiding it makes this not be `public_api_eligible` anymore
+    /// This struct should trigger `struct_with_no_pub_fields_changed_type` instead of
+    /// `struct_with_pub_fields_changed_type`
+    /// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+    #[doc(hidden)]
+    pub foo: usize,
+}
+
+pub struct PubStructWithNonPubDocFieldChangedToUnion {
+    /// Despite this field being pub, hiding it makes this not be `public_api_eligible` anymore
+    /// This struct should trigger `struct_with_no_pub_fields_changed_type` instead of
+    /// `struct_with_pub_fields_changed_type`
+    /// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+    #[doc(hidden)]
+    pub foo: usize,
+}
+
+pub struct PubStructWithNonPubDocFieldAndNonPubFieldChangedToEnum {
+    foo: usize,
+    /// Despite this field being pub, hiding it makes this not be `public_api_eligible` anymore
+    /// This struct should trigger `struct_with_no_pub_fields_changed_type` instead of
+    /// `struct_with_pub_fields_changed_type`
+    /// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+    #[doc(hidden)]
+    pub bar: usize,
+}
+
+pub struct PubStructWithNonPubDocFieldAndNonPubFieldChangedToUnion {
+    foo: usize,
+    /// Despite this field being pub, hiding it makes this not be `public_api_eligible` anymore
+    /// This struct should trigger `struct_with_no_pub_fields_changed_type` instead of
+    /// `struct_with_pub_fields_changed_type`
+    /// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+    #[doc(hidden)]
+    pub bar: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// The struct is not `public_api_eligible`.
+/// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+#[doc(hidden)]
+pub struct NonPubDocStructChangedToEnum {
+    foo: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// The struct is not `public_api_eligible`.
+/// See <https://predr.ag/blog/checking-semver-for-doc-hidden-items/> for additional context
+#[doc(hidden)]
+pub struct NonPubDocStructChangedToUnion {
+    foo: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `struct_with_pub_fields_changed_type` instead
+pub struct PubStructWithPubFieldChangedToEnum {
+    foo: usize,
+    pub bar: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `struct_with_pub_fields_changed_type` instead
+pub struct PubStructWithPubFieldChangedToUnion {
+    foo: usize,
+    pub bar: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `struct_missing` instead
+pub struct PubStructChangedToType(u8);
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// This is `constructible_struct_changed_type` instead
+pub struct PubStructChangedToNoFieldsEnum {}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// since the struct is not pub in the first place, changing it does not change the API
+struct NonPubStructChangedToEnum {
+    foo: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// since the struct is not pub in the first place, changing it does not change the API
+struct NonPubStructChangedToUnion {
+    foo: usize,
+}
+
+/// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+/// since the struct is not pub in the first place, changing it does not change the API
+struct NonPubStructChangedToType(u8);
+
+mod not_pub_visible {
+    /// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+    /// since the struct is not in a pub module, changing it does not change the API
+    pub struct NonReachableStructChangedToEnum {
+        foo: usize,
+    }
+
+    /// This struct should not be reported by the `struct_with_no_pub_fields_changed_type` rule:
+    /// since the struct is not in a pub module, changing it does not change the API
+    pub struct NonReachableStructChangedToUnion {
+        foo: usize,
+    }
+
+    /// This struct should not be reported by the `struct_pub_field_missing` rule:
+    /// since the struct is not in a pub module, changing it does not change the API
+    pub struct NonReachableStructChangedToType(u8);
+}

test_outputs/query_execution/constructible_struct_changed_type.snap

@@ -44,4 +44,19 @@ expression: "&query_execution_results"
       "visibility_limit": String("public"),
     },
   ],
+  "./test_crates/struct_with_no_pub_fields_changed_type/": [
+    {
+      "current_typename": String("Enum"),
+      "name": String("PubStructChangedToNoFieldsEnum"),
+      "path": List([
+        String("struct_with_no_pub_fields_changed_type"),
+        String("PubStructChangedToNoFieldsEnum"),
+      ]),
+      "span_begin_line": Uint64(80),
+      "span_filename": String("src/lib.rs"),
+      "struct_type": String("plain"),
+      "struct_typename": String("Struct"),
+      "visibility_limit": String("public"),
+    },
+  ],
 }

test_outputs/query_execution/struct_missing.snap

@@ -123,6 +123,19 @@ expression: "&query_execution_results"
       "visibility_limit": String("public"),
     },
   ],
+  "./test_crates/struct_with_no_pub_fields_changed_type/": [
+    {
+      "name": String("PubStructChangedToType"),
+      "path": List([
+        String("struct_with_no_pub_fields_changed_type"),
+        String("PubStructChangedToType"),
+      ]),
+      "span_begin_line": Uint64(89),
+      "span_filename": String("src/lib.rs"),
+      "struct_type": String("tuple"),
+      "visibility_limit": String("public"),
+    },
+  ],
   "./test_crates/switch_to_reexport_as_underscore/": [
     {
       "name": String("Struct"),