Skip to content

Commit

Permalink
refactor: add deserilization context
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos committed Oct 22, 2024
1 parent 2b9a69f commit 2da418c
Show file tree
Hide file tree
Showing 26 changed files with 583 additions and 466 deletions.
54 changes: 54 additions & 0 deletions .changeset/biome_deserialize_context
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
---
biome_deserialize: major
biome_deserialize_macros: major
---

## Replace `diagnostics` with `context`

All deserialization methods take a `name` parameter.
This parameter is used to name the deserialized value.
For instance, in the case of deserialized value from an object field, the name is set to the field name.
However, this parameter has been abused to pass data such as file names between deserializers.

When we introduced macros to automatically implement `Deserializable`, users request a way of passing the name parameter between deserializers.
Then, we added the `passthrough_name` attribute to satisfy the request.
As said before, this was never intended for this usage.
There are some inherent limitations, notably some deserializers such as the deserializers of `Vec` set `name` to the empty string for all deserialized items.

We now introduce **deserialization context** to provide a proper way to pass a filename or any string identifier to all deserializers.
All deserializers now take a context parameter that stores this identifier.
We take the opportunity to integrate diagnostic reporting directly in this deserialization context.

This is a **breaking change**.
You have to update all manual implementations of the `Deserializable` trait.
Let's take the deserializer of `Day` presented in the `biome_deserialize` [README](https://github.com/biomejs/biome/tree/main/crates/biome_deserialize) as an example for migrating to the new paradigm.
You have to remove the `diagnostics` parameter and to add a new `ctx` parameter.
Note that the diagnostics are now reported using `ctx.report()`.

```diff
impl Deserializable for Day {
fn deserialize(
+ ctx: &mut impl DeserializationContext,
value: &impl DeserializableValue,
name: &str,
- diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
// We deserialize the value into a number represented as a string.
let value_text = TextNumber::deserialize(ctx, value, name)?;
// We attempt to convert the string into a `Day`.
value_text.parse::<Day>().map_err(|error| {
// If the conversion failed, then we report the error.
- diagnostics.push(DeserializationDiagnostic::new(error).with_range(value.range()));
+ ctx.report(DeserializationDiagnostic::new(error).with_range(value.range()));
}).ok()
}
}
```

Biome provides `DefaultDeserializationContext` as a default implementation of `DeserializationContext`.
It is the implementation used by `biome_deserialize::deserialize_from_json_ast`, `biome_deserialize::deserialize_from_json_str`, or `biome_deserialize::::deserialize_from_str`.
The name (renamed `id`) passed as last parameter to `deserialize_from_json_ast` and `deserialize_from_json_str` now corresponds to the identifier that you can retrieve using `ctx.id()`.
This no longer sets the name of the deserialized root value.
The name of the deserialized root value is now the empty string.

