Skip to content

Commit

Permalink
refactor(deserialize): new abstractions for generic deserialization
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos committed Nov 9, 2023
1 parent 8475169 commit 959db97
Show file tree
Hide file tree
Showing 68 changed files with 4,888 additions and 3,740 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.

170 changes: 161 additions & 9 deletions crates/biome_analyze/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -300,20 +300,144 @@ just ready
### Rule configuration

Some rules may allow customization using configuration.
The first step is to setup a struct to represent the rule configuration.
Biome tries to introduce a minimum of the rule configuration.
Before adding an option discuss that.

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

```rust,ignore
#[derive(Debug, Default, Clone)]
pub struct GreatRuleOptions {
main_behavior: Behavior,
extra_behaviors: Vec<Behavior>,
}
#[derive(Debug, Default, Clone)]
pub enum Behavior {
#[default]
A,
B,
C,
}
```

You also need to picture the equivalent data representation in _JSON_:

```json
{
"mainBehavior": "A",
"extraBehaviors": ["C"]
}
```

So, you have to implement the `Deserializable` trait for these two types.
An implementation can reuse an existing type that implements `Deserializable`.
For example, we could deserialize `Behavior` by first deserializing a string,
and then checking that the string is either `A`, `B`, or `C`.
This is what we do in the following code snippet.
Note that, instead of using `String`, we use `TokenText`.
This avoids a string allocation.

```rust,ignore
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ReactExtensiveDependenciesOptions {
hooks_config: FxHashMap<String, ReactHookConfiguration>,
stable_config: FxHashSet<StableReactHookConfiguration>,
impl Deserializable for Behavior {
fn deserialize(
value: impl DeserializableValue,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
const ALLOWED_VARIANTS: &[&str] = &["A", "B", "C"];
let range = value.range();
let value = TokenText::deserialize(value, diagnostics)?;
match value.text() {
"A" => Some(Behavior.A),
"B" => Some(Behavior.B),
"C" => Some(Behavior.C),
_ => {
diagnostics.push(DeserializationDiagnostic::new_unknown_value(
value.text(),
range,
ALLOWED_VARIANTS,
));
None
}
}
}
}
```

Implementing `Deserializable` for `GreatRuleOptions` requires more work,
because we cannot rely on an existing deserializable type.
We have to use a _deserialization visitor_.
We create a visitor by creating a zero-sized `struct` that implements `DeserializationVisitor`.
A visitor must specify the type that it produces in its associated type `Output`.
Here the visitor produces a `GreatRuleOptions`.
It must also specify which type is expected with the associated constant `EXPECTED_TYPE`.
Here we deserialize an object (a _map_ of string-value pairs).
Thus, it expects a `ExpectedType::MAP`.
So we implement `visit_map` that traverses the key-value pairs,
deserializes every key as a string (a token text to avoid allocating a string),
and deserializes the value based on the key.

