Add enum variant discriminant changed lint (#912)

* Add enum variant discriminant changed lint

* Change human_readable name

* Change CI test to allow extra failure for this lint

* Change behavior of lint to lint API specific breakage

* Make modifications to lint and add extra test cases

* Add two test cases to cover `non_exhaustive`

Typos and grammar

* spacing on lint file

.github/workflows/ci.yml

@@ -1329,8 +1329,8 @@ jobs:
       - name: Check output
         run: |
           cd semver
-          EXPECTED="$(echo -e "--- failure auto_trait_impl_removed: auto trait no longer implemented ---")"
-          RESULT="$(cat output | grep failure)"
+          EXPECTED="$(echo -e "--- failure auto_trait_impl_removed: auto trait no longer implemented ---\n--- failure enum_no_repr_variant_discriminant_changed: enum variant had its discriminant change value ---")"
+          RESULT="$(cat output | grep failure | sort)"
           diff <(echo "$RESULT") <(echo "$EXPECTED")
 
       - name: Cleanup
@@ -1361,8 +1361,8 @@ jobs:
       - name: Check output
         run: |
           cd semver
-          EXPECTED="$(echo -e "--- failure auto_trait_impl_removed: auto trait no longer implemented ---")"
-          RESULT="$(cat output | grep failure)"
+          EXPECTED="$(echo -e "--- failure auto_trait_impl_removed: auto trait no longer implemented ---\n--- failure enum_no_repr_variant_discriminant_changed: enum variant had its discriminant change value ---")"
+          RESULT="$(cat output | grep failure | sort)"
           diff <(echo "$RESULT") <(echo "$EXPECTED")
 
       - name: Save rustdoc

src/lints/enum_no_repr_variant_discriminant_changed.ron

@@ -0,0 +1,84 @@
+SemverQuery(
+    id: "enum_no_repr_variant_discriminant_changed",
+    human_readable_name: "enum variant had its discriminant change value",
+    description: "A public enum's variant had its discriminant changed from its previous value.",
+    reference: Some("A public enum's variant had its discriminant value change. This breaks downstream code that used its value via a numeric cast like `as isize`."),
+    required_update: Major,
+    lint_level: Deny,
+    reference_link: Some("https://doc.rust-lang.org/reference/items/enumerations.html#assigning-discriminant-values"),
+    query: r#"
+    {
+        CrateDiff {
+            baseline {
+                item {
+                    ... on Enum {
+                        visibility_limit @filter(op: "=", value: ["$public"]) @output
+                        enum_name: name @output @tag
+
+                        attribute @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
+                            content {
+                                base @filter(op: "=", value: ["$repr"])
+                            }
+                        }
+
+                        importable_path {
+                            path @output @tag
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+
+                        variant {
+                            variant_name: name @output @tag
+
+                            discriminant {
+                                old_value: value @output @tag
+                            }
+                        }
+
+                        variant @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
+                            attrs @filter(op: "contains", value: ["$non_exhaustive"])
+                        }
+                    }
+                }
+            }
+            current {
+                item {
+                    ... on Enum {
+                        visibility_limit @filter(op: "=", value: ["$public"])
+                        name @filter(op: "=", value: ["%enum_name"])
+
+                        importable_path {
+                            path @filter(op: "=", value: ["%path"])
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+
+                        variant {
+                            name @filter(op: "=", value: ["%variant_name"])
+
+                            discriminant {
+                                new_value: value @output @filter(op: "!=", value: ["%old_value"])
+                            }
+
+                            span_: span @optional {
+                                filename @output
+                                begin_line @output
+                            }
+                        }
+
+                        variant @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
+                            attrs @filter(op: "contains", value: ["$non_exhaustive"])
+                        }
+                    }
+                }
+            }
+        }
+    }"#,
+    arguments: {
+        "public": "public",
+        "repr": "repr",
+        "non_exhaustive": "#[non_exhaustive]",
+        "zero": 0,
+        "true": true,
+    },
+    error_message: "The enum's variant had its discriminant value change. This breaks downstream code that used its value via a numeric cast like `as isize`.",
+    per_result_error_template: Some("variant {{enum_name}}::{{variant_name}} {{old_value}} -> {{new_value}} in {{span_filename}}:{{span_begin_line}}"),
+)

src/query.rs

