Chore: Add witness hint for enum_tuple_variant_field_marked_deprecated lint
#1216
Conversation
+ Added witness hint for enum_tuple_variant_field_marked_deprecated + Added new parts to query for enum_tuple_variant_field_marked_deprecated to fetch relevant data for the witness
We usually call the most-directly-involved value |
| field @fold { | ||
| baseline_field_names: name @output | ||
| } |
There was a problem hiding this comment.
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")
There was a problem hiding this comment.
Genuinely forgot I could do this, thanks for the reminder, probably able to use this in some of the other enum_tuple_variant_... lints.
| 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}}) => (), | ||
| _ => (), | ||
| }"#, |
There was a problem hiding this comment.
Excellent use of templating, well done!
Adding my suggestion to name the deprecated field 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}}) => (), | |
| _ => (), | |
| }"#, | |
| 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}}) => (), | |
| _ => (), | |
| }"#, |
Adds a custom handlebars block helper called `repeat`. The goal of this
helper is to allow one to iterate a value `n` number of times. The
syntax of `repeat` is similar to that of the built-in `each` helper. It
assigns to the local block variables `@index`, `@first`, and `@last`.
`@index` is a number, `@first` and `@last` are booleans.
## Example
The following example handlebars template assumes the existence of an
outputted value called `field_count`, which is a `Number` type
representing the number of fields on an enum variant (for example).
```
"path_to_enum::ExampleEnum::ExampleVariant({{#repeat field_count }}_{{#unless @last }}, {{/unless}}{{/times}})"
```
This template string would produce the following output string for an
input tuple enum with 3 fields:
```
"path_to_enum::ExampleEnum::ExampleVariant(_, _, _)"
```
## Commit Notes
+ Adds a #repeat block helper, similar to the built-in #each helper
## Comments
This PR is to support the progress of #1212, #1214, and #1216. I'm
creating this because as far as I am aware, there is not currently a way
to iterate based on a Number in the existing set of handlebars helpers.
Let me know if there's any improvements I can make to this!
---------
Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>
…eprecated-witness
Adds a `to_string` custom handlebars helper. ## Comments There's not much to show for this one, it just converts a Number or Bool to a String Value. This isn't useful most of the time, as templating converts a variable to a string anyways. It's main use case is for comparing a number and a string. This is currently needed in #1214 and #1216, as although we can now iterate using the `repeat` helper, it produces Number Values, while `field_name`, which `@index` is compared against, is a String Value, which will always fail the `eq` test. This helper exists to resolve that issue. --------- Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>
…eprecated-witness
+ Added field_count + Removed baseline_field_names + Added repeat helper + Removed each helper + Added to_string helper + Replaced to_be_deprecated with witness
|
I think this has resolved all the aforementioned improvements? If it hasn't please do let me know! |
GlitchlessCode
deleted the
enum_tuple_variant_field_marked_deprecated-witness
branch
Adds a witness hint for the
enum_tuple_variant_field_marked_deprecatedlint.Example
enum_tuple_variant_field_marked_deprecatedoutputs the following witness hint for one of the./test_crates/enum_tuple_variant_field_marked_deprecated/tests.This compiles on both versions, but demonstrates the issue with the deprecated field, and would emit an error if compiled.
Commit Notes
enum_tuple_variant_field_marked_deprecatedenum_tuple_variant_field_marked_deprecatedto fetch relevant data for the witnessComments
Once again, similar to #1214 I wasn't 100% sure on what to call the bound variable (
to_be_deprecated) when destructuring the tuple, but I think this mostly works. Let me know if you think the name should get changed!