Skip to content

Commit

Permalink
refactor(deserialize): generic config deserialization (#683)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos authored Nov 13, 2023
1 parent 484f451 commit e99387e
Show file tree
Hide file tree
Showing 75 changed files with 6,027 additions and 3,855 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

202 changes: 179 additions & 23 deletions crates/biome_analyze/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,52 +299,208 @@ just ready

### Rule configuration

Some rules may allow customization using configuration.
The first step is to setup a struct to represent the rule configuration.
Some rules may allow customization using options.
We try to keep rule options to a minimum and only when needed.
Before adding an option, it's worth a discussion.
Options should follow our [technical philosophy](https://biomejs.dev/internals/philosophy/#technical).

```rust,ignore
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ReactExtensiveDependenciesOptions {
hooks_config: FxHashMap<String, ReactHookConfiguration>,
stable_config: FxHashSet<StableReactHookConfiguration>,
}
Let's assume that the rule we implement support the following options:

impl Rule for UseExhaustiveDependencies {
type Query = Semantic<JsCallExpression>;
type State = Fix;
type Signals = Vec<Self::State>;
type Options = ReactExtensiveDependenciesOptions;
- `behavior`: a string among `"A"`, `"B"`, and `"C"`;
- `threshold`: an integer between 0 and 255;
- `behaviorExceptions`: an array of strings.

...
}
```

This allows the rule to be configured inside `biome.json` file like:
We would like to set the options in the `biome.json` configuration file:

```json
{
"linter": {
"rules": {
"recommended": true,
"nursery": {
"useExhaustiveDependencies": {
"level": "error",
"options": {
"hooks": [["useMyEffect", 0, 1]]
}
"my-rule": {
"behavior": "A",
"threshold": 30,
"behaviorExceptions": ["f"],
}
}
}
}
}
```

The first step is to create the Rust data representation of the rule's options.

```rust,ignore
#[derive(Debug, Default, Clone)]
pub struct MyRuleOptions {
behavior: Behavior,
threshold: u8,
behavior_exceptions: Vec<String>
}
#[derive(Debug, Default, Clone)]
pub enum Behavior {
#[default]
A,
B,
C,
}
```

To allow deserializing instances of the types `MyRuleOptions` and `Behavior`,
they have to implement the `Deserializable` trait from the `biome_deserialize` crate.

In the following code, we implement `Deserializable` for `Behavior`.
We first deserialize the input into a `TokenText`.
Then we validate the retrieved text by checking that it is one of the allowed string variants.
If it is an unknown variant, we emit a diagnostic and return `None` to signal that the deserialization failed.
Otherwise, we return the corresponding variant.

```rust,ignore
use biome_deserialize::{Deserializable, DeserializableValue, DeserializationVisitor, Text};
impl Deserializable for Behavior {
fn deserialize(
value: &impl DeserializableValue,
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
match Text::deserialize(&value, name, diagnostics)?.text() {
"A" => Some(Behavior::A),
"B" => Some(Behavior::B),
"C" => Some(Behavior::C),
unknown_variant => {
const ALLOWED_VARIANTS: &[&str] = &["A", "B", "C"];
diagnostics.push(DeserializationDiagnostic::new_unknown_value(
unknown_variant,
value.range(),
ALLOWED_VARIANTS,
));
None
}
}
}
}
```

To implement `Deserializable` for `MyRuleOptions`,
we cannot reuse an existing deserializer because a `struct` has custom fields.
Instead, we delegate the deserialization to a visitor.
We implement a visitor by implementing the `DeserializationVisitor` trait from the `biome_deserialize` crate.
The visitor traverses every field (key-value pair) of our object and deserialize them.
If an unknown field is found, we emit a diagnostic.

```rust,ignore
use biome_deserialize::{DeserializationDiagnostic, Deserializable, DeserializableValue, DeserializationVisitor, Text, VisitableType};
impl Deserializable for MyRuleOptions {
fn deserialize(
value: &impl DeserializableValue,
name: &str,
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
value.deserialize(MyRuleOptionsVisitor, name, diagnostics)
}
}
struct MyRuleOptionsVisitor;
impl DeserializationVisitor for MyRuleOptionsVisitor {
type Output = MyRuleOptions;
const EXPECTED_TYPE: VisitableType = VisitableType::MAP;
fn visit_map(
self,
members: impl Iterator<Item = Option<(impl DeserializableValue, impl DeserializableValue)>>,
_name: &str,
_range: TextRange,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self::Output> {
let mut result = Self::Output::default();
for (key, value) in members.flatten() {
let Some(key_text) = Text::deserialize(&key, "", diagnostics) else {
continue;
};
match key_text.text() {
"behavior" => {
if let Some(behavior) = Deserialize::deserialize(&value, &key_text, diagnostics) {
result.behavior = behavior;
}
}
"threshold" => {
if let Some(threshold) = Deserialize::deserialize(&value, &key_text, diagnostics) {
result.behavior = threshold;
}
}
"behaviorExceptions" => {
if let Some(exceptions) = Deserialize::deserialize(&value, &key_text, diagnostics) {
result.behavior_exceptions = exceptions;
}
}
unknown_key => {
const ALLOWED_KEYS: &[&str] = &["behavior", "threshold", "behaviorExceptions"];
diagnostics.push(DeserializationDiagnostic::new_unknown_key(
unknown_key,
key.range(),
ALLOWED_KEYS,
))
}
}
}
Some(result)
}
}
```

Once done, you can set the associated type `Options` of the rule:

```rust,ignore
impl Rule for MyRule {
type Query = Semantic<JsCallExpression>;
type State = Fix;
type Signals = Vec<Self::State>;
type Options = MyRuleOptions;
...
}
```

A rule can retrieve its option with:

```rust,ignore
let options = ctx.options();
```

The compiler should warn you that `MyRuleOptions` does not implement some required types.
We currently require implementing _serde_'s traits `Deserialize`/`Serialize` and _Bpaf_'s parser trait.
You can simply use a derive macros:

```rust,ignore
#[derive(Debug, Default, Clone, Serialize, Deserialize, Bpaf)]
#[cfg_attr(feature = "schemars", derive(JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct MyRuleOptions {
#[bpaf(hide)]
#[serde(default, skip_serializing_if = "is_default")]
main_behavior: Behavior,
#[bpaf(hide)]
#[serde(default, skip_serializing_if = "is_default")]
extra_behaviors: Vec<Behavior>,
}
#[derive(Debug, Default, Clone)]
#[cfg_attr(feature = "schemars", derive(JsonSchema))]
pub enum Behavior {
#[default]
A,
B,
C,
}
```

### Deprecate a rule

There are occasions when a rule must be deprecated, to avoid breaking changes. The reason
Expand Down
1 change: 1 addition & 0 deletions crates/biome_cli/src/commands/rage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ impl Display for RageConfiguration<'_, '_> {
Ok(None) => KeyValuePair("Status", markup!(<Dim>"unset"</Dim>)).fmt(fmt)?,
Ok(Some(result)) => {
let (configuration, diagnostics) = result.deserialized.consume();
let configuration = configuration.unwrap_or_default();
let status = if !diagnostics.is_empty() {
for diagnostic in diagnostics {
(markup! {
Expand Down
11 changes: 7 additions & 4 deletions crates/biome_cli/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ impl LoadedConfiguration {
/// If a configuration can't be resolved from the file system, the operation will fail.
pub fn apply_extends(mut self, fs: &DynRef<dyn FileSystem>) -> Result<Self, WorkspaceError> {
let deserialized = self.deserialize_extends(fs)?;
let (configurations, errors): (Vec<_>, Vec<_>) =
deserialized.into_iter().map(|d| d.consume()).unzip();
let (configurations, errors): (Vec<_>, Vec<_>) = deserialized
.into_iter()
.map(|d| d.consume())
.map(|(config, diagnostics)| (config.unwrap_or_default(), diagnostics))
.unzip();

let extended_configuration = configurations.into_iter().reduce(
|mut previous_configuration, current_configuration| {
Expand Down Expand Up @@ -72,7 +75,7 @@ impl LoadedConfiguration {
.cloned()
.unwrap_or(fs.working_directory().unwrap_or(PathBuf::from("./")));
let mut deserialized_configurations = vec![];
for path in extends.index_set() {
for path in extends.iter() {
let config_path = directory_path.join(path);
let mut file = fs
.open_with_options(config_path.as_path(), OpenOptions::default().read(true))
Expand Down Expand Up @@ -172,7 +175,7 @@ impl From<Option<ConfigurationPayload>> for LoadedConfiguration {
} = value;
let (configuration, diagnostics) = deserialized.consume();
LoadedConfiguration {
configuration,
configuration: configuration.unwrap_or_default(),
diagnostics,
directory_path: Some(configuration_directory_path),
file_path: Some(configuration_file_path),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ flags/invalid ━━━━━━━━━━━━━━━━━━━━━━
× Failed to parse CLI arguments.
Caused by:
couldn't parse `321`: The line width exceeds the maximum value (320)
couldn't parse `321`: The line width should be between 1 and 320
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ configuration ━━━━━━━━━━━━━━━━━━━━━━
```block
biome.json:6:17 deserialize ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Incorrect type, expected a string.
× Incorrect type, expected a string, but received a boolean.
4 │ },
5 │ "javascript": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ configuration ━━━━━━━━━━━━━━━━━━━━━━
```block
biome.json:3:18 deserialize ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× The line width exceeds the maximum value (320)
× The number should be an integer between 1 and 320.
1 │ {
2 │ "formatter": {
Expand All @@ -38,8 +37,6 @@ biome.json:3:18 deserialize ━━━━━━━━━━━━━━━━━
4 │ }
5 │ }
i Maximum value accepted is 320
```

Expand Down
1 change: 1 addition & 0 deletions crates/biome_deserialize/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ biome_diagnostics = { workspace = true }
biome_json_parser = { workspace = true }
biome_json_syntax = { workspace = true }
biome_rowan = { workspace = true }
bitflags = { workspace = true }
indexmap = { workspace = true, features = ["serde"] }
schemars = { workspace = true, optional = true }
serde = { workspace = true }
Expand Down
Loading

0 comments on commit e99387e

Please sign in to comment.