fix: servings

Author: dubadub

fuzz/fuzz_targets/fuzz_parser.rs

@@ -2,7 +2,7 @@
 
 use libfuzzer_sys::fuzz_target;
 
-use cooklang::{CooklangParser, Extensions, Converter};
+use cooklang::{Converter, CooklangParser, Extensions};
 
 fuzz_target!(|contents: &str| {
     let parser = CooklangParser::new(Extensions::all(), Converter::default());

src/aisle.rs

@@ -275,7 +275,8 @@ pub fn parse(input: &str) -> Result<AisleConf, AisleConfError> {
 pub fn parse_lenient(input: &str) -> PassResult<AisleConf> {
     let mut report = SourceReport::empty();
 
-    let conf = parse_core(input, true, Some(&mut report)).expect("lenient parsing should never fail");
+    let conf =
+        parse_core(input, true, Some(&mut report)).expect("lenient parsing should never fail");
     PassResult::new(Some(conf), report)
 }
 

src/metadata.rs

@@ -200,12 +200,8 @@ impl Metadata {
     }
 
     /// Servings the recipe is made for
-    ///
-    /// This returns a list of servings to support scaling. See
-    /// [`CooklangValueExt::as_servings`] for the expected format.
-    pub fn servings(&self) -> Option<Vec<u32>> {
-        self.get(StdKey::Servings)
-            .and_then(CooklangValueExt::as_servings)
+    pub fn servings(&self) -> Option<Servings> {
+        self.get(StdKey::Servings).and_then(|v| v.as_servings())
     }
 
     /// Recipe locale
@@ -271,35 +267,6 @@ pub trait CooklangValueExt: private::Sealed {
     /// Duplicates and empty entries removed.
     fn as_tags(&self) -> Option<Vec<Cow<str>>>;
 
-    /// Pipe ('|') separated string or YAML sequence of numbers
-    ///
-    /// This extracts only the number at the beginning, but the entry can have
-    /// extra text, like the unit. For example:
-    /// ```yaml
-    /// servings: 5 cups worth
-    /// ```
-    /// will return `vec![5]`.
-    ///
-    /// These values will be used for scaling the recipe. If you want to display
-    /// the full text alongside the values, you can do something like:
-    ///
-    /// ```no_run
-    /// # use cooklang::metadata::*;
-    /// # fn f() -> Option<()> {
-    /// # let metadata = Metadata::default();
-    /// let nums = metadata.get("servings")?.as_servings()?;
-    /// let texts  = metadata.get("servings")?.as_string_list("|")?;
-    ///
-    /// for (num, text) in nums.iter().zip(texts.iter()) {
-    ///     println!("{num} - '{text}'")
-    /// }
-    /// # Some(())
-    /// # }
-    /// ```
-    ///
-    /// Duplicates not allowed, will return `None`.
-    fn as_servings(&self) -> Option<Vec<u32>>;
-
     /// String separated by `sep` or YAML sequence of strings and/or numbers
     ///
     /// This only checks types and convert numbers to strings if neccesary.
@@ -345,17 +312,18 @@ pub trait CooklangValueExt: private::Sealed {
 
     /// String or number as a string
     fn as_str_like(&self) -> Option<Cow<str>>;
+
+    /// Get servings information
+    ///
+    /// Can be a number or a string that parses to [`Servings`]
+    fn as_servings(&self) -> Option<Servings>;
 }
 
 impl CooklangValueExt for serde_yaml::Value {
     fn as_tags(&self) -> Option<Vec<Cow<str>>> {
         value_as_tags(self).ok()
     }
 
-    fn as_servings(&self) -> Option<Vec<u32>> {
-        value_as_servings(self).ok()
-    }
-
     fn as_string_list<'a>(&'a self, sep: &str) -> Option<Vec<Cow<'a, str>>> {
         if let Some(s) = self.as_str() {
             let v = s.split(sep).map(|e| e.into()).collect();
@@ -415,6 +383,25 @@ impl CooklangValueExt for serde_yaml::Value {
             None
         }
     }
+
+    fn as_servings(&self) -> Option<Servings> {
+        // Try as number first
+        if let Some(n) = self.as_u32() {
+            return Some(Servings::Number(n));
+        }
+
+        // Return as text if it's a string
+        if let Some(s) = self.as_str() {
+            // Try to parse as number
+            if let Ok(n) = s.parse::<u32>() {
+                Some(Servings::Number(n))
+            } else {
+                Some(Servings::Text(s.to_string()))
+            }
+        } else {
+            None
+        }
+    }
 }
 
 fn value_as_tags(val: &serde_yaml::Value) -> Result<Vec<Cow<str>>, MetadataError> {
@@ -441,54 +428,6 @@ fn value_as_tags(val: &serde_yaml::Value) -> Result<Vec<Cow<str>>, MetadataError
     Ok(tags)
 }
 
-fn value_as_servings(val: &serde_yaml::Value) -> Result<Vec<u32>, MetadataError> {
-    fn extract_value(s: &str) -> Result<u32, std::num::ParseIntError> {
-        let idx = s
-            .find(|c: char| !c.is_ascii_alphanumeric())
-            .unwrap_or(s.len());
-        let n_str = &s[..idx];
-        n_str.parse()
-    }
-
-    let servings: Vec<u32> = if let Some(n) = val.as_u32() {
-        vec![n]
-    } else if let Some(s) = val.as_str() {
-        s.split('|')
-            .map(str::trim)
-            .map(extract_value)
-            .collect::<Result<Vec<_>, _>>()?
-    } else if let Some(seq) = val.as_sequence() {
-        let mut v = Vec::with_capacity(seq.len());
-        for e in seq {
-            if let Some(n) = e.as_u32() {
-                v.push(n);
-            } else if let Some(s) = e.as_str() {
-                v.push(extract_value(s)?);
-            } else {
-                return Err(MetadataError::BadSequenceType {
-                    expected: MetaType::Number,
-                    got: MetaType::from(e),
-                });
-            }
-        }
-        v
-    } else {
-        return Err(MetadataError::expect_type(MetaType::Sequence, val));
-    };
-
-    let l = servings.len();
-    let dedup_l = {
-        let mut temp = servings.clone();
-        temp.sort_unstable();
-        temp.dedup();
-        temp.len()
-    };
-    if l != dedup_l {
-        return Err(MetadataError::DuplicateServings { servings });
-    }
-    Ok(servings)
-}
-
 fn value_as_minutes(val: &serde_yaml::Value, converter: &Converter) -> Result<u32, MetadataError> {
     if let Some(s) = val.as_str() {
         let t = parse_time(s, converter)?;
@@ -554,7 +493,9 @@ pub(crate) fn check_std_entry(
 ) -> Result<(), MetadataError> {
     match key {
         StdKey::Servings => {
-            value_as_servings(value)?;
+            value
+                .as_u32()
+                .ok_or(MetadataError::expect_type(MetaType::Number, value))?;
         }
         StdKey::Tags => {
             value_as_tags(value)?;
@@ -589,6 +530,43 @@ pub(crate) fn check_std_entry(
     Ok(())
 }
 
+/// Servings information that can be numeric or a string
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
+#[serde(untagged)]
+pub enum Servings {
+    /// Numeric servings count
+    Number(u32),
+    /// String servings (when it can't be parsed as a number)
+    Text(String),
+}
+
+impl Servings {
+    /// Get the numeric value if available
+    pub fn as_number(&self) -> Option<u32> {
+        match self {
+            Servings::Number(n) => Some(*n),
+            Servings::Text(_) => None,
+        }
+    }
+
+    /// Get as text
+    pub fn as_text(&self) -> Option<&str> {
+        match self {
+            Servings::Number(_) => None,
+            Servings::Text(s) => Some(s),
+        }
+    }
+}
+
+impl std::fmt::Display for Servings {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Servings::Number(n) => write!(f, "{}", n),
+            Servings::Text(s) => write!(f, "{}", s),
+        }
+    }
+}
+
 /// Combination of name and URL.
 ///
 /// At least one of the fields is [`Some`].
@@ -845,8 +823,6 @@ pub(crate) enum MetadataError {
     BadMapping,
     #[error(transparent)]
     ParseIntError(#[from] std::num::ParseIntError),
-    #[error("Duplicate servings: {servings:?}")]
-    DuplicateServings { servings: Vec<u32> },
     #[error(transparent)]
     ParseTimeError(#[from] ParseTimeError),
     #[error("Invalid locale: {0}")]

src/parser/quantity.rs

@@ -87,11 +87,7 @@ fn parse_regular_quantity<'i>(bp: &mut BlockParser<'_, 'i>) -> ParsedQuantity<'i
 }
 
 fn parse_advanced_quantity<'i>(bp: &mut BlockParser<'_, 'i>) -> Option<ParsedQuantity<'i>> {
-    if bp
-        .tokens()
-        .iter()
-        .any(|t| matches!(t.kind, T![%]))
-    {
+    if bp.tokens().iter().any(|t| matches!(t.kind, T![%])) {
         return None;
     }
 
@@ -138,7 +134,10 @@ fn parse_advanced_quantity<'i>(bp: &mut BlockParser<'_, 'i>) -> Option<ParsedQua
     Some(ParsedQuantity {
         quantity: Located::new(
             Quantity {
-                value: QuantityValue {value, scaling_lock},
+                value: QuantityValue {
+                    value,
+                    scaling_lock,
+                },
                 unit: Some(unit),
             },
             tokens_span(bp.tokens()),
@@ -152,7 +151,10 @@ fn value(bp: &mut BlockParser) -> QuantityValue {
     let value_tokens = bp.consume_while(|t| !matches!(t, T![%]));
     let value = parse_value(value_tokens, bp);
 
-    QuantityValue { value, scaling_lock }
+    QuantityValue {
+        value,
+        scaling_lock,
+    }
 }
 
 fn scaling_lock(bp: &mut BlockParser) -> Option<Span> {
@@ -162,8 +164,8 @@ fn scaling_lock(bp: &mut BlockParser) -> Option<Span> {
         T![=] => {
             let tok = bp.bump_any();
             Some(tok.span)
-        },
-        _ => None
+        }
+        _ => None,
     }
 }
 
@@ -329,7 +331,7 @@ fn float(tokens: &[Token], bp: &BlockParser) -> Result<f64, SourceDiag> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{parser::TokenStream};
+    use crate::parser::TokenStream;
     use test_case::test_case;
 
     macro_rules! t {