Also, the `passthrough_name` macro attribute was removed because we now have a way of retrieving an identfiier from the context.
79 changes: 38 additions & 41 deletions crates/biome_cli/src/execute/migrate/eslint_eslint.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use biome_deserialize::{
Deserializable, DeserializableType, DeserializableTypes, DeserializableValue,
DeserializationDiagnostic, DeserializationVisitor, Merge,
DeserializationContext, DeserializationDiagnostic, DeserializationVisitor, Merge,
};
use biome_deserialize_macros::Deserializable;
use biome_rowan::TextRange;
Expand Down Expand Up @@ -125,15 +125,15 @@ impl Deref for IgnorePattern {
}
impl biome_deserialize::Deserializable for IgnorePattern {
fn deserialize(
ctx: &mut impl DeserializationContext,
value: &impl DeserializableValue,
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
let s = biome_deserialize::Text::deserialize(value, name, diagnostics)?;
let s = biome_deserialize::Text::deserialize(ctx, value, name)?;
match ignorefile::convert_pattern(s.text()) {
Ok(pattern) => Some(Self(pattern)),
Err(msg) => {
diagnostics.push(DeserializationDiagnostic::new(msg).with_range(value.range()));
ctx.report(DeserializationDiagnostic::new(msg).with_range(value.range()));
None
}
}
Expand Down Expand Up @@ -184,14 +184,14 @@ impl GlobalConf {
}
impl Deserializable for GlobalConf {
fn deserialize(
ctx: &mut impl DeserializationContext,
value: &impl biome_deserialize::DeserializableValue,
name: &str,
diagnostics: &mut Vec<biome_deserialize::DeserializationDiagnostic>,
) -> Option<Self> {
if value.visitable_type()? == DeserializableType::Str {
Deserializable::deserialize(value, name, diagnostics).map(Self::Qualifier)
Deserializable::deserialize(ctx, value, name).map(Self::Qualifier)
} else {
Deserializable::deserialize(value, name, diagnostics).map(Self::Flag)
Deserializable::deserialize(ctx, value, name).map(Self::Flag)
}
}
}
Expand Down Expand Up @@ -249,15 +249,15 @@ impl<T> IntoIterator for ShorthandVec<T> {
}
impl<T: Deserializable> Deserializable for ShorthandVec<T> {
fn deserialize(
ctx: &mut impl DeserializationContext,
value: &impl DeserializableValue,
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
Some(ShorthandVec(
if value.visitable_type()? == DeserializableType::Array {
Deserializable::deserialize(value, name, diagnostics)?
Deserializable::deserialize(ctx, value, name)?
} else {
Vec::from_iter([Deserializable::deserialize(value, name, diagnostics)?])
Vec::from_iter([Deserializable::deserialize(ctx, value, name)?])
},
))
}
Expand Down Expand Up @@ -306,9 +306,9 @@ impl<T: Default, U: Default> RuleConf<T, U> {
}
impl<T: Deserializable + 'static, U: Deserializable + 'static> Deserializable for RuleConf<T, U> {
fn deserialize(
ctx: &mut impl DeserializationContext,
value: &impl biome_deserialize::DeserializableValue,
name: &str,
diagnostics: &mut Vec<biome_deserialize::DeserializationDiagnostic>,
) -> Option<Self> {
struct Visitor<T, U>(PhantomData<(T, U)>);
impl<T: Deserializable + 'static, U: Deserializable + 'static> DeserializationVisitor
Expand All @@ -318,58 +318,58 @@ impl<T: Deserializable + 'static, U: Deserializable + 'static> Deserializable fo
const EXPECTED_TYPE: DeserializableTypes = DeserializableTypes::ARRAY;
fn visit_array(
self,
ctx: &mut impl DeserializationContext,
values: impl Iterator<Item = Option<impl DeserializableValue>>,
range: TextRange,
_name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self::Output> {
let mut values = values.flatten();
let Some(first_value) = values.next() else {
diagnostics.push(
ctx.report(
DeserializationDiagnostic::new("A severity is expected.").with_range(range),
);
return None;
};
let severity = Deserializable::deserialize(&first_value, "", diagnostics)?;
let severity = Deserializable::deserialize(ctx, &first_value, "")?;
if TypeId::of::<T>() == TypeId::of::<()>() {
return Some(RuleConf::Severity(severity));
}
let Some(second_value) = values.next() else {
return Some(RuleConf::Severity(severity));
};
let Some(option) = T::deserialize(&second_value, "", diagnostics) else {
let Some(option) = T::deserialize(ctx, &second_value, "") else {
// Recover by ignoring the failed deserialization
return Some(RuleConf::Severity(severity));
};
let Some(third_value) = values.next() else {
return Some(RuleConf::Option(severity, option));
};
if TypeId::of::<U>() != TypeId::of::<()>() {
if let Some(option2) = U::deserialize(&third_value, "", diagnostics) {
if let Some(option2) = U::deserialize(ctx, &third_value, "") {
return Some(RuleConf::Options(severity, option, option2));
} else {
// Recover by ignoring the failed deserialization
return Some(RuleConf::Option(severity, option));
}
}
let Some(option2) = T::deserialize(&third_value, "", diagnostics) else {
let Some(option2) = T::deserialize(ctx, &third_value, "") else {
// Recover by ignoring the failed deserialization
return Some(RuleConf::Option(severity, option));
};
let mut spread = Vec::new();
spread.push(option);
spread.push(option2);
spread.extend(values.filter_map(|val| T::deserialize(&val, "", diagnostics)));
spread.extend(values.filter_map(|val| T::deserialize(ctx, &val, "")));
Some(RuleConf::Spread(severity, spread))
}
}
if matches!(
value.visitable_type()?,
DeserializableType::Number | DeserializableType::Str
) {
Deserializable::deserialize(value, name, diagnostics).map(RuleConf::Severity)
Deserializable::deserialize(ctx, value, name).map(RuleConf::Severity)
} else {
value.deserialize(Visitor(PhantomData), name, diagnostics)
value.deserialize(ctx, Visitor(PhantomData), name)
}
}
}
Expand Down Expand Up @@ -417,14 +417,14 @@ enum NumberOrString {
}
impl Deserializable for NumberOrString {
fn deserialize(
ctx: &mut impl DeserializationContext,
value: &impl biome_deserialize::DeserializableValue,
name: &str,
diagnostics: &mut Vec<biome_deserialize::DeserializationDiagnostic>,
) -> Option<Self> {
Some(if value.visitable_type()? == DeserializableType::Str {
Self::String(Deserializable::deserialize(value, name, diagnostics)?)
Self::String(Deserializable::deserialize(ctx, value, name)?)
} else {
Self::Number(Deserializable::deserialize(value, name, diagnostics)?)
Self::Number(Deserializable::deserialize(ctx, value, name)?)
})
}
}
Expand All @@ -450,16 +450,17 @@ impl Deref for Rules {
}
impl Deserializable for Rules {
fn deserialize(
ctx: &mut impl DeserializationContext,
value: &impl biome_deserialize::DeserializableValue,
name: &str,
diagnostics: &mut Vec<biome_deserialize::DeserializationDiagnostic>,
) -> Option<Self> {
struct Visitor;
impl DeserializationVisitor for Visitor {
type Output = Rules;
const EXPECTED_TYPE: DeserializableTypes = DeserializableTypes::MAP;
fn visit_map(
self,
ctx: &mut impl DeserializationContext,
members: impl Iterator<
Item = Option<(
impl biome_deserialize::DeserializableValue,
Expand All @@ -468,57 +469,54 @@ impl Deserializable for Rules {
>,
_range: biome_rowan::TextRange,
name: &str,
diagnostics: &mut Vec<biome_deserialize::DeserializationDiagnostic>,
) -> Option<Self::Output> {
use biome_deserialize::Text;
let mut result = IndexSet::default();
for (key, value) in members.flatten() {
let Some(rule_name) = Text::deserialize(&key, "", diagnostics) else {
let Some(rule_name) = Text::deserialize(ctx, &key, "") else {
continue;
};
match rule_name.text() {
// Eslint rules with options that we handle
"no-console" => {
if let Some(conf) = RuleConf::deserialize(&value, name, diagnostics) {
if let Some(conf) = RuleConf::deserialize(ctx, &value, name) {
result.insert(Rule::NoConsole(conf));
}
}
"no-restricted-globals" => {
if let Some(conf) = RuleConf::deserialize(&value, name, diagnostics) {
if let Some(conf) = RuleConf::deserialize(ctx, &value, name) {
result.insert(Rule::NoRestrictedGlobals(conf));
}
}
// Eslint plugin rules with options that we handle
"jsx-a11y/aria-role" => {
if let Some(conf) = RuleConf::deserialize(&value, name, diagnostics) {
if let Some(conf) = RuleConf::deserialize(ctx, &value, name) {
result.insert(Rule::Jsxa11yArioaRoles(conf));
}
}
"@typescript-eslint/array-type" => {
if let Some(conf) = RuleConf::deserialize(&value, name, diagnostics) {
if let Some(conf) = RuleConf::deserialize(ctx, &value, name) {
result.insert(Rule::TypeScriptArrayType(conf));
}
}
"@typescript-eslint/explicit-member-accessibility" => {
if let Some(conf) = RuleConf::deserialize(&value, name, diagnostics) {
if let Some(conf) = RuleConf::deserialize(ctx, &value, name) {
result.insert(Rule::TypeScriptExplicitMemberAccessibility(conf));
}
}
"@typescript-eslint/naming-convention" => {
if let Some(conf) = RuleConf::deserialize(&value, name, diagnostics) {
if let Some(conf) = RuleConf::deserialize(ctx, &value, name) {
result.insert(Rule::TypeScriptNamingConvention(conf));
}
}
"unicorn/filename-case" => {
if let Some(conf) = RuleConf::deserialize(&value, name, diagnostics) {
if let Some(conf) = RuleConf::deserialize(ctx, &value, name) {
result.insert(Rule::UnicornFilenameCase(conf));
}
}
// Other rules
rule_name => {
if let Some(conf) =
RuleConf::<()>::deserialize(&value, name, diagnostics)
{
if let Some(conf) = RuleConf::<()>::deserialize(ctx, &value, name) {
result.insert(Rule::Any(
Cow::Owned(rule_name.to_string()),
conf.severity(),
Expand All @@ -530,7 +528,7 @@ impl Deserializable for Rules {
Some(Rules(result))
}
}
value.deserialize(Visitor, name, diagnostics)
value.deserialize(ctx, Visitor, name)
}
}

Expand Down Expand Up @@ -560,15 +558,14 @@ impl NoRestrictedGlobal {
}
impl Deserializable for NoRestrictedGlobal {
fn deserialize(
ctx: &mut impl DeserializationContext,
value: &impl DeserializableValue,
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
if value.visitable_type()? == DeserializableType::Str {
Deserializable::deserialize(value, name, diagnostics).map(NoRestrictedGlobal::Plain)
Deserializable::deserialize(ctx, value, name).map(NoRestrictedGlobal::Plain)
} else {
Deserializable::deserialize(value, name, diagnostics)
.map(NoRestrictedGlobal::WithMessage)
Deserializable::deserialize(ctx, value, name).map(NoRestrictedGlobal::WithMessage)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_cli/src/execute/migrate/eslint_typescript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,9 @@ impl NamingConventionSelection {
pub(crate) struct Anything;
impl Deserializable for Anything {
fn deserialize(
_ctx: &mut impl biome_deserialize::DeserializationContext,
_value: &impl biome_deserialize::DeserializableValue,
_name: &str,
_diagnostics: &mut Vec<biome_deserialize::DeserializationDiagnostic>,
) -> Option<Self> {
Some(Anything)
}
Expand Down
Loading

0 comments on commit 2da418c

Please sign in to comment.