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.

Sign up for GitHub

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

Chore: Add witness hint for enum_tuple_variant_field_marked_deprecated lint #1216

Merged
merged 4 commits into from
Mar 25, 2025
Merged

Chore: Add witness hint for enum_tuple_variant_field_marked_deprecated lint #1216

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac288c7
Add enum_tuple_variant_field_marked_deprecated witness
GlitchlessCode Mar 24, 2025
63fa297
Merge branch 'obi1kenobi:main' into enum_tuple_variant_field_marked_d…
GlitchlessCode Mar 24, 2025
0995d34
Merge branch 'obi1kenobi:main' into enum_tuple_variant_field_marked_d…
GlitchlessCode Mar 25, 2025
a7146ed
Swap baseline_field_names with field_count
GlitchlessCode Mar 25, 2025
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
10 changes: 10 additions & 0 deletions src/lints/enum_tuple_variant_field_marked_deprecated.ron
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ SemverQuery(
public_api_eligible @filter(op: "=", value: ["$true"])
deprecated @filter(op: "!=", value: ["$true"])
}

field @fold {
baseline_field_names: name @output
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a tuple variant these are just always going to be 0, 1, 2, ... since the fields are purely positional, and not otherwise named.

Consider perhaps just counting the fields instead of outputting a (potentially large) number of known-consecutive numbers? It would make the query a bit more efficient and a bit more obvious.

As another idea, it might be good to mention that this info is used for the witness hint and isn't otherwise necessary for the detection of the issue itself.

                                # Figure out how many fields exist in the variant
                                # so we can generate a proper witness hint here.
                                field @fold @transform(op: "count") @output(name: "field_count")

Copy link
Contributor Author

@GlitchlessCode GlitchlessCode Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Genuinely forgot I could do this, thanks for the reminder, probably able to use this in some of the other enum_tuple_variant_... lints.

}
}
}
Expand Down Expand Up @@ -79,4 +83,10 @@ SemverQuery(
},
error_message: "A field in an enum's tuple variant is now #[deprecated]. Downstream crates will get a compiler warning when accessing this field.",
per_result_error_template: Some("field {{enum_name}}::{{variant_name}}.{{field_name}} in {{span_filename}}:{{span_begin_line}}"),
witness: (
hint_template: r#"match value {
{{ join "::" path }}::{{ variant_name }} ({{#each baseline_field_names }}{{#if (eq this ../field_name)}}to_be_deprecated{{else}}_{{/if}}{{#unless @last }}, {{/unless}}{{/each}}) => (),
_ => (),
}"#,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent use of templating, well done!

Adding my suggestion to name the deprecated field witness:

Suggested change
hint_template: r#"match value {
{{ join "::" path }}::{{ variant_name }} ({{#each baseline_field_names }}{{#if (eq this ../field_name)}}to_be_deprecated{{else}}_{{/if}}{{#unless @last }}, {{/unless}}{{/each}}) => (),
_ => (),
}"#,
hint_template: r#"match value {
{{ join "::" path }}::{{ variant_name }} ({{#each baseline_field_names }}{{#if (eq this ../field_name)}}witness{{else}}_{{/if}}{{#unless @last }}, {{/unless}}{{/each}}) => (),
_ => (),
}"#,

),
)
23 changes: 23 additions & 0 deletions test_outputs/query_execution/enum_tuple_variant_field_marked_deprecated.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ expression: "&query_execution_results"
{
"./test_crates/enum_struct_variant_field_marked_deprecated/": [
{
"baseline_field_names": List([
String("0"),
String("1"),
]),
"enum_name": String("NormalEnum"),
"field_name": String("0"),
"path": List([
Expand All @@ -19,6 +23,11 @@ expression: "&query_execution_results"
],
"./test_crates/enum_tuple_variant_field_marked_deprecated/": [
{
"baseline_field_names": List([
String("0"),
String("1"),
String("2"),
]),
"enum_name": String("TupleVariantEnum"),
"field_name": String("0"),
"path": List([
Expand All @@ -31,6 +40,11 @@ expression: "&query_execution_results"
"variant_name": String("TupleVariant"),
},
{
"baseline_field_names": List([
String("0"),
String("1"),
String("2"),
]),
"enum_name": String("TupleVariantEnum"),
"field_name": String("1"),
"path": List([
Expand All @@ -43,6 +57,11 @@ expression: "&query_execution_results"
"variant_name": String("TupleVariant"),
},
{
"baseline_field_names": List([
String("0"),
String("1"),
String("2"),
]),
"enum_name": String("TupleVariantEnum"),
"field_name": String("1"),
"path": List([
Expand All @@ -55,6 +74,10 @@ expression: "&query_execution_results"
"variant_name": String("AnotherTuple"),
},
{
"baseline_field_names": List([
String("0"),
String("1"),
]),
"enum_name": String("EnumWithHiddenVariant"),
"field_name": String("0"),
"path": List([
Expand Down
50 changes: 50 additions & 0 deletions test_outputs/witnesses/enum_tuple_variant_field_marked_deprecated.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
source: src/query.rs
description: "Lint `enum_tuple_variant_field_marked_deprecated` 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"
snapshot_kind: text
---
[["./test_crates/enum_struct_variant_field_marked_deprecated/"]]
filename = 'src/lib.rs'
begin_line = 19
hint = '''
match value {
enum_struct_variant_field_marked_deprecated::NormalEnum::TupleVariant (to_be_deprecated, _) => (),
_ => (),
}'''

[["./test_crates/enum_tuple_variant_field_marked_deprecated/"]]
filename = 'src/lib.rs'
begin_line = 7
hint = '''
match value {
enum_tuple_variant_field_marked_deprecated::TupleVariantEnum::TupleVariant (to_be_deprecated, _, _) => (),
_ => (),
}'''

[["./test_crates/enum_tuple_variant_field_marked_deprecated/"]]
filename = 'src/lib.rs'
begin_line = 8
hint = '''
match value {
enum_tuple_variant_field_marked_deprecated::TupleVariantEnum::TupleVariant (_, to_be_deprecated, _) => (),
_ => (),
}'''

[["./test_crates/enum_tuple_variant_field_marked_deprecated/"]]
filename = 'src/lib.rs'
begin_line = 13
hint = '''
match value {
enum_tuple_variant_field_marked_deprecated::TupleVariantEnum::AnotherTuple (_, to_be_deprecated, _) => (),
_ => (),
}'''

[["./test_crates/enum_tuple_variant_field_marked_deprecated/"]]
filename = 'src/lib.rs'
begin_line = 57
hint = '''
match value {
enum_tuple_variant_field_marked_deprecated::EnumWithHiddenVariant::NormalTuple (to_be_deprecated, _) => (),
_ => (),
}'''
Loading