From 5b9a24b5dc27f866050212dde1090e634f39f7df Mon Sep 17 00:00:00 2001 From: David Matos Date: Fri, 6 Sep 2024 22:52:08 +0200 Subject: [PATCH] Change behavior of lint to lint API specific breakage --- .github/workflows/ci.yml | 4 +- ..._no_repr_variant_discriminant_changed.ron} | 21 ++++++-- src/query.rs | 2 +- .../new/Cargo.toml | 2 +- .../new/src/lib.rs | 50 +++++++++++++++++ .../old/Cargo.toml | 2 +- .../old/src/lib.rs | 53 +++++++++++++++++++ .../new/src/lib.rs | 31 ----------- .../old/src/lib.rs | 30 ----------- ...r_variant_discriminant_changed.output.ron} | 27 +++------- ...enum_struct_variant_field_added.output.ron | 13 +++++ 11 files changed, 145 insertions(+), 90 deletions(-) rename src/lints/{enum_variant_discriminant_changed.ron => enum_no_repr_variant_discriminant_changed.ron} (73%) rename test_crates/{enum_variant_discriminant_changed => enum_no_repr_variant_discriminant_changed}/new/Cargo.toml (60%) create mode 100644 test_crates/enum_no_repr_variant_discriminant_changed/new/src/lib.rs rename test_crates/{enum_variant_discriminant_changed => enum_no_repr_variant_discriminant_changed}/old/Cargo.toml (60%) create mode 100644 test_crates/enum_no_repr_variant_discriminant_changed/old/src/lib.rs delete mode 100644 test_crates/enum_variant_discriminant_changed/new/src/lib.rs delete mode 100644 test_crates/enum_variant_discriminant_changed/old/src/lib.rs rename test_outputs/{enum_variant_discriminant_changed.output.ron => enum_no_repr_variant_discriminant_changed.output.ron} (59%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04566e4e..84280445 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1329,7 +1329,7 @@ jobs: - name: Check output run: | cd semver - EXPECTED="$(echo -e "--- failure auto_trait_impl_removed: auto trait no longer implemented ---\n--- failure enum_variant_discriminant_changed: enum variant had its discriminant change value ---")" + 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)" diff <(echo "$RESULT") <(echo "$EXPECTED") @@ -1361,7 +1361,7 @@ jobs: - name: Check output run: | cd semver - EXPECTED="$(echo -e "--- failure auto_trait_impl_removed: auto trait no longer implemented ---\n--- failure enum_variant_discriminant_changed: enum variant had its discriminant change value ---")" + 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)" diff <(echo "$RESULT") <(echo "$EXPECTED") diff --git a/src/lints/enum_variant_discriminant_changed.ron b/src/lints/enum_no_repr_variant_discriminant_changed.ron similarity index 73% rename from src/lints/enum_variant_discriminant_changed.ron rename to src/lints/enum_no_repr_variant_discriminant_changed.ron index 65d9d49a..906d7a35 100644 --- a/src/lints/enum_variant_discriminant_changed.ron +++ b/src/lints/enum_no_repr_variant_discriminant_changed.ron @@ -1,11 +1,11 @@ SemverQuery( - id: "enum_variant_discriminant_changed", + 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("The public enum's variant had its discriminant changed from its previous value. This can cause compatibility issues, which may break FFI use cases."), + reference: Some("A public enum's variant had its discriminant changed from its previous value, which breaks implementors relying on the discriminant value."), required_update: Major, lint_level: Deny, - reference_link: Some("https://doc.rust-lang.org/reference/items/enumerations.html#discriminants"), + reference_link: Some("https://doc.rust-lang.org/reference/items/enumerations.html#assigning-discriminant-values"), query: r#" { CrateDiff { @@ -15,6 +15,12 @@ SemverQuery( visibility_limit @filter(op: "=", value: ["$public"]) @output enum_name: name @output @tag + attribute @optional { + content { + base @filter(op: "!=", value: ["$repr"]) + } + } + importable_path { path @output @tag public_api @filter(op: "=", value: ["$true"]) @@ -35,6 +41,12 @@ SemverQuery( visibility_limit @filter(op: "=", value: ["$public"]) name @filter(op: "=", value: ["%enum_name"]) + attribute @optional { + content { + base @filter(op: "!=", value: ["$repr"]) + } + } + importable_path { path @filter(op: "=", value: ["%path"]) public_api @filter(op: "=", value: ["$true"]) @@ -58,8 +70,9 @@ SemverQuery( }"#, arguments: { "public": "public", + "repr": "repr", "true": true, }, - error_message: "The public enum's variant had its discriminant changed from its previous value. This can cause compatibility issues, which may break FFI use cases.", + error_message: "The public enum's variant had its discriminant changed from its previous value. This breaks implementors that relied on the discriminant value.", per_result_error_template: Some("variant {{enum_name}}::{{variant_name}} {{old_value}} -> {{new_value}} in {{span_filename}}:{{span_begin_line}}"), ) diff --git a/src/query.rs b/src/query.rs index 520cdd83..fd81c980 100644 --- a/src/query.rs +++ b/src/query.rs @@ -792,6 +792,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, @@ -805,7 +806,6 @@ add_lints!( enum_tuple_variant_field_now_doc_hidden, enum_unit_variant_changed_kind, enum_variant_added, - enum_variant_discriminant_changed, enum_variant_marked_non_exhaustive, enum_variant_missing, exported_function_changed_abi, diff --git a/test_crates/enum_variant_discriminant_changed/new/Cargo.toml b/test_crates/enum_no_repr_variant_discriminant_changed/new/Cargo.toml similarity index 60% rename from test_crates/enum_variant_discriminant_changed/new/Cargo.toml rename to test_crates/enum_no_repr_variant_discriminant_changed/new/Cargo.toml index a36927a1..fcfd8659 100644 --- a/test_crates/enum_variant_discriminant_changed/new/Cargo.toml +++ b/test_crates/enum_no_repr_variant_discriminant_changed/new/Cargo.toml @@ -1,6 +1,6 @@ [package] publish = false -name = "enum_variant_discriminant_changed" +name = "enum_no_repr_variant_discriminant_changed" version = "0.1.0" edition = "2021" diff --git a/test_crates/enum_no_repr_variant_discriminant_changed/new/src/lib.rs b/test_crates/enum_no_repr_variant_discriminant_changed/new/src/lib.rs new file mode 100644 index 00000000..2931c5cc --- /dev/null +++ b/test_crates/enum_no_repr_variant_discriminant_changed/new/src/lib.rs @@ -0,0 +1,50 @@ +// 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 to be doc hidden and 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 value, but with repr, 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 enum 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 }, +} diff --git a/test_crates/enum_variant_discriminant_changed/old/Cargo.toml b/test_crates/enum_no_repr_variant_discriminant_changed/old/Cargo.toml similarity index 60% rename from test_crates/enum_variant_discriminant_changed/old/Cargo.toml rename to test_crates/enum_no_repr_variant_discriminant_changed/old/Cargo.toml index a36927a1..fcfd8659 100644 --- a/test_crates/enum_variant_discriminant_changed/old/Cargo.toml +++ b/test_crates/enum_no_repr_variant_discriminant_changed/old/Cargo.toml @@ -1,6 +1,6 @@ [package] publish = false -name = "enum_variant_discriminant_changed" +name = "enum_no_repr_variant_discriminant_changed" version = "0.1.0" edition = "2021" diff --git a/test_crates/enum_no_repr_variant_discriminant_changed/old/src/lib.rs b/test_crates/enum_no_repr_variant_discriminant_changed/old/src/lib.rs new file mode 100644 index 00000000..760b8f88 --- /dev/null +++ b/test_crates/enum_no_repr_variant_discriminant_changed/old/src/lib.rs @@ -0,0 +1,53 @@ +// My task is: +// discriminant changed value but no repr (API only) +// the enum does not have an explicit representation (no repr) +// we want to be super sure the 4 lints are disjoint here -- we should never have multiple of them trigger on the same variant + +// 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 to be doc hidden and 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 value, but with repr, 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 enum 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 {}, +} diff --git a/test_crates/enum_variant_discriminant_changed/new/src/lib.rs b/test_crates/enum_variant_discriminant_changed/new/src/lib.rs deleted file mode 100644 index be64ea6b..00000000 --- a/test_crates/enum_variant_discriminant_changed/new/src/lib.rs +++ /dev/null @@ -1,31 +0,0 @@ -// Explicit discriminant changed values. By doing so, it changed the implicit -// discriminant's value as well, should be reported. -#[repr(u8, C)] -pub enum ExplicitAndImplicitDiscriminantsAreChanged { - First = 2, - Second, - Third = 5, -} - -// Implicit discriminant changed values when becoming explicit, should be reported. -#[repr(u8)] -pub enum ImplicitDiscriminantBecomesExplicit { - Tuple(), - Struct, - Unit = 5, -} - -// Discriminant changed to be doc hidden and 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, -} diff --git a/test_crates/enum_variant_discriminant_changed/old/src/lib.rs b/test_crates/enum_variant_discriminant_changed/old/src/lib.rs deleted file mode 100644 index c1b1db4e..00000000 --- a/test_crates/enum_variant_discriminant_changed/old/src/lib.rs +++ /dev/null @@ -1,30 +0,0 @@ -// Explicit discriminant changed values. By doing so, it changed the implicit -// discriminant's value as well, should be reported. -#[repr(u8, C)] -pub enum ExplicitAndImplicitDiscriminantsAreChanged { - First = 1, - Second, - Third = 5, -} - -// Implicit discriminant changed values when becoming explicit, should be reported. -#[repr(u8)] -pub enum ImplicitDiscriminantBecomesExplicit { - Tuple(), - Struct {}, - Unit, -} - -// Discriminant changed to be doc hidden and 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, -} diff --git a/test_outputs/enum_variant_discriminant_changed.output.ron b/test_outputs/enum_no_repr_variant_discriminant_changed.output.ron similarity index 59% rename from test_outputs/enum_variant_discriminant_changed.output.ron rename to test_outputs/enum_no_repr_variant_discriminant_changed.output.ron index 0b9b3015..678161f6 100644 --- a/test_outputs/enum_variant_discriminant_changed.output.ron +++ b/test_outputs/enum_no_repr_variant_discriminant_changed.output.ron @@ -1,14 +1,14 @@ { - "./test_crates/enum_variant_discriminant_changed/": [ + "./test_crates/enum_no_repr_variant_discriminant_changed/": [ { "enum_name": String("ExplicitAndImplicitDiscriminantsAreChanged"), "new_value": String("2"), "old_value": String("1"), "path": List([ - String("enum_variant_discriminant_changed"), + String("enum_no_repr_variant_discriminant_changed"), String("ExplicitAndImplicitDiscriminantsAreChanged"), ]), - "span_begin_line": Uint64(4), + "span_begin_line": Uint64(3), "span_filename": String("src/lib.rs"), "variant_name": String("First"), "visibility_limit": String("public"), @@ -18,36 +18,23 @@ "new_value": String("3"), "old_value": String("2"), "path": List([ - String("enum_variant_discriminant_changed"), + String("enum_no_repr_variant_discriminant_changed"), String("ExplicitAndImplicitDiscriminantsAreChanged"), ]), - "span_begin_line": Uint64(4), + "span_begin_line": Uint64(3), "span_filename": String("src/lib.rs"), "variant_name": String("Second"), "visibility_limit": String("public"), }, - { - "enum_name": String("ImplicitDiscriminantBecomesExplicit"), - "new_value": String("5"), - "old_value": String("2"), - "path": List([ - String("enum_variant_discriminant_changed"), - String("ImplicitDiscriminantBecomesExplicit"), - ]), - "span_begin_line": Uint64(12), - "span_filename": String("src/lib.rs"), - "variant_name": String("Unit"), - "visibility_limit": String("public"), - }, { "enum_name": String("DiscriminantBecomesDocHiddenAndExplicit"), "new_value": String("2"), "old_value": String("1"), "path": List([ - String("enum_variant_discriminant_changed"), + String("enum_no_repr_variant_discriminant_changed"), String("DiscriminantBecomesDocHiddenAndExplicit"), ]), - "span_begin_line": Uint64(20), + "span_begin_line": Uint64(11), "span_filename": String("src/lib.rs"), "variant_name": String("Second"), "visibility_limit": String("public"), diff --git a/test_outputs/enum_struct_variant_field_added.output.ron b/test_outputs/enum_struct_variant_field_added.output.ron index 1aa468c5..ce5188e1 100644 --- a/test_outputs/enum_struct_variant_field_added.output.ron +++ b/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(49), + "span_filename": String("src/lib.rs"), + "variant_name": String("Struct"), + }, + ], "./test_crates/enum_struct_field_hidden_from_public_api/": [ { "enum_name": String("AddedVariantField"),