Repository: ast-grep/ast-grep (language: rust)

Issue:
[bug] YAML Serialization Issue with SerializableStopBy in ast-grep
### Please read the FAQ for the bug you encountered.

- [x] I have read the existing FAQ

### ⏯ Playground Link

https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6ImNwcCIsInF1ZXJ5IjoiSTE4blJpdFR5cGU6OiQiLCJyZXdyaXRlIjoiIiwic3RyaWN0bmVzcyI6ImNzdCIsInNlbGVjdG9yIjoiIiwiY29uZmlnIjoicnVsZTpcbiAga2luZDogZmllbGRfZGVjbGFyYXRpb25cbiAgYW55OlxuICAtIGhhczpcbiAgICAgIG5vdDpcbiAgICAgICAgcmVnZXg6IF5bYS16XVthLXowLTldKihfW2EtejAtOV0rKSpfJFxuICAgICAgbWF0Y2hlczogTUVNQkVSX1ZBUklBQkxFXG4gICAgICBzdG9wQnk6IGVuZFxuICAgICAgZmllbGQ6IGRlY2xhcmF0b3JcbiAgICBtYXRjaGVzOiBQUklWQVRFX0ZJRUxEX0RFQ0xBUkFUSU9OXG4gIC0gaGFzOlxuICAgICAgbm90OlxuICAgICAgICByZWdleDogXlthLXpdW2EtejAtOV0qKF9bYS16MC05XSspKl8/JFxuICAgICAgbWF0Y2hlczogTUVNQkVSX1ZBUklBQkxFXG4gICAgICBzdG9wQnk6IGVuZFxuICAgICAgZmllbGQ6IGRlY2xhcmF0b3JcbiAgICBtYXRjaGVzOiBQVUJMSUNfRklFTERfREVDTEFSQVRJT05cbiAgbm90OlxuICAgIG1hdGNoZXM6IENPTlNUX1RZUEVfUVVBTElGSUVSXG5jb25zdHJhaW50czogbnVsbFxudXRpbHM6XG4gIFBSSVZBVEVfRklFTERfREVDTEFSQVRJT046XG4gICAgYW55OlxuICAgIC0gaW5zaWRlOlxuICAgICAgICBraW5kOiBmaWVsZF9kZWNsYXJhdGlvbl9saXN0XG4gICAgICAgIGluc2lkZTpcbiAgICAgICAgICBraW5kOiBjbGFzc19zcGVjaWZpZXJcbiAgICAgICAgICBzdG9wQnk6IG5laWdoYm9yXG4gICAgICAgICAgZmllbGQ6IG51bGxcbiAgICAgICAgc3RvcEJ5OiBuZWlnaGJvclxuICAgICAgICBmaWVsZDogbnVsbFxuICAgICAgbm90OlxuICAgICAgICBmb2xsb3dzOlxuICAgICAgICAgIHBhdHRlcm46XG4gICAgICAgICAgICBjb250ZXh0OiBjbGFzcyB7cHVibGljOn07XG4gICAgICAgICAgICBzZWxlY3RvcjogYWNjZXNzX3NwZWNpZmllclxuICAgICAgICAgICAgc3RyaWN0bmVzczogbnVsbFxuICAgICAgICAgIHN0b3BCeTogIXJ1bGVcbiAgICAgICAgICAgIGtpbmQ6IGFjY2Vzc19zcGVjaWZpZXJcbiAgICAgICAgICBmaWVsZDogbnVsbFxuICAgIC0gaW5zaWRlOlxuICAgICAgICBraW5kOiBmaWVsZF9kZWNsYXJhdGlvbl9saXN0XG4gICAgICAgIGluc2lkZTpcbiAgICAgICAgICBraW5kOiBzdHJ1Y3Rfc3BlY2lmaWVyXG4gICAgICAgICAgc3RvcEJ5OiBuZWlnaGJvclxuICAgICAgICAgIGZpZWxkOiBudWxsXG4gICAgICAgIHN0b3BCeTogbmVpZ2hib3JcbiAgICAgICAgZmllbGQ6IG51bGxcbiAgICAgIGFueTpcbiAgICAgIC0gZm9sbG93czpcbiAgICAgICAgICBwYXR0ZXJuOlxuICAgICAgICAgICAgY29udGV4dDogc3RydWN0IHtwcml2YXRlOn07XG4gICAgICAgICAgICBzZWxlY3RvcjogYWNjZXNzX3NwZWNpZmllclxuICAgICAgICAgICAgc3RyaWN0bmVzczogbnVsbFxuICAgICAgICAgIHN0b3BCeTogIXJ1bGVcbiAgICAgICAgICAgIGtpbmQ6IGFjY2Vzc19zcGVjaWZpZXJcbiAgICAgICAgICBmaWVsZDogbnVsbFxuICAgICAgLSBmb2xsb3dzOlxuICAgICAgICAgIHBhdHRlcm46XG4gICAgICAgICAgICBjb250ZXh0OiBzdHJ1Y3Qge3Byb3RlY3RlZDp9O1xuICAgICAgICAgICAgc2VsZWN0b3I6IGFjY2Vzc19zcGVjaWZpZXJcbiAgICAgICAgICAgIHN0cmljdG5lc3M6IG51bGxcbiAgICAgICAgICBzdG9wQnk6ICFydWxlXG4gICAgICAgICAgICBraW5kOiBhY2Nlc3Nfc3BlY2lmaWVyXG4gICAgICAgICAgZmllbGQ6IG51bGxcbiAgUFVCTElDX0ZJRUxEX0RFQ0xBUkFUSU9OOlxuICAgIG5vdDpcbiAgICAgIG1hdGNoZXM6IFBSSVZBVEVfRklFTERfREVDTEFSQVRJT05cbiAgQ09OU1RfVFlQRV9RVUFMSUZJRVI6XG4gICAgYW55OlxuICAgIC0gaGFzOlxuICAgICAgICBwYXR0ZXJuOlxuICAgICAgICAgIGNvbnRleHQ6IGNvbnN0IGludCBhID0gMDtcbiAgICAgICAgICBzZWxlY3RvcjogdHlwZV9xdWFsaWZpZXJcbiAgICAgICAgICBzdHJpY3RuZXNzOiBudWxsXG4gICAgICAgIHN0b3BCeTogbmVpZ2hib3JcbiAgICAgICAgZmllbGQ6IG51bGxcbiAgICAtIGhhczpcbiAgICAgICAgcGF0dGVybjpcbiAgICAgICAgICBjb250ZXh0OiBjb25zdGV4cHIgaW50IGEgPSAwO1xuICAgICAgICAgIHNlbGVjdG9yOiB0eXBlX3F1YWxpZmllclxuICAgICAgICAgIHN0cmljdG5lc3M6IG51bGxcbiAgICAgICAgc3RvcEJ5OiBuZWlnaGJvclxuICAgICAgICBmaWVsZDogbnVsbFxuICAgIC0gaGFzOlxuICAgICAgICBwYXR0ZXJuOlxuICAgICAgICAgIGNvbnRleHQ6IGNvbnN0aW5pdCBpbnQgYSA9IDA7XG4gICAgICAgICAgc2VsZWN0b3I6IHR5cGVfcXVhbGlmaWVyXG4gICAgICAgICAgc3RyaWN0bmVzczogbnVsbFxuICAgICAgICBzdG9wQnk6IG5laWdoYm9yXG4gICAgICAgIGZpZWxkOiBudWxsXG4gIE1FTUJFUl9WQVJJQUJMRTpcbiAgICBraW5kOiBmaWVsZF9pZGVudGlmaWVyXG4gICAgaW5zaWRlOlxuICAgICAgYW55OlxuICAgICAgLSBraW5kOiBmaWVsZF9kZWNsYXJhdGlvblxuICAgICAgLSBraW5kOiByZWZlcmVuY2VfZGVjbGFyYXRvclxuICAgICAgICBpbnNpZGU6XG4gICAgICAgICAga2luZDogZmllbGRfZGVjbGFyYXRpb25cbiAgICAgICAgICBzdG9wQnk6IG5laWdoYm9yXG4gICAgICAgICAgZmllbGQ6IG51bGxcbiAgICAgIC0ga2luZDogcG9pbnRlcl9kZWNsYXJhdG9yXG4gICAgICAgIGluc2lkZTpcbiAgICAgICAgICBraW5kOiBmaWVsZF9kZWNsYXJhdGlvblxuICAgICAgICAgIHN0b3BCeTogbmVpZ2hib3JcbiAgICAgICAgICBmaWVsZDogbnVsbFxuICAgICAgc3RvcEJ5OiBuZWlnaGJvclxuICAgICAgZmllbGQ6IG51bGxcbnRyYW5zZm9ybTogbnVsbFxuZml4OiBudWxsXG5pZDogRmllbGRNZW1iZXJWYXJpYWJsZUlkZW50aWZpZXJOYW1pbmdDaGVja1xubGFuZ3VhZ2U6IENwcFxucmV3cml0ZXJzOiBudWxsXG5tZXNzYWdlOiBJbnZhbGlkIGlkZW50aWZpZXIgbmFtaW5nLlxubm90ZTogbnVsbFxuc2V2ZXJpdHk6IGVycm9yXG5maWxlczogbnVsbFxuaWdub3Jlczpcbi0gLyoqL3Rlc3QvKiovKlxuLSAvKiovdW5pdHRlc3QvKiovKlxudXJsOiBudWxsXG5tZXRhZGF0YTogbnVsbFxuIiwic291cmNlIjoic3RydWN0IFRlc3Qge1xucHJpdmF0ZTpcbiAgaW50IGV4cGVjdGVkX25hbWVfO1xuICBpbnQgaW52YWxpZF9uYW1lO1xufTsifQ==

