Skip to content

Commit

Permalink
feat: add lint struct_marked_non_exhaustive_changed_type (#963)
Browse files Browse the repository at this point in the history
* implemented `struct_marked_non_exhaustive_changed_type`

* reworded some comments

* Apply suggestions from code review

Co-authored-by: Predrag Gruevski <[email protected]>

* renamed the lint to `non_exhaustive_struct_changed_type`

* fixed the lints not being ordered correctly

* fix: ran the testing

---------

Co-authored-by: Predrag Gruevski <[email protected]>
  • Loading branch information
CommanderStorm and obi1kenobi authored Oct 16, 2024
1 parent ad4552c commit fb4ee17
Show file tree
Hide file tree
Showing 10 changed files with 209 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/lints/constructible_struct_changed_type.ron
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ to make it an enum or union is a breaking change, since the new enum / union can
with the same struct literal syntax.
While struct literals can be used to create any exhaustive struct whose fields are all public, \
structs with *any* (not *only*) public fields are handled in a different lint. \
structs with *any* fields are handled in a different lints. \
This lint covers the remaining edge case: exhaustive structs with no fields whatsoever.
More info: https://github.com/obi1kenobi/cargo-semver-checks/issues/297
Expand All @@ -29,6 +29,7 @@ More info: https://github.com/obi1kenobi/cargo-semver-checks/issues/297
struct_type @output
# If the struct is non-exhaustive, it can't be constructed using a literal.
# A struct having this is breaking via non_exhaustive_struct_changed_type instead.
attrs @filter(op: "not_contains", value: ["$non_exhaustive"])
# The struct must not have any fields.
Expand Down
74 changes: 74 additions & 0 deletions src/lints/non_exhaustive_struct_changed_type.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
SemverQuery(
id: "non_exhaustive_struct_changed_type",
human_readable_name: "struct marked #[non_exhaustive] with no fields became an enum or union",
description: "A struct marked #[non_exhaustive] with no fields was converted into an enum or union, breaking pattern matching.",
required_update: Major,
lint_level: Deny,
reference_link: Some("https://internals.rust-lang.org/t/rest-patterns-foo-should-match-non-struct-types/21607"),
reference: Some(
r#"\
Even if a struct does not have fields and is marked `#[non_exhaustive]`, pattern matching like
`matches!(value, Example { .. })` is allowed outside the struct's own crate. \
Changing such a struct into an enum or union will break such pattern matching.
More info: https://github.com/obi1kenobi/cargo-semver-checks/issues/954
"#
),
query: r#"
{
CrateDiff {
baseline {
item {
... on Struct {
struct_typename: __typename @tag @output
visibility_limit @filter(op: "=", value: ["$public"]) @output
struct_type @output
# Requiring #[non_exhaustive] removes overlap with constructible_struct_changed_type
attrs @filter(op: "contains", value: ["$non_exhaustive"])
# Requiring at least one field removes overlap with struct_with_no_pub_fields_changed_type
# That this is not a special case in said query is a limitation of our query engine
field @fold @transform(op: "count") @filter(op: "=", value: ["$zero"])
importable_path {
path @output @tag
public_api @filter(op: "=", value: ["$true"])
}
}
}
}
current {
item {
... on ImplOwner {
current_typename: __typename @filter(op: "!=", value: ["%struct_typename"])
@output
visibility_limit @filter(op: "=", value: ["$public"])
name @output
importable_path {
path @filter(op: "=", value: ["%path"])
public_api @filter(op: "=", value: ["$true"])
}
span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}"#,
arguments: {
"public": "public",
"non_exhaustive": "#[non_exhaustive]",
"true": true,
"zero": 0,
},
error_message: "A struct marked #[non_exhaustive] with no fields was converted into an enum or union, breaking pattern matching",
per_result_error_template: Some("struct {{join \"::\" path}} became {{lowercase current_typename}} in file {{span_filename}}:{{span_begin_line}}"),
witness: (
hint_template: r#"matches!(value, {{join "::" path}} {..});"#,
),
)
5 changes: 4 additions & 1 deletion src/lints/struct_with_no_pub_fields_changed_type.ron
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ More info: https://github.com/obi1kenobi/cargo-semver-checks/issues/954
}
# Ensure the struct does have non-pub fields
# This prevents overlap with constructible_struct_changed_type
#
# This prevents overlap with constructible_struct_changed_type, but doing this requires
# non_exhaustive_struct_changed_type instead. Merging into this lint is currently
# impossible due the query engine limitation of not having OR-conditions.
field @fold @transform(op: "count") @filter(op: ">", value: ["$zero"])
importable_path {
Expand Down
1 change: 1 addition & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,7 @@ add_lints!(
inherent_method_unsafe_added,
method_parameter_count_changed,
module_missing,
non_exhaustive_struct_changed_type,
pub_module_level_const_missing,
pub_module_level_const_now_doc_hidden,
pub_static_missing,
Expand Down
7 changes: 7 additions & 0 deletions test_crates/non_exhaustive_struct_changed_type/new/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "non_exhaustive_struct_changed_type"
version = "0.1.0"
edition = "2021"

[dependencies]
5 changes: 5 additions & 0 deletions test_crates/non_exhaustive_struct_changed_type/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub enum PubNonExhaustiveStructChangedToEnum {}

pub union PubNonExhaustiveStructChangedToUnion {
foo: usize
}
7 changes: 7 additions & 0 deletions test_crates/non_exhaustive_struct_changed_type/old/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "non_exhaustive_struct_changed_type"
version = "0.1.0"
edition = "2021"

[dependencies]
5 changes: 5 additions & 0 deletions test_crates/non_exhaustive_struct_changed_type/old/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[non_exhaustive]
pub struct PubNonExhaustiveStructChangedToEnum {}

#[non_exhaustive]
pub struct PubNonExhaustiveStructChangedToUnion {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
source: src/query.rs
expression: "&query_execution_results"
---
{
"./test_crates/non_exhaustive_struct_changed_type/": [
{
"current_typename": String("Enum"),
"name": String("PubNonExhaustiveStructChangedToEnum"),
"path": List([
String("non_exhaustive_struct_changed_type"),
String("PubNonExhaustiveStructChangedToEnum"),
]),
"span_begin_line": Uint64(1),
"span_filename": String("src/lib.rs"),
"struct_type": String("plain"),
"struct_typename": String("Struct"),
"visibility_limit": String("public"),
},
{
"current_typename": String("Union"),
"name": String("PubNonExhaustiveStructChangedToUnion"),
"path": List([
String("non_exhaustive_struct_changed_type"),
String("PubNonExhaustiveStructChangedToUnion"),
]),
"span_begin_line": Uint64(3),
"span_filename": String("src/lib.rs"),
"struct_type": String("plain"),
"struct_typename": String("Struct"),
"visibility_limit": String("public"),
},
],
"./test_crates/struct_becomes_enum/": [
{
"current_typename": String("Enum"),
"name": String("NonExhaustiveEmptyStructToEnum"),
"path": List([
String("struct_becomes_enum"),
String("NonExhaustiveEmptyStructToEnum"),
]),
"span_begin_line": Uint64(82),
"span_filename": String("src/lib.rs"),
"struct_type": String("plain"),
"struct_typename": String("Struct"),
"visibility_limit": String("public"),
},
{
"current_typename": String("Enum"),
"name": String("NonExhaustiveFieldlessUnit"),
"path": List([
String("struct_becomes_enum"),
String("NonExhaustiveFieldlessUnit"),
]),
"span_begin_line": Uint64(95),
"span_filename": String("src/lib.rs"),
"struct_type": String("unit"),
"struct_typename": String("Struct"),
"visibility_limit": String("public"),
},
{
"current_typename": String("Enum"),
"name": String("NonExhaustiveFieldlessTuple"),
"path": List([
String("struct_becomes_enum"),
String("NonExhaustiveFieldlessTuple"),
]),
"span_begin_line": Uint64(100),
"span_filename": String("src/lib.rs"),
"struct_type": String("tuple"),
"struct_typename": String("Struct"),
"visibility_limit": String("public"),
},
],
}
29 changes: 29 additions & 0 deletions test_outputs/witnesses/non_exhaustive_struct_changed_type.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
source: src/query.rs
description: "Lint `non_exhaustive_struct_changed_type` did not have the expected witness output.\nSee https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md#testing-witnesses\nfor more information."
expression: "&actual_witnesses"
---
[["./test_crates/non_exhaustive_struct_changed_type/"]]
filename = 'src/lib.rs'
begin_line = 1
hint = 'matches!(value, non_exhaustive_struct_changed_type::PubNonExhaustiveStructChangedToEnum {..});'

[["./test_crates/non_exhaustive_struct_changed_type/"]]
filename = 'src/lib.rs'
begin_line = 3
hint = 'matches!(value, non_exhaustive_struct_changed_type::PubNonExhaustiveStructChangedToUnion {..});'

[["./test_crates/struct_becomes_enum/"]]
filename = 'src/lib.rs'
begin_line = 82
hint = 'matches!(value, struct_becomes_enum::NonExhaustiveEmptyStructToEnum {..});'

[["./test_crates/struct_becomes_enum/"]]
filename = 'src/lib.rs'
begin_line = 95
hint = 'matches!(value, struct_becomes_enum::NonExhaustiveFieldlessUnit {..});'

[["./test_crates/struct_becomes_enum/"]]
filename = 'src/lib.rs'
begin_line = 100
hint = 'matches!(value, struct_becomes_enum::NonExhaustiveFieldlessTuple {..});'

0 comments on commit fb4ee17

Please sign in to comment.