@@ -1044,6 +1044,7 @@ add_lints!(
     enum_marked_non_exhaustive,
     enum_missing,
     enum_must_use_added,
+    enum_no_repr_variant_discriminant_changed,
     enum_now_doc_hidden,
     enum_repr_int_changed,
     enum_repr_int_removed,

test_crates/enum_no_repr_variant_discriminant_changed/new/Cargo.toml

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

test_crates/enum_no_repr_variant_discriminant_changed/new/src/lib.rs

@@ -0,0 +1,95 @@
+// Explicit discriminant changed values. By doing so, it changed the implicit
+// discriminant's value as well. Should be reported.
+pub enum ExplicitAndImplicitDiscriminantsAreChanged {
+    First = 2,
+    Second,
+    Third = 5,
+}
+
+// Discriminant changed values. Having #[non_exhaustive] on the enum should not have any effect
+// on the *API* breakage. Should be reported.
+#[non_exhaustive]
+pub enum DiscriminantIsChanged {
+    First = 1,
+}
+
+// Discriminant changed values and the variant is also marked as `non_exhaustive`.
+// This now implies that the discriminant is no longer `well-defined`, which means that a numeric
+// cast is no longer possible on the enum. Should not be reported.
+// https://github.com/rust-lang/reference/pull/1249#issuecomment-2339003824
+#[non_exhaustive]
+pub enum DiscriminantIsChangedAndMarkedNonExhaustive {
+    First,
+    #[non_exhaustive]
+    Second = 2,
+}
+
+// Discriminant changed values, but the variant is already `non_exhaustive`.
+// This means that the discriminant is already not `well-defined`, and the numeric cast
+// was never possible. Should not be reported.
+#[non_exhaustive]
+pub enum DiscriminantIsChangedButAlreadyNonExhaustive {
+    First,
+    #[non_exhaustive]
+    Second = 2,
+}
+
+// Variant became doc(hidden) while variant became explicit. Being doc(hidden) is not relevant
+// since it's still part of the public API. Should be reported.
+pub enum DiscriminantBecomesDocHiddenAndExplicit {
+    First,
+    #[doc(hidden)]
+    Second = 2,
+}
+
+// Explicit discriminants changed values, but being private dominates. Should not be
+// reported.
+enum PrivateEnum {
+    First = 10,
+    Second = 11,
+}
+
+// Discriminant changed values, but the enum has a `repr` attr. Should not be reported as it
+// involves ABI breakage as well, and this is linted at *API* only breakage.
+#[repr(u8)]
+pub enum FieldlessWithDiscrimants {
+    First = 10,
+    Tuple(),
+    Struct {},
+    Unit,
+    Last = 21,
+}
+
+// Implicit discriminant value changed by becoming explicit. Should not be reported as it involves
+// ABI breakage as well, and this is linted at *API* only breakage.
+#[repr(u8)]
+pub enum FieldlessChangesToExplicitDescriminant {
+    Tuple(),
+    Struct {},
+    Unit = 5,
+}
+
+// Variant `Struct` acquires a field. By doing so, it makes the discriminant not `well-defined`,
+// meaning it is not possible to do a numeric cast anymore on the enum. Should not be reported.
+pub enum UnitOnlyBecomesUndefined {
+    First,
+    Second,
+    Struct { a: i64 },
+}
+
+// Discriminant changed values, but the other variant is marked `non_exhaustive`.
+// This means that a numeric cast is not possible on the enum. Should not be reported.
+#[non_exhaustive]
+pub enum DiscriminantIsChangedButAVariantIsNonExhaustive {
+    #[non_exhaustive]
+    First,
+    Second = 2,
+}
+
+// Discriminant changed values, and the variant is no longer `non_exhaustive`.
+// The fact that the variant is no longer `non_exhaustive` is not relevant, since a numeric cast
+// was impossible to begin with. Should not be reported.
+pub enum DiscriminantIsChangedAndVariantNoLongerNonExhaustive {
+    First,
+    Second = 2,
+}

test_crates/enum_no_repr_variant_discriminant_changed/old/Cargo.toml

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

test_crates/enum_no_repr_variant_discriminant_changed/old/src/lib.rs

@@ -0,0 +1,93 @@
+// Explicit discriminant changed values. By doing so, it changed the implicit
+// discriminant's value as well. Should be reported.
+pub enum ExplicitAndImplicitDiscriminantsAreChanged {
+    First = 1,
+    Second,
+    Third = 5,
+}
+
+// Discriminant changed values. Having #[non_exhaustive] on the enum should not have any effect
+// on the *API* breakage. Should be reported.
+#[non_exhaustive]
+pub enum DiscriminantIsChanged {
+    First,
+}
+
+// Discriminant changed values and the variant is also marked as `non_exhaustive`.
+// This now implies that the discriminant is no longer `well-defined`, which means that a numeric
+// cast is no longer possible on the enum. Should not be reported.
+// https://github.com/rust-lang/reference/pull/1249#issuecomment-2339003824
+#[non_exhaustive]
+pub enum DiscriminantIsChangedAndMarkedNonExhaustive {
+    First,
+    Second,
+}
+
+// Discriminant changed values, but the variant is already `non_exhaustive`.
+// This means that the discriminant is already not `well-defined`, and the numeric cast
+// was never possible. Should not be reported.
+#[non_exhaustive]
+pub enum DiscriminantIsChangedButAlreadyNonExhaustive {
+    First,
+    #[non_exhaustive]
+    Second,
+}
+
+// Variant became doc(hidden) while variant became explicit. Being doc(hidden) is not relevant
+// since it's still part of the public API. Should be reported.
+pub enum DiscriminantBecomesDocHiddenAndExplicit {
+    First,
+    Second,
+}
+
+// Explicit discriminants changed values, but being private dominates. Should not be
+// reported.
+enum PrivateEnum {
+    First = 1,
+    Second = 2,
+}
+
+// Discriminant changed values, but the enum has a `repr` attr. Should not be reported as it
+// involves ABI breakage as well, and this is linted at *API* only breakage.
+#[repr(u8)]
+pub enum FieldlessWithDiscrimants {
+    First = 10,
+    Tuple(),
+    Struct {},
+    Unit,
+    Last = 20,
+}
+
+// Implicit discriminant value changed by becoming explicit. Should not be reported as it involves
+// ABI breakage as well, and this is linted at *API* only breakage.
+pub enum FieldlessChangesToExplicitDescriminant {
+    Tuple(),
+    Struct {},
+    Unit,
+}
+
+// Variant `Struct` acquires a field. By doing so, it makes the discriminant not `well-defined`,
+// meaning it is not possible to do a numeric cast anymore on the enum. Should not be reported.
+pub enum UnitOnlyBecomesUndefined {
+    First,
+    Second,
+    Struct {},
+}
+
+// Discriminant changed values, but the other variant is marked `non_exhaustive`.
+// This means that a numeric cast is not possible on the enum. Should not be reported.
+#[non_exhaustive]
+pub enum DiscriminantIsChangedButAVariantIsNonExhaustive {
+    #[non_exhaustive]
+    First,
+    Second,
+}
+
+// Discriminant changed values, and the variant is no longer `non_exhaustive`.
+// The fact that the variant is no longer `non_exhaustive` is not relevant, since a numeric cast
+// was impossible to begin with. Should not be reported.
+pub enum DiscriminantIsChangedAndVariantNoLongerNonExhaustive {
+    #[non_exhaustive]
+    First,
+    Second,
+}

test_outputs/enum_no_repr_variant_discriminant_changed.output.ron

@@ -0,0 +1,56 @@
+{
+    "./test_crates/enum_no_repr_variant_discriminant_changed/": [
+        {
+            "enum_name": String("ExplicitAndImplicitDiscriminantsAreChanged"),
+            "new_value": String("2"),
+            "old_value": String("1"),
+            "path": List([
+                String("enum_no_repr_variant_discriminant_changed"),
+                String("ExplicitAndImplicitDiscriminantsAreChanged"),
+            ]),
+            "span_begin_line": Uint64(4),
+            "span_filename": String("src/lib.rs"),
+            "variant_name": String("First"),
+            "visibility_limit": String("public"),
+        },
+        {
+            "enum_name": String("ExplicitAndImplicitDiscriminantsAreChanged"),
+            "new_value": String("3"),
+            "old_value": String("2"),
+            "path": List([
+                String("enum_no_repr_variant_discriminant_changed"),
+                String("ExplicitAndImplicitDiscriminantsAreChanged"),
+            ]),
+            "span_begin_line": Uint64(5),
+            "span_filename": String("src/lib.rs"),
+            "variant_name": String("Second"),
+            "visibility_limit": String("public"),
+        },
+        {
+            "enum_name": String("DiscriminantIsChanged"),
+            "new_value": String("1"),
+            "old_value": String("0"),
+            "path": List([
+                String("enum_no_repr_variant_discriminant_changed"),
+                String("DiscriminantIsChanged"),
+            ]),
+            "span_begin_line": Uint64(13),
+            "span_filename": String("src/lib.rs"),
+            "variant_name": String("First"),
+            "visibility_limit": String("public"),
+        },
+        {
+            "enum_name": String("DiscriminantBecomesDocHiddenAndExplicit"),
+            "new_value": String("2"),
+            "old_value": String("1"),
+            "path": List([
+                String("enum_no_repr_variant_discriminant_changed"),
+                String("DiscriminantBecomesDocHiddenAndExplicit"),
+            ]),
+            "span_begin_line": Uint64(42),
+            "span_filename": String("src/lib.rs"),
+            "variant_name": String("Second"),
+            "visibility_limit": String("public"),
+        },
+    ]
+}

test_outputs/enum_struct_variant_field_added.output.ron

@@ -1,4 +1,17 @@
 {
+    "./test_crates/enum_no_repr_variant_discriminant_changed/": [
+        {
+            "enum_name": String("UnitOnlyBecomesUndefined"),
+            "field_name": String("a"),
+            "path": List([
+                String("enum_no_repr_variant_discriminant_changed"),
+                String("UnitOnlyBecomesUndefined"),
+            ]),
+            "span_begin_line": Uint64(77),
+            "span_filename": String("src/lib.rs"),
+            "variant_name": String("Struct"),
+        },
+    ],
     "./test_crates/enum_struct_field_hidden_from_public_api/": [
         {
             "enum_name": String("AddedVariantField"),