### Description:
There is an issue with the serialization and deserialization of SerializableStopBy in ast-grep. The current implementation provides a custom deserialization logic for SerializableStopBy, but the serialization logic relies on Serde’s Serialize derive macro. This asymmetry causes YAML configurations to be transformed into an unrecognized format when serialized back, making them incompatible with ast-grep’s own deserialization.

### Steps to Reproduce:
1. Start with a valid YAML configuration that ast-grep can parse correctly.
2. Deserialize the YAML into a Rust structure.
3. Serialize the structure back into YAML.
4. Attempt to use the newly serialized YAML with ast-grep.
5. Observe that ast-grep fails to recognize the modified YAML.

### Expected Behavior:
- The serialized YAML should be compatible with ast-grep’s deserialization logic.
- Serialization and deserialization should be symmetrical, ensuring that round-tripping YAML configurations do not break compatibility.

### Actual Behavior:
- The YAML configuration changes after serialization.
- ast-grep fails to recognize the new YAML format due to the mismatch between its custom deserialization and the standard Serde serialization.

### Potential Fix:
- Implement a custom serialization logic for SerializableStopBy that mirrors the deserialization logic.
- Ensure that the serialized YAML output remains compatible with ast-grep’s expected input format.

### Environment:
- ast-grep version: 0.34.4
- Rust version: 1.84.1

