Skip to content

Commit

Permalink
Change behavior of lint to lint API specific breakage
Browse files Browse the repository at this point in the history
  • Loading branch information
dmatos2012 committed Sep 6, 2024
1 parent 164eff7 commit 5b9a24b
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 90 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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"])
Expand All @@ -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"])
Expand All @@ -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}}"),
)
2 changes: 1 addition & 1 deletion src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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"

Expand Down
Original file line number Diff line number Diff line change
@@ -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 },
}
Original file line number Diff line number Diff line change
@@ -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"

Expand Down
Original file line number Diff line number Diff line change
@@ -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 {},
}
31 changes: 0 additions & 31 deletions test_crates/enum_variant_discriminant_changed/new/src/lib.rs

This file was deleted.

30 changes: 0 additions & 30 deletions test_crates/enum_variant_discriminant_changed/old/src/lib.rs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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"),
Expand All @@ -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"),
Expand Down
13 changes: 13 additions & 0 deletions test_outputs/enum_struct_variant_field_added.output.ron
Original file line number Diff line number Diff line change
@@ -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"),
Expand Down

0 comments on commit 5b9a24b

Please sign in to comment.