From c54724c20a330984b0aa1e8626af67a969852dbe Mon Sep 17 00:00:00 2001
From: Changyuan Lyu <changyuanl@google.com>
Date: Sun, 28 Jul 2024 09:39:17 -0700
Subject: [PATCH] fix(aco)!: do not deserialize empty string to None

For a struct like

  struct Foo {
    path: PathBuf,
    param: Option<String>,
  }

to set `param` to `None`, it is more intuitive to write `path=foo`
than `path=foo,param=`. The latter looks more like setting `param` to
`Some("")`.

On the other hand, if the value is an id and the id points to an empty
string, it is interpreted as `None`.

Fixes: 8f3ba3445ac4 ("fix(aco): differentiate `Some("")` from `None`")

Signed-off-by: Changyuan Lyu <changyuanl@google.com>
---
 serde-aco/src/de.rs | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/serde-aco/src/de.rs b/serde-aco/src/de.rs
index ea6d96d..583f52d 100644
--- a/serde-aco/src/de.rs
+++ b/serde-aco/src/de.rs
@@ -166,10 +166,10 @@ impl<'s, 'o, 'a> de::Deserializer<'s> for &'a mut Deserializer<'s, 'o> {
         V: Visitor<'s>,
     {
         let id = self.consume_input();
-        if id.is_empty() {
+        let s = self.deref_id(id);
+        if id.starts_with("id_") && s.is_empty() {
             visitor.visit_none()
         } else {
-            let s = self.deref_id(id);
             let mut sub_de = Deserializer { input: s, ..*self };
             visitor.visit_some(&mut sub_de)
         }
@@ -540,13 +540,13 @@ mod test {
 
     #[test]
     fn test_option() {
-        assert_eq!(from_arg::<Option<u32>>("").unwrap(), None);
+        assert_matches!(from_arg::<Option<u32>>(""), Err(Error::ExpectedInteger));
         assert_eq!(from_arg::<Option<u32>>("12").unwrap(), Some(12));
 
-        assert_eq!(from_arg::<Option<&'static str>>("").unwrap(), None);
+        assert_eq!(from_arg::<Option<&'static str>>("").unwrap(), Some(""));
         assert_eq!(
             from_args::<Option<&'static str>>("id_1", &HashMap::from([("id_1", "")])).unwrap(),
-            Some("")
+            None
         );
         assert_eq!(from_arg::<Option<&'static str>>("12").unwrap(), Some("12"));
         assert_eq!(
@@ -554,19 +554,23 @@ mod test {
             Some("id_1")
         );
 
+        let map_none = HashMap::from([("id_none", "")]);
         assert_eq!(from_arg::<Vec<Option<u32>>>("").unwrap(), vec![]);
-        assert_eq!(from_arg::<Vec<Option<u32>>>(",").unwrap(), vec![None]);
+        assert_eq!(
+            from_args::<Vec<Option<u32>>>("id_none,", &map_none).unwrap(),
+            vec![None]
+        );
         assert_eq!(from_arg::<Vec<Option<u32>>>("1,").unwrap(), vec![Some(1)]);
         assert_eq!(
             from_arg::<Vec<Option<u32>>>("1,2,").unwrap(),
             vec![Some(1), Some(2)]
         );
         assert_eq!(
-            from_arg::<Vec<Option<u32>>>("1,2,,").unwrap(),
+            from_args::<Vec<Option<u32>>>("1,2,id_none,", &map_none).unwrap(),
             vec![Some(1), Some(2), None]
         );
         assert_eq!(
-            from_arg::<Vec<Option<u32>>>(",2").unwrap(),
+            from_args::<Vec<Option<u32>>>("id_none,2", &map_none).unwrap(),
             vec![None, Some(2)]
         );
     }