Skip to content

Commit

Permalink
Add the enum_repr_variant_discriminant_changed lint.
Browse files Browse the repository at this point in the history
Addresses a checkbox in #898.
  • Loading branch information
obi1kenobi committed Dec 11, 2024
1 parent fe144fc commit 0cd2058
Show file tree
Hide file tree
Showing 11 changed files with 272 additions and 1 deletion.
89 changes: 89 additions & 0 deletions src/lints/enum_repr_variant_discriminant_changed.ron
Original file line number Diff line number Diff line change
@@ -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}}"),
)
1 change: 1 addition & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "enum_repr_variant_discriminant_changed"
version = "0.1.0"
edition = "2021"

[dependencies]
25 changes: 25 additions & 0 deletions test_crates/enum_repr_variant_discriminant_changed/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "enum_repr_variant_discriminant_changed"
version = "0.1.0"
edition = "2021"

[dependencies]
26 changes: 26 additions & 0 deletions test_crates/enum_repr_variant_discriminant_changed/old/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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,
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: src/query.rs
expression: "&query_execution_results"
snapshot_kind: text
---
{
"./test_crates/enum_no_repr_variant_discriminant_changed/": [
Expand Down Expand Up @@ -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"),
},
],
}
Original file line number Diff line number Diff line change
@@ -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"),
},
],
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: src/query.rs
expression: "&query_execution_results"
snapshot_kind: text
---
{
"./test_crates/enum_no_repr_variant_discriminant_changed/": [
Expand All @@ -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"),
},
Expand Down

0 comments on commit 0cd2058

Please sign in to comment.