diff --git a/src/lints/enum_repr_variant_discriminant_changed.ron b/src/lints/enum_repr_variant_discriminant_changed.ron new file mode 100644 index 00000000..04361b2e --- /dev/null +++ b/src/lints/enum_repr_variant_discriminant_changed.ron @@ -0,0 +1,89 @@ +SemverQuery( + id: "enum_repr_variant_discriminant_changed", + human_readable_name: "variant of an enum with explicit repr changed discriminant", + description: "The variant of an enum with explicit repr() had its discriminant change from its previous value.", + reference: Some("The variant of an enum with an explicit repr() had its discriminant value change. This breaks downstream code that accessed the discriminant via pointer casting."), + required_update: Major, + lint_level: Deny, + reference_link: Some("https://doc.rust-lang.org/reference/items/enumerations.html#pointer-casting"), + query: r#" + { + CrateDiff { + baseline { + item { + ... on Enum { + visibility_limit @filter(op: "=", value: ["$public"]) @output + enum_name: name @output @tag + + importable_path { + path @output @tag + public_api @filter(op: "=", value: ["$true"]) + } + + attribute { + old_attr: raw_attribute @output + content { + base @filter(op: "=", value: ["$repr"]) + argument { + base @filter(op: "regex", value: ["$repr_regex"]) + } + } + } + + variant { + variant_name: name @output @tag + + discriminant { + old_value: value @output @tag + } + } + } + } + } + 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"]) + } + + attribute { + new_attr: raw_attribute @output + content { + base @filter(op: "=", value: ["$repr"]) + argument { + base @filter(op: "regex", value: ["$repr_regex"]) + } + } + } + + 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 + } + } + } + } + } + } + }"#, + arguments: { + "public": "public", + "repr": "repr", + "repr_regex": "[ui](\\d+|size)", + "true": true, + }, + error_message: "An enum variant has changed its discriminant value. The enum has a defined primitive representation, so this breaks downstream code that used the discriminant value via an unsafe pointer cast.", + 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 c865b6b1..a52e4991 100644 --- a/src/query.rs +++ b/src/query.rs @@ -1065,6 +1065,7 @@ add_lints!( enum_repr_int_changed, enum_repr_int_removed, enum_repr_transparent_removed, + enum_repr_variant_discriminant_changed, enum_struct_variant_field_added, enum_struct_variant_field_missing, enum_struct_variant_field_now_doc_hidden, 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 index 84545953..00314c88 100644 --- 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 @@ -42,6 +42,14 @@ pub enum DiscriminantBecomesDocHiddenAndExplicit { Second = 2, } +// This enum starts off not having repr(), then gains repr() +// while also changing discriminant values. This should be reported here: +// the enum originally has no ABI committments so the addition of the repr() doesn't matter. +#[repr(u16)] +pub enum GainsRepr { + First = 2, +} + // Explicit discriminants changed values, but being private dominates. Should not be // reported. enum PrivateEnum { 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 index ad935b9f..077979f9 100644 --- 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 @@ -40,6 +40,13 @@ pub enum DiscriminantBecomesDocHiddenAndExplicit { Second, } +// This enum starts off not having repr(), then gains repr() +// while also changing discriminant values. This should be reported here: +// the enum originally has no ABI committments so the addition of the repr() doesn't matter. +pub enum GainsRepr { + First = 1, +} + // Explicit discriminants changed values, but being private dominates. Should not be // reported. enum PrivateEnum { diff --git a/test_crates/enum_repr_variant_discriminant_changed/new/Cargo.toml b/test_crates/enum_repr_variant_discriminant_changed/new/Cargo.toml new file mode 100644 index 00000000..e85b2a31 --- /dev/null +++ b/test_crates/enum_repr_variant_discriminant_changed/new/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "enum_repr_variant_discriminant_changed" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/enum_repr_variant_discriminant_changed/new/src/lib.rs b/test_crates/enum_repr_variant_discriminant_changed/new/src/lib.rs new file mode 100644 index 00000000..bb8cd40a --- /dev/null +++ b/test_crates/enum_repr_variant_discriminant_changed/new/src/lib.rs @@ -0,0 +1,25 @@ +// Explicit discriminant changed values. By doing so, it changed the implicit +// discriminant's value as well. Should be reported. +#[repr(isize)] +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 or ABI breakage. Should be reported. +#[repr(i16)] +#[non_exhaustive] +pub enum DiscriminantIsChanged { + First = 1, +} + +// Discriminant changed values, while the other variant is marked `non_exhaustive`. +// The variant's exhaustiveness is not relevant for pointer casting, so this should be reported. +#[repr(u32)] +pub enum DiscriminantIsChangedButAVariantIsNonExhaustive { + #[non_exhaustive] + First, + Second = 2, +} diff --git a/test_crates/enum_repr_variant_discriminant_changed/old/Cargo.toml b/test_crates/enum_repr_variant_discriminant_changed/old/Cargo.toml new file mode 100644 index 00000000..e85b2a31 --- /dev/null +++ b/test_crates/enum_repr_variant_discriminant_changed/old/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "enum_repr_variant_discriminant_changed" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/enum_repr_variant_discriminant_changed/old/src/lib.rs b/test_crates/enum_repr_variant_discriminant_changed/old/src/lib.rs new file mode 100644 index 00000000..7452b9ba --- /dev/null +++ b/test_crates/enum_repr_variant_discriminant_changed/old/src/lib.rs @@ -0,0 +1,26 @@ +// Explicit discriminant changed values. By doing so, it changed the implicit +// discriminant's value as well. Should be reported. +#[repr(isize)] +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 or ABI breakage. Should be reported. +#[repr(i16)] +#[non_exhaustive] +pub enum DiscriminantIsChanged { + First, +} + +// Discriminant changed values, while the other variant is marked `non_exhaustive`. +// The variant's exhaustiveness is not relevant for pointer casting, so this should be reported. +#[non_exhaustive] +#[repr(u32)] +pub enum DiscriminantIsChangedButAVariantIsNonExhaustive { + #[non_exhaustive] + First, + Second, +} diff --git a/test_outputs/query_execution/enum_no_repr_variant_discriminant_changed.snap b/test_outputs/query_execution/enum_no_repr_variant_discriminant_changed.snap index b62f0302..afe5b99d 100644 --- a/test_outputs/query_execution/enum_no_repr_variant_discriminant_changed.snap +++ b/test_outputs/query_execution/enum_no_repr_variant_discriminant_changed.snap @@ -1,6 +1,7 @@ --- source: src/query.rs expression: "&query_execution_results" +snapshot_kind: text --- { "./test_crates/enum_no_repr_variant_discriminant_changed/": [ @@ -56,5 +57,18 @@ expression: "&query_execution_results" "variant_name": String("Second"), "visibility_limit": String("public"), }, + { + "enum_name": String("GainsRepr"), + "new_value": String("2"), + "old_value": String("1"), + "path": List([ + String("enum_no_repr_variant_discriminant_changed"), + String("GainsRepr"), + ]), + "span_begin_line": Uint64(50), + "span_filename": String("src/lib.rs"), + "variant_name": String("First"), + "visibility_limit": String("public"), + }, ], } diff --git a/test_outputs/query_execution/enum_repr_variant_discriminant_changed.snap b/test_outputs/query_execution/enum_repr_variant_discriminant_changed.snap new file mode 100644 index 00000000..14c2706b --- /dev/null +++ b/test_outputs/query_execution/enum_repr_variant_discriminant_changed.snap @@ -0,0 +1,86 @@ +--- +source: src/query.rs +expression: "&query_execution_results" +snapshot_kind: text +--- +{ + "./test_crates/enum_no_repr_variant_discriminant_changed/": [ + { + "enum_name": String("FieldlessWithDiscrimants"), + "new_attr": String("#[repr(u8)]"), + "new_value": String("21"), + "old_attr": String("#[repr(u8)]"), + "old_value": String("20"), + "path": List([ + String("enum_no_repr_variant_discriminant_changed"), + String("FieldlessWithDiscrimants"), + ]), + "span_begin_line": Uint64(68), + "span_filename": String("src/lib.rs"), + "variant_name": String("Last"), + "visibility_limit": String("public"), + }, + ], + "./test_crates/enum_repr_variant_discriminant_changed/": [ + { + "enum_name": String("ExplicitAndImplicitDiscriminantsAreChanged"), + "new_attr": String("#[repr(isize)]"), + "new_value": String("2"), + "old_attr": String("#[repr(isize)]"), + "old_value": String("1"), + "path": List([ + String("enum_repr_variant_discriminant_changed"), + String("ExplicitAndImplicitDiscriminantsAreChanged"), + ]), + "span_begin_line": Uint64(5), + "span_filename": String("src/lib.rs"), + "variant_name": String("First"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("ExplicitAndImplicitDiscriminantsAreChanged"), + "new_attr": String("#[repr(isize)]"), + "new_value": String("3"), + "old_attr": String("#[repr(isize)]"), + "old_value": String("2"), + "path": List([ + String("enum_repr_variant_discriminant_changed"), + String("ExplicitAndImplicitDiscriminantsAreChanged"), + ]), + "span_begin_line": Uint64(6), + "span_filename": String("src/lib.rs"), + "variant_name": String("Second"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("DiscriminantIsChanged"), + "new_attr": String("#[repr(i16)]"), + "new_value": String("1"), + "old_attr": String("#[repr(i16)]"), + "old_value": String("0"), + "path": List([ + String("enum_repr_variant_discriminant_changed"), + String("DiscriminantIsChanged"), + ]), + "span_begin_line": Uint64(15), + "span_filename": String("src/lib.rs"), + "variant_name": String("First"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("DiscriminantIsChangedButAVariantIsNonExhaustive"), + "new_attr": String("#[repr(u32)]"), + "new_value": String("2"), + "old_attr": String("#[repr(u32)]"), + "old_value": String("1"), + "path": List([ + String("enum_repr_variant_discriminant_changed"), + String("DiscriminantIsChangedButAVariantIsNonExhaustive"), + ]), + "span_begin_line": Uint64(24), + "span_filename": String("src/lib.rs"), + "variant_name": String("Second"), + "visibility_limit": String("public"), + }, + ], +} diff --git a/test_outputs/query_execution/enum_struct_variant_field_added.snap b/test_outputs/query_execution/enum_struct_variant_field_added.snap index cc7cde5e..cbd5bd04 100644 --- a/test_outputs/query_execution/enum_struct_variant_field_added.snap +++ b/test_outputs/query_execution/enum_struct_variant_field_added.snap @@ -1,6 +1,7 @@ --- source: src/query.rs expression: "&query_execution_results" +snapshot_kind: text --- { "./test_crates/enum_no_repr_variant_discriminant_changed/": [ @@ -11,7 +12,7 @@ expression: "&query_execution_results" String("enum_no_repr_variant_discriminant_changed"), String("UnitOnlyBecomesUndefined"), ]), - "span_begin_line": Uint64(77), + "span_begin_line": Uint64(85), "span_filename": String("src/lib.rs"), "variant_name": String("Struct"), },