From 96b22d0cb1e64f2ed17a6674d0a8210997eb67f3 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski Date: Wed, 11 Dec 2024 03:52:16 +0000 Subject: [PATCH] Add two lints for cases where enum discriminants are no longer defined. - `enum_discriminants_undefined_non_exhaustive_variant` checks for enums that gain a new non-exhaustive variant, which then causes the discriminant to become undefined. - `enum_discriminants_undefined_non_unit_variant` checks for enums that gain a new non-unit variant, which then causes the discriminant to become undefined. Resolves #898. --- ...nants_undefined_non_exhaustive_variant.ron | 83 +++++++++++++++++ ...scriminants_undefined_non_unit_variant.ron | 79 ++++++++++++++++ src/query.rs | 2 + .../new/Cargo.toml | 7 ++ .../new/src/lib.rs | 68 ++++++++++++++ .../old/Cargo.toml | 7 ++ .../old/src/lib.rs | 62 +++++++++++++ ...ants_undefined_non_exhaustive_variant.snap | 31 +++++++ ...criminants_undefined_non_unit_variant.snap | 91 +++++++++++++++++++ .../query_execution/enum_variant_added.snap | 14 +++ 10 files changed, 444 insertions(+) create mode 100644 src/lints/enum_discriminants_undefined_non_exhaustive_variant.ron create mode 100644 src/lints/enum_discriminants_undefined_non_unit_variant.ron create mode 100644 test_crates/enum_discriminant_no_longer_defined/new/Cargo.toml create mode 100644 test_crates/enum_discriminant_no_longer_defined/new/src/lib.rs create mode 100644 test_crates/enum_discriminant_no_longer_defined/old/Cargo.toml create mode 100644 test_crates/enum_discriminant_no_longer_defined/old/src/lib.rs create mode 100644 test_outputs/query_execution/enum_discriminants_undefined_non_exhaustive_variant.snap create mode 100644 test_outputs/query_execution/enum_discriminants_undefined_non_unit_variant.snap diff --git a/src/lints/enum_discriminants_undefined_non_exhaustive_variant.ron b/src/lints/enum_discriminants_undefined_non_exhaustive_variant.ron new file mode 100644 index 00000000..422fd439 --- /dev/null +++ b/src/lints/enum_discriminants_undefined_non_exhaustive_variant.ron @@ -0,0 +1,83 @@ +SemverQuery( + id: "enum_discriminants_undefined_non_exhaustive_variant", + human_readable_name: "enum's variants no longer have defined discriminants due to a non-exhaustive variant", + description: "A public enum's variants no longer have well-defined discriminants value due to a non-exhaustive variant.", + reference: Some("A public enum's variants no longer have well-defined discriminants due to a non-exhaustive variant. This breaks downstream code that accessed the discriminant 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 @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) { + discriminant { + value + } + } + + variant @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) { + __typename @filter(op: "!=", value: ["$plain_variant"]) + } + + 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 @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) { + __typename @filter(op: "!=", value: ["$plain_variant"]) + } + + variant @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) { + attrs @filter(op: "contains", value: ["$non_exhaustive"]) + } + + span_: span @optional { + filename @output + begin_line @output + } + } + } + } + } + }"#, + arguments: { + "public": "public", + "repr": "repr", + "non_exhaustive": "#[non_exhaustive]", + "plain_variant": "PlainVariant", + "zero": 0, + "true": true, + }, + error_message: "An enum's variants no longer have well-defined discriminant values due to a non-exhaustive variant in the enum. This breaks downstream code that accesses discriminants via a numeric cast like `as isize`.", + per_result_error_template: Some("enum {{enum_name}} in {{span_filename}}:{{span_begin_line}}"), +) diff --git a/src/lints/enum_discriminants_undefined_non_unit_variant.ron b/src/lints/enum_discriminants_undefined_non_unit_variant.ron new file mode 100644 index 00000000..bb69295f --- /dev/null +++ b/src/lints/enum_discriminants_undefined_non_unit_variant.ron @@ -0,0 +1,79 @@ +SemverQuery( + id: "enum_discriminants_undefined_non_unit_variant", + human_readable_name: "enum's variants no longer have defined discriminants due to non-unit variant", + description: "A public enum's variants no longer have well-defined discriminants value due to a non-unit variant.", + reference: Some("A public enum's variants no longer have well-defined discriminants due to a non-unit variant. This breaks downstream code that accessed the discriminant 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 @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) { + discriminant { + value + } + } + + variant @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) { + __typename @filter(op: "!=", value: ["$plain_variant"]) + } + + 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 @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) { + __typename @filter(op: "!=", value: ["$plain_variant"]) + } + + span_: span @optional { + filename @output + begin_line @output + } + } + } + } + } + }"#, + arguments: { + "public": "public", + "repr": "repr", + "non_exhaustive": "#[non_exhaustive]", + "plain_variant": "PlainVariant", + "zero": 0, + "true": true, + }, + error_message: "An enum's variants no longer have well-defined discriminant values due to a tuple or struct variant in the enum. This breaks downstream code that accesses discriminants via a numeric cast like `as isize`.", + per_result_error_template: Some("enum {{enum_name}} in {{span_filename}}:{{span_begin_line}}"), +) diff --git a/src/query.rs b/src/query.rs index c865b6b1..82992e76 100644 --- a/src/query.rs +++ b/src/query.rs @@ -1057,6 +1057,8 @@ add_lints!( derive_helper_attr_removed, derive_proc_macro_missing, derive_trait_impl_removed, + enum_discriminants_undefined_non_exhaustive_variant, + enum_discriminants_undefined_non_unit_variant, enum_marked_non_exhaustive, enum_missing, enum_must_use_added, diff --git a/test_crates/enum_discriminant_no_longer_defined/new/Cargo.toml b/test_crates/enum_discriminant_no_longer_defined/new/Cargo.toml new file mode 100644 index 00000000..8804ba26 --- /dev/null +++ b/test_crates/enum_discriminant_no_longer_defined/new/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "enum_discriminant_no_longer_defined" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/enum_discriminant_no_longer_defined/new/src/lib.rs b/test_crates/enum_discriminant_no_longer_defined/new/src/lib.rs new file mode 100644 index 00000000..6f43bbd3 --- /dev/null +++ b/test_crates/enum_discriminant_no_longer_defined/new/src/lib.rs @@ -0,0 +1,68 @@ +// It's not allowed to use `as isize` to cast another crate's enum when +// the enum contains a non-exhaustive variant: +// +// ---- src/lib.rs - GainsNonExhaustiveVariant (line 1) stdout ---- +// error[E0606]: casting `GainsNonExhaustiveVariant` as `isize` is invalid +// --> src/lib.rs:4:5 +// | +// 6 | value as isize; +// | ^^^^^^^^^^^^^^ +// | +// = note: cannot cast an enum with a non-exhaustive variant when it's defined in another crate +// +// error: aborting due to 1 previous error +// +// To see this, run the following doctest: +/// ```rust +/// fn example(value: enum_discriminant_no_longer_defined::GainsNonExhaustiveVariant) { +/// value as isize; +/// } +/// ``` +#[non_exhaustive] +pub enum GainsNonExhaustiveVariant { + First, + Second, + #[non_exhaustive] // TODO: this needs to be flagged but currently isn't. + Third, +} + +/// This shouldn't be reported: this enum's discriminants were not well-defined to begin with +/// because of the non-unit variant. +#[non_exhaustive] +pub enum NonUnitVariantButGainsNonExhaustiveVariant { + First, + Second(u16), + #[non_exhaustive] + Third, +} + +// It's not allowed to use `as isize` to cast another crate's enum when +// the enum contains a non-unit variant: +// +// ---- src/lib.rs - GainsTupleVariant (line 1) stdout ---- +// error[E0605]: non-primitive cast: `GainsTupleVariant` as `isize` +// --> src/lib.rs:4:5 +// | +// 6 | value as isize; +// | ^^^^^^^^^^^^^^ an `as` expression can be used to convert enum types to numeric types only if the enum type is unit-only or field-less +// | +// = note: see https://doc.rust-lang.org/reference/items/enumerations.html#casting for more information +// +// To see this, run the following doctest: +/// ```rust +/// fn example(value: enum_discriminant_no_longer_defined::GainsTupleVariant) { +/// value as isize; +/// } +/// ``` +#[non_exhaustive] +pub enum GainsTupleVariant { + None, + Never, + Some(core::num::NonZeroUsize), +} + +// Same as above, just with a struct variant instead of a tuple variant. +pub enum GainsStructVariant { + None, + Some { value: core::num::NonZeroUsize }, +} diff --git a/test_crates/enum_discriminant_no_longer_defined/old/Cargo.toml b/test_crates/enum_discriminant_no_longer_defined/old/Cargo.toml new file mode 100644 index 00000000..8804ba26 --- /dev/null +++ b/test_crates/enum_discriminant_no_longer_defined/old/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "enum_discriminant_no_longer_defined" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/enum_discriminant_no_longer_defined/old/src/lib.rs b/test_crates/enum_discriminant_no_longer_defined/old/src/lib.rs new file mode 100644 index 00000000..b561281a --- /dev/null +++ b/test_crates/enum_discriminant_no_longer_defined/old/src/lib.rs @@ -0,0 +1,62 @@ +// It's not allowed to use `as isize` to cast another crate's enum when +// the enum contains a non-exhaustive variant: +// +// ---- src/lib.rs - GainsNonExhaustiveVariant (line 1) stdout ---- +// error[E0606]: casting `GainsNonExhaustiveVariant` as `isize` is invalid +// --> src/lib.rs:4:5 +// | +// 6 | value as isize; +// | ^^^^^^^^^^^^^^ +// | +// = note: cannot cast an enum with a non-exhaustive variant when it's defined in another crate +// +// error: aborting due to 1 previous error +// +// To see this, run the following doctest: +/// ```rust +/// fn example(value: enum_discriminant_no_longer_defined::GainsNonExhaustiveVariant) { +/// value as isize; +/// } +/// ``` +#[non_exhaustive] +pub enum GainsNonExhaustiveVariant { + First, + Second, +} + +/// This shouldn't be reported: this enum's discriminants were not well-defined to begin with +/// because of the non-unit variant. +#[non_exhaustive] +pub enum NonUnitVariantButGainsNonExhaustiveVariant { + First, + Second(u16), +} + +// It's not allowed to use `as isize` to cast another crate's enum when +// the enum contains a non-unit variant: +// +// ---- src/lib.rs - GainsTupleVariant (line 1) stdout ---- +// error[E0605]: non-primitive cast: `GainsTupleVariant` as `isize` +// --> src/lib.rs:4:5 +// | +// 6 | value as isize; +// | ^^^^^^^^^^^^^^ an `as` expression can be used to convert enum types to numeric types only if the enum type is unit-only or field-less +// | +// = note: see https://doc.rust-lang.org/reference/items/enumerations.html#casting for more information +// +// To see this, run the following doctest: +/// ```rust +/// fn example(value: enum_discriminant_no_longer_defined::GainsTupleVariant) { +/// value as isize; +/// } +/// ``` +#[non_exhaustive] +pub enum GainsTupleVariant { + None, + Never, +} + +// Same as above, just with a struct variant instead of a tuple variant. +pub enum GainsStructVariant { + None, +} diff --git a/test_outputs/query_execution/enum_discriminants_undefined_non_exhaustive_variant.snap b/test_outputs/query_execution/enum_discriminants_undefined_non_exhaustive_variant.snap new file mode 100644 index 00000000..30d1bcf1 --- /dev/null +++ b/test_outputs/query_execution/enum_discriminants_undefined_non_exhaustive_variant.snap @@ -0,0 +1,31 @@ +--- +source: src/query.rs +expression: "&query_execution_results" +snapshot_kind: text +--- +{ + "./test_crates/enum_discriminant_no_longer_defined/": [ + { + "enum_name": String("GainsNonExhaustiveVariant"), + "path": List([ + String("enum_discriminant_no_longer_defined"), + String("GainsNonExhaustiveVariant"), + ]), + "span_begin_line": Uint64(22), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], + "./test_crates/enum_no_repr_variant_discriminant_changed/": [ + { + "enum_name": String("DiscriminantIsChangedAndMarkedNonExhaustive"), + "path": List([ + String("enum_no_repr_variant_discriminant_changed"), + String("DiscriminantIsChangedAndMarkedNonExhaustive"), + ]), + "span_begin_line": Uint64(21), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], +} diff --git a/test_outputs/query_execution/enum_discriminants_undefined_non_unit_variant.snap b/test_outputs/query_execution/enum_discriminants_undefined_non_unit_variant.snap new file mode 100644 index 00000000..4a14d596 --- /dev/null +++ b/test_outputs/query_execution/enum_discriminants_undefined_non_unit_variant.snap @@ -0,0 +1,91 @@ +--- +source: src/query.rs +expression: "&query_execution_results" +snapshot_kind: text +--- +{ + "./test_crates/enum_discriminant_no_longer_defined/": [ + { + "enum_name": String("GainsTupleVariant"), + "path": List([ + String("enum_discriminant_no_longer_defined"), + String("GainsTupleVariant"), + ]), + "span_begin_line": Uint64(58), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("GainsStructVariant"), + "path": List([ + String("enum_discriminant_no_longer_defined"), + String("GainsStructVariant"), + ]), + "span_begin_line": Uint64(65), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], + "./test_crates/enum_unit_variant_changed_kind/": [ + { + "enum_name": String("TestStruct"), + "path": List([ + String("enum_unit_variant_changed_kind"), + String("TestStruct"), + ]), + "span_begin_line": Uint64(2), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("TestTuple"), + "path": List([ + String("enum_unit_variant_changed_kind"), + String("TestTuple"), + ]), + "span_begin_line": Uint64(6), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("MultipleTest"), + "path": List([ + String("enum_unit_variant_changed_kind"), + String("MultipleTest"), + ]), + "span_begin_line": Uint64(10), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("TestBecomeDocHidden"), + "path": List([ + String("enum_unit_variant_changed_kind"), + String("TestBecomeDocHidden"), + ]), + "span_begin_line": Uint64(16), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("TestBecomeNonExhaustive"), + "path": List([ + String("enum_unit_variant_changed_kind"), + String("TestBecomeNonExhaustive"), + ]), + "span_begin_line": Uint64(21), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "enum_name": String("TestDocHidden"), + "path": List([ + String("enum_unit_variant_changed_kind"), + String("TestDocHidden"), + ]), + "span_begin_line": Uint64(50), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], +} diff --git a/test_outputs/query_execution/enum_variant_added.snap b/test_outputs/query_execution/enum_variant_added.snap index 8afad3bb..3d018d10 100644 --- a/test_outputs/query_execution/enum_variant_added.snap +++ b/test_outputs/query_execution/enum_variant_added.snap @@ -1,8 +1,22 @@ --- source: src/query.rs expression: "&query_execution_results" +snapshot_kind: text --- { + "./test_crates/enum_discriminant_no_longer_defined/": [ + { + "enum_name": String("GainsStructVariant"), + "path": List([ + String("enum_discriminant_no_longer_defined"), + String("GainsStructVariant"), + ]), + "span_begin_line": Uint64(67), + "span_filename": String("src/lib.rs"), + "variant_name": String("Some"), + "visibility_limit": String("public"), + }, + ], "./test_crates/enum_variant_added/": [ { "enum_name": String("EnumWithNewVariant"),