refactor: take review into account

crates/biome_analyze/CONTRIBUTING.md

@@ -299,17 +299,47 @@ just ready
 
 ### Rule configuration
 
-Some rules may allow customization using configuration.
-Biome tries to introduce a minimum of the rule configuration.
-Before adding an option discuss that.
+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).
 
-The first step is to create the data representation of the rule's configuration.
+Rule options are set in the `biome.json` configuration file.
+For example, `useNamingConvention` have two options: `strictCase` and `enumMemberCase`:
+
+Let's assume that the rule we implement support the following options:
+
+- `behavior`: a string among `"A"`, `"B"`, and `"C"`;
+- `threshold`: an integer between 0 and 255;
+- `behaviorExceptions`: an array of strings.
+
+We would like to set the options in the `biome.json` configuration file:
+
+```json
+{
+  "linter": {
+    "rules": {
+      "recommended": true,
+      "nursery": {
+        "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 GreatRuleOptions {
-    main_behavior: Behavior,
-    extra_behaviors: Vec<Behavior>,
+pub struct MyRuleOptions {
+    behavior: Behavior,
+    threshold: u8,
+    behavior_exceptions: Vec<String>
 }
 
 #[derive(Debug, Default, Clone)]
@@ -321,24 +351,18 @@ pub enum Behavior {
 }
 ```
 
-You also need to picture the equivalent data representation in _JSON_:
-
-```json
-{
-    "mainBehavior": "A",
-    "extraBehaviors": ["C"]
-}
-```
+To allow deserializing instances of the types `MyRuleOptions` and `Behavior`,
+they have to implement the `Deserializable` trait from the `biome_deserialize` crate.
 