impl Rule for UseExhaustiveDependencies {
```rust,ignore
impl Deserializable for GreatRuleOptions {
fn deserialize(
value: impl DeserializableValue,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
value.deserialize(GreatRuleOptionsVisitor, diagnostics)
}
}
struct GreatRuleOptionsVisitor;
impl DeserializationVisitor for GreatRuleOptionsVisitor {
type Output = GreatRuleOptions;
const EXPECTED_TYPE: ExpectedType = ExpectedType::MAP;
fn visit_map(
self,
members: impl Iterator<Item = (impl DeserializableValue, impl DeserializableValue)>,
_range: TextRange,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self::Output> {
const ALLOWED_KEYS: &[&str] = &["mainBehavior", "extraBehavior"];
let mut result = Self::Output::default();
for (key, value) in members {
let key_range = key.range();
let Some(key) = TokenText::deserialize(key, diagnostics) else {
continue;
};
match key.text() {
"mainBehavior" => {
if let Some(strict_case) = Deserialize::deserialize(value, diagnostics) {
result.main_behavior = value;
}
}
"extraBehavior" => {
if let Some(enum_member_case) = Deserialize::deserialize(value, diagnostics) {
result.extra_behavior = enum_member_case;
}
}
_ => diagnostics.push(DeserializationDiagnostic::new_unknown_key(
key.text(),
key_range,
ALLOWED_KEYS,
)),
}
}
Some(result)
}
}
```

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


```rust,ignore
impl Rule for GreatRule {
type Query = Semantic<JsCallExpression>;
type State = Fix;
type Signals = Vec<Self::State>;
type Options = ReactExtensiveDependenciesOptions;
type Options = GreatRuleOptions;
...
}
Expand All @@ -327,10 +451,10 @@ This allows the rule to be configured inside `biome.json` file like:
"rules": {
"recommended": true,
"nursery": {
"useExhaustiveDependencies": {
"greatRule": {
"level": "error",
"options": {
"hooks": [["useMyEffect", 0, 1]]
"mainBehavior": "A"
}
}
}
Expand All @@ -345,6 +469,34 @@ A rule can retrieve its option with:
let options = ctx.options();
```

The compiler should warn you that `GreatRuleOptions` 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 GreatRuleOptions {
#[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
9 changes: 6 additions & 3 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 @@ -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 @@ -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
60 changes: 50 additions & 10 deletions crates/biome_deserialize/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,47 @@ use biome_diagnostics::{
Advices, Diagnostic, DiagnosticTags, LogCategory, MessageAndDescription, Severity, Visit,
};
use biome_rowan::{SyntaxError, TextRange};
use bitflags::bitflags;
use serde::{Deserialize, Serialize};

bitflags! {
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub struct ExpectedType: u8 {
const NULL = 1 << 0;
const BOOL = 1 << 1;
const NUMBER = 1 << 2;
const STR = 1 << 3;
const ARRAY = 1 << 4;
const MAP = 1 << 5;
}
}

impl Display for ExpectedType {
fn fmt(&self, fmt: &mut biome_console::fmt::Formatter) -> std::io::Result<()> {
if self.is_empty() {
return write!(fmt, "no value");
}
let mut is_not_first = false;
for expected_type in self.iter() {
if is_not_first {
write!(fmt, " or ")?;
}
let expected_type = match expected_type {
ExpectedType::NULL => "null",
ExpectedType::BOOL => "a boolean",
ExpectedType::NUMBER => "a number",
ExpectedType::STR => "a string",
ExpectedType::ARRAY => "an array",
ExpectedType::MAP => "an object",
_ => unreachable!("Unhandled deserialization type."),
};
write!(fmt, "{}", expected_type)?;
is_not_first = true;
}
Ok(())
}
}

/// Diagnostic emitted during the deserialization
#[derive(Debug, Serialize, Clone, Deserialize, Diagnostic)]
#[diagnostic(category = "deserialize")]
Expand Down Expand Up @@ -35,21 +74,22 @@ impl DeserializationDiagnostic {
}
}

/// Emitted when the type of a value is incorrect.
pub fn new_incorrect_type_for_value(
key_name: impl Display,
expected_type: impl Display,
range: impl AsSpan,
) -> Self {
/// Emitted when a generic node has an incorrect type
pub fn new_incorrect_type(expected_type: ExpectedType, range: impl AsSpan) -> Self {
Self::new(markup! {
"The value of key "<Emphasis>{{key_name}}</Emphasis>" is incorrect. Expected a "<Emphasis>{{expected_type}}"."</Emphasis>
}).with_range(range)
"Incorrect type, expected "<Emphasis>{expected_type}"."</Emphasis>
})
.with_range(range)
}

/// Emitted when a generic node has an incorrect type
pub fn new_incorrect_type(expected_type: impl Display, range: impl AsSpan) -> Self {
pub fn new_out_of_bound_integer(
min: impl Display,
max: impl Display,
range: impl AsSpan,
) -> Self {
Self::new(markup! {
"Incorrect type, expected a "<Emphasis>{{expected_type}}"."</Emphasis>
"The number should be an integer between "<Emphasis>{min}</Emphasis>" and "<Emphasis>{max}</Emphasis>"."
})
.with_range(range)
}
Expand Down
Loading

0 comments on commit 959db97

Please sign in to comment.