scylla-macros: attributes for better control over name checks #882
Conversation
In some cases, it might be desirable to name the Rust struct fields differently than the UDT fields (e.g. due to a different naming convention). Add an attribute to the `SerializeCql` macro, inspired by serde's `rename` attribute, which causes the fields to be matched on a specified name instead of the Rust name.
Like we just did for `SerializeCql`, introduce a `rename` annotation to `SerializeRow` which allows associating given Rust field to a column / bind marker with given name, instead of matching on the Rust field name. The motivation is similar as in the case of `SerializeCql`: sometimes, users might want to use a different naming scheme in Rust and for the column names.
Implement `Default` for `Flavor`, which allows to get rid of the `Option` wrapper in the macro attributes struct and simplify some of the code as a result.
Introduce an attribute to the `SerializeCql` macro which causes the generated code to skip checking names of the serialized Rust fields against the UDT field names. The motivation behind the attribute is to ease transition off the old `IntoUserType` macro which blindly serialized the fields of the struct as they are defined in the Rust code. While it disables the name checks, type checking is still done, so there is protection against type confusion, at least.
Introduces an attribute to `SerializeRow` macro which causes name checks to be skipped when serializing in `enforce_order` flavor. The motivation is the same as in the case of `SerializeCql` - the old `ValueList` macro didn't have access to the bind marker names and types, and disabling the name check might make it easier for some to transition off the old macro.
| /// `#[scylla(skip_name_checks)] | ||
| /// | ||
| /// _Specific only to the `enforce_order` flavor._ | ||
| /// | ||
| /// Skips checking Rust field names against names of the UDT fields. With this | ||
| /// annotation, the generated implementation will allow mismatch between Rust | ||
| /// struct field names and UDT field names, i.e. it's OK if i-th field has a | ||
| /// different name in Rust and in the UDT. Fields are still being type-checked. | ||
| /// |
There was a problem hiding this comment.
I'm wondering if it wouldn't be better to make this a new flavor instead of an attribute - one less place to make a mistake for a user. But seeing that it's a compile time error, maybe it doesn't matter?
There was a problem hiding this comment.
I'm not entirely sure either; implementation-wise it was more natural to add an attribute as it introduces very slight changes to how the enforce_order flavor generates code.
One argument that I see for leaving it as a separate attribute is that it disables some safety checks, while both current flavors don't compromise on safety. It's good to make the less safe option more verbose IMO.
In the future we should consider adding support for tuple structs which very natually fit into the enforce_order + skip_name_checks pattern, we could probably remove the current skip_name_checks attribute when doing that.
The PR introduces two attributes related to matching Rust struct fields by name in the new
SerializeRow/SerializeCqlmacros:#[scylla(rename = "xyz")]- instead of using the name of the field verbatim, the name provided in the attribute is used when matching to a column/bind marker/UDT field. This can be useful e.g. if the user has different naming convention in Rust code and in the schema definitions.#[scylla(skip_name_checks)] - inenforce_orderflavor, skips checking the names of the fields as they are being serialized. With this attribute, the names of the corresponding fields in Rust struct and UDT/columns/bind markers don't have to match at all, but their types still need to. This attribute is supposed to help migrate from the oldIntoUserType/ValueListmacros.Refs: #802
Pre-review checklist
I have adjusted the documentation in./docs/source/.I added appropriateFixes:annotations to PR description.