-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.
+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};
+
 impl Deserializable for Behavior {
     fn deserialize(
         value: impl DeserializableValue,
@@ -348,9 +372,9 @@ impl Deserializable for Behavior {
         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),
+            "A" => Some(Behavior::A),
+            "B" => Some(Behavior::B),
+            "C" => Some(Behavior::C),
             _ => {
                 diagnostics.push(DeserializationDiagnostic::new_unknown_value(
                     value.text(),
@@ -364,57 +388,58 @@ impl Deserializable for Behavior {
 }
 ```
 
-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.
+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
-impl Deserializable for GreatRuleOptions {
+use biome_deserialize::{DeserializationDiagnostic,  Deserializable, DeserializableValue, DeserializationVisitor, VisitableType};
+
+impl Deserializable for MyRuleOptions {
     fn deserialize(
         value: impl DeserializableValue,
         diagnostics: &mut Vec<DeserializationDiagnostic>,
     ) -> Option<Self> {
-        value.deserialize(GreatRuleOptionsVisitor, diagnostics)
+        value.deserialize(MyRuleOptionsVisitor, diagnostics)
     }
 }
 
-struct GreatRuleOptionsVisitor;
-impl DeserializationVisitor for GreatRuleOptionsVisitor {
-    type Output = GreatRuleOptions;
+struct MyRuleOptionsVisitor;
+impl DeserializationVisitor for MyRuleOptionsVisitor {
+    type Output = MyRuleOptions;
 
-    const EXPECTED_TYPE: ExpectedType = ExpectedType::MAP;
+    const EXPECTED_TYPE: VisitableType = VisitableType::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"];
+        const ALLOWED_KEYS: &[&str] = &["behavior", "threshold", "behaviorExceptions"];
         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;
+                "behavior" => {
+                    if let Some(behavior) = Deserialize::deserialize(value, diagnostics) {
+                        result.behavior = behavior;
                     }
                 }
-                "extraBehavior" => {
-                    if let Some(enum_member_case) = Deserialize::deserialize(value, diagnostics) {
-                        result.extra_behavior = enum_member_case;
+                "threshold" => {
+                    if let Some(threshold) = Deserialize::deserialize(value, diagnostics) {
+                        result.behavior = threshold;
+                    }
+                }
+                "behaviorExceptions" => {
+                    if let Some(exceptions) = Deserialize::deserialize(value, diagnostics) {
+                        result.behavior_exceptions = exceptions;
                     }
                 }
                 _ => diagnostics.push(DeserializationDiagnostic::new_unknown_key(
@@ -431,7 +456,6 @@ impl DeserializationVisitor for GreatRuleOptionsVisitor {
 
 Once done, you can set the associated type `Options` of the rule:
 
-
 ```rust,ignore
 impl Rule for GreatRule {
     type Query = Semantic<JsCallExpression>;
@@ -443,26 +467,6 @@ impl Rule for GreatRule {
 }
 ```
 
-This allows the rule to be configured inside `biome.json` file like:
-
-```json
-{
-  "linter": {
-    "rules": {
-      "recommended": true,
-      "nursery": {
-        "greatRule": {
-          "level": "error",
-          "options": {
-            "mainBehavior": "A"
-          }
-        }
-      }
-    }
-  }
-}
-```
-
 A rule can retrieve its option with:
 
 ```rust,ignore

crates/biome_cli/tests/snapshots/main_configuration/incorrect_globals.snap

@@ -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": {

crates/biome_deserialize/README.md

@@ -0,0 +1 @@
+# `biome_deserialize`

crates/biome_deserialize/src/diagnostics.rs

@@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize};
 
 bitflags! {
     #[derive(Debug, Copy, Clone, Eq, PartialEq)]
-    pub struct ExpectedType: u8 {
+    pub struct VisitableType: u8 {
         const NULL = 1 << 0;
         const BOOL = 1 << 1;
         const NUMBER = 1 << 2;
@@ -20,27 +20,25 @@ bitflags! {
     }
 }
 
-impl Display for ExpectedType {
-    fn fmt(&self, fmt: &mut biome_console::fmt::Formatter) -> std::io::Result<()> {
+impl std::fmt::Display for VisitableType {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::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 ")?;
+        for (i, expected_type) in self.iter().enumerate() {
+            if i != 0 {
+                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",
+                VisitableType::NULL => "null",
+                VisitableType::BOOL => "a boolean",
+                VisitableType::NUMBER => "a number",
+                VisitableType::STR => "a string",
+                VisitableType::ARRAY => "an array",
+                VisitableType::MAP => "an object",
                 _ => unreachable!("Unhandled deserialization type."),
             };
             write!(fmt, "{}", expected_type)?;
-            is_not_first = true;
         }
         Ok(())
     }
@@ -75,9 +73,13 @@ impl DeserializationDiagnostic {
     }
 
     /// Emitted when a generic node has an incorrect type
-    pub fn new_incorrect_type(expected_type: ExpectedType, range: impl AsSpan) -> Self {
+    pub fn new_incorrect_type(
+        actual_type: VisitableType,
+        expected_type: VisitableType,
+        range: impl AsSpan,
+    ) -> Self {
         Self::new(markup! {
-            "Incorrect type, expected "<Emphasis>{expected_type}"."</Emphasis>
+            "Incorrect type, expected "<Emphasis>{format_args!("{}", expected_type)}</Emphasis>", but received "<Emphasis>{format_args!("{}", actual_type)}</Emphasis>"."
         })
         .with_range(range)
     }
@@ -198,3 +200,22 @@ impl Advices for DeserializationAdvice {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_visitable_type_fmt() {
+        assert_eq!(VisitableType::empty().to_string(), "no value");
+        assert_eq!(VisitableType::NULL.to_string(), "null");
+        assert_eq!(
+            VisitableType::NULL.union(VisitableType::BOOL).to_string(),
+            "null, or a boolean"
+        );
+        assert_eq!(
+            VisitableType::all().to_string(),
+            "null, or a boolean, or a number, or a string, or an array, or an object"
+        );
+    }
+}