### Additional Context:
I am working on a wrapper around ast-grep that needs to modify its YAML configuration dynamically. However, due to this issue, my tool is unable to generate valid YAML configurations that ast-grep can understand.

Let me know if more details are needed. Thanks!


Discussion / hints:
Hi thanks for the report.

> Implement a custom serialization logic for SerializableStopBy that mirrors the deserialization logic.


I think this is a better pathway. However, I'm not interested in implementing it myself. Pull requests are welcome.

Fix patch:
diff --git a/crates/config/src/rule/stop_by.rs b/crates/config/src/rule/stop_by.rs
index d9264b8a08..f7dcf42937 100644
--- a/crates/config/src/rule/stop_by.rs
+++ b/crates/config/src/rule/stop_by.rs
@@ -13,7 +13,7 @@ use std::fmt;
 
 // NB StopBy's JsonSchema is changed in xtask/schema.rs
 // revise schema is easier than manually implementation
-#[derive(Serialize, Clone, Default, JsonSchema)]
+#[derive(Clone, Default, JsonSchema)]
 #[serde(rename_all = "camelCase")]
 pub enum SerializableStopBy {
   #[default]
@@ -22,6 +22,13 @@ pub enum SerializableStopBy {
   Rule(SerializableRule),
 }
 
+impl SerializableStopBy {
+  /// String key used for serializing the Neighbor variant
+  const NEIGHBOR_KEY: &str = "neighbor";
+  /// String key used for serializing the End variant
+  const END_KEY: &str = "end";
+}
+
 struct StopByVisitor;
 impl<'de> Visitor<'de> for StopByVisitor {
   type Value = SerializableStopBy;
@@ -34,10 +41,12 @@ impl<'de> Visitor<'de> for StopByVisitor {
     E: de::Error,
   {
     match value {
-      "neighbor" => Ok(SerializableStopBy::Neighbor),
-      "end" => Ok(SerializableStopBy::End),
+      SerializableStopBy::NEIGHBOR_KEY => Ok(SerializableStopBy::Neighbor),
+      SerializableStopBy::END_KEY => Ok(SerializableStopBy::End),
       v => Err(de::Error::custom(format!(
-        "unknown variant `{v}`, expected `neighbor`, `end` or a rule object",
+        "unknown variant `{v}`, expected `{}`, `{}` or a rule object",
+        SerializableStopBy::NEIGHBOR_KEY,
+        SerializableStopBy::END_KEY,
       ))),
     }
   }
@@ -60,6 +69,19 @@ impl<'de> Deserialize<'de> for SerializableStopBy {
   }
 }
 
+impl Serialize for SerializableStopBy {
+  fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+  where
+    S: serde::Serializer,
+  {
+    match self {
+      SerializableStopBy::Neighbor => serializer.serialize_str(SerializableStopBy::NEIGHBOR_KEY),
+      SerializableStopBy::End => serializer.serialize_str(SerializableStopBy::END_KEY),
+      SerializableStopBy::Rule(rule) => rule.serialize(serializer),
+    }
+  }
+}
+
 pub enum StopBy<L: Language> {
   Neighbor,
   End,
@@ -207,4 +229,22 @@ inside:
     let stop_by = parse_stop_by("pattern: $A");
     assert_eq!(stop_by.defined_vars(), ["A"].into_iter().collect());
   }
+
+  #[test]
+  fn test_serialization_deserialization_symmetry() {
+    let stop = to_stop_by("'neighbor'").expect("cannot parse stopBy");
+    let serialized = serde_yaml::to_string(&stop).expect("cannot serialize stopBy");
+    let deserialized = to_stop_by(&serialized).expect("cannot parse stopBy");
+    assert!(matches!(deserialized, SerializableStopBy::Neighbor));
+
+    let stop = to_stop_by("'end'").expect("cannot parse stopBy");
+    let serialized = serde_yaml::to_string(&stop).expect("cannot serialize stopBy");
+    let deserialized = to_stop_by(&serialized).expect("cannot parse stopBy");
+    assert!(matches!(deserialized, SerializableStopBy::End));
+
+    let stop = to_stop_by("kind: some-kind").expect("cannot parse stopBy");
+    let serialized = serde_yaml::to_string(&stop).expect("cannot serialize stopBy");
+    let deserialized = to_stop_by(&serialized).expect("cannot parse stopBy");
+    assert!(matches!(deserialized, SerializableStopBy::Rule(_)));
+  }
 }


Test patch: