fix(lint): options for noLabelWithoutControl are optional
#4138
Conversation
github-actions
bot
added
A-Linter
CodSpeed Performance ReportMerging #4138 will degrade performances by 6.34%Comparing Summary
Benchmarks breakdown
|
| @@ -159,14 +152,124 @@ impl Rule for NoLabelWithoutControl { | |||
|
|
|||
| #[derive(Clone, Debug, Default, Deserialize, Deserializable, Eq, PartialEq, Serialize)] | |||
| #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] | |||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | |||
| #[serde(rename_all = "camelCase", deny_unknown_fields, default)] | |||
There was a problem hiding this comment.
Using the serde(default) attribute should be enough to make optional the fields.
Is there an interest to have both Option<> and serde(default)?
There was a problem hiding this comment.
Agreed it might make more sense to use plain Vecs only. It uses less memory, and makes consuming code easier. The only exception should be when an empty vector and None are semantically different.
There was a problem hiding this comment.
I'll try the serde(default), but the issue is the schema, not the default values.
| @@ -161,16 +161,16 @@ We would like to set the options in the `biome.json` configuration file: | |||
| } | |||
| ``` | |||
|
|
|||
| The first step is to create the Rust data representation of the rule's options. | |||
| The first step is to create the Rust data representation of the rule's options. Each option must be wrapped in a `Option`, this is required so the configuration schema won't mark them as required. | |||
There was a problem hiding this comment.
And then I see this as motivation for the Option wrappers :) Could we make the configuration schema respect the default annotations in addition to only Option?
Co-authored-by: Arend van Beelen jr. <[email protected]>
Summary
Closes #4125
I updated the contribution guide too. I took the opportunity to do some small refactor, and move functions inside the options struct, so we don't pass around the same object.
Test Plan
CI should pass. I checked the code generated files manually.