Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the enum_repr_variant_discriminant_changed lint. #1036

Merged
merged 1 commit into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading