feat(fluent-bundle): Resolve more pattern types into &str references

JasperDeSutter · alerque · commit a599792a3ad6 · 2024-05-06T15:11:03.000+03:00
diff --git a/fluent-bundle/examples/custom_formatter.rs b/fluent-bundle/examples/custom_formatter.rs
@@ -36,7 +36,7 @@ key-var-with-arg = Here is a variable formatted with an argument { NUMBER($num,
         .expect("Failed to add FTL resources to the bundle.");
     bundle
         .add_function("NUMBER", |positional, named| {
-            match positional.get(0) {
+            match positional.first() {
                 Some(FluentValue::Number(n)) => {
                     let mut num = n.clone();
                     // This allows us to merge the arguments provided
diff --git a/fluent-bundle/examples/custom_type.rs b/fluent-bundle/examples/custom_type.rs
@@ -165,7 +165,7 @@ key-date = Today is { DATETIME($epoch, dateStyle: "long", timeStyle: "short") }
         .expect("Failed to add FTL resources to the bundle.");
 
     bundle
-        .add_function("DATETIME", |positional, named| match positional.get(0) {
+        .add_function("DATETIME", |positional, named| match positional.first() {
             Some(FluentValue::Number(n)) => {
                 let epoch = n.value as usize;
                 let options = named.into();
diff --git a/fluent-bundle/examples/functions.rs b/fluent-bundle/examples/functions.rs
@@ -22,7 +22,7 @@ fn main() {
     // Test for a function that accepts unnamed positional arguments
     bundle
         .add_function("MEANING_OF_LIFE", |args, _named_args| {
-            if let Some(arg0) = args.get(0) {
+            if let Some(arg0) = args.first() {
                 if *arg0 == 42.into() {
                     return "The answer to life, the universe, and everything".into();
                 }
diff --git a/fluent-bundle/examples/simple-app.rs b/fluent-bundle/examples/simple-app.rs
@@ -96,8 +96,7 @@ fn main() {
         Some(&default_locale),
         NegotiationStrategy::Filtering,
     );
-    let current_locale = resolved_locales
-        .get(0)
+    let current_locale = resolved_locales.first()
         .cloned()
         .expect("At least one locale should match.");
 
diff --git a/fluent-bundle/src/bundle.rs b/fluent-bundle/src/bundle.rs
@@ -482,10 +482,10 @@ impl<R, M> FluentBundle<R, M> {
     ///
     /// assert_eq!(result, "Hello World!");
     /// ```
-    pub fn format_pattern<'bundle, 'args>(
+    pub fn format_pattern<'bundle>(
         &'bundle self,
         pattern: &'bundle ast::Pattern<&'bundle str>,
-        args: Option<&'args FluentArgs>,
+        args: Option<&FluentArgs>,
         errors: &mut Vec<FluentError>,
     ) -> Cow<'bundle, str>
     where
@@ -577,7 +577,7 @@ impl<R> FluentBundle<R, IntlLangMemoizer> {
     ///
     /// This will panic if no formatters can be found for the locales.
     pub fn new(locales: Vec<LanguageIdentifier>) -> Self {
-        let first_locale = locales.get(0).cloned().unwrap_or_default();
+        let first_locale = locales.first().cloned().unwrap_or_default();
         Self {
             locales,
             resources: vec![],
diff --git a/fluent-bundle/src/concurrent.rs b/fluent-bundle/src/concurrent.rs
@@ -30,7 +30,7 @@ impl<R> FluentBundle<R> {
     ///     FluentBundle::new_concurrent(vec![langid_en]);
     /// ```
     pub fn new_concurrent(locales: Vec<LanguageIdentifier>) -> Self {
-        let first_locale = locales.get(0).cloned().unwrap_or_default();
+        let first_locale = locales.first().cloned().unwrap_or_default();
         Self {
             locales,
             resources: vec![],
diff --git a/fluent-bundle/src/resolver/expression.rs b/fluent-bundle/src/resolver/expression.rs
@@ -64,3 +64,45 @@ impl<'bundle> WriteValue<'bundle> for ast::Expression<&'bundle str> {
         }
     }
 }
+
+impl<'bundle> ResolveValue<'bundle> for ast::Expression<&'bundle str> {
+    fn resolve<'ast, 'args, 'errors, R, M>(
+        &'ast self,
+        scope: &mut Scope<'bundle, 'ast, 'args, 'errors, R, M>,
+    ) -> FluentValue<'bundle>
+    where
+        R: Borrow<FluentResource>,
+        M: MemoizerKind,
+    {
+        match self {
+            Self::Inline(exp) => exp.resolve(scope),
+            Self::Select { selector, variants } => {
+                let selector = selector.resolve(scope);
+                match selector {
+                    FluentValue::String(_) | FluentValue::Number(_) => {
+                        for variant in variants {
+                            let key = match variant.key {
+                                ast::VariantKey::Identifier { name } => name.into(),
+                                ast::VariantKey::NumberLiteral { value } => {
+                                    FluentValue::try_number(value)
+                                }
+                            };
+                            if key.matches(&selector, scope) {
+                                return variant.value.resolve(scope);
+                            }
+                        }
+                    }
+                    _ => {}
+                }
+
+                for variant in variants {
+                    if variant.default {
+                        return variant.value.resolve(scope);
+                    }
+                }
+                scope.add_error(ResolverError::MissingDefault);
+                FluentValue::Error
+            }
+        }
+    }
+}
diff --git a/fluent-bundle/src/resolver/inline_expression.rs b/fluent-bundle/src/resolver/inline_expression.rs
@@ -183,14 +183,72 @@ impl<'bundle> ResolveValue<'bundle> for ast::InlineExpression<&'bundle str> {
                     let result = func(resolved_positional_args.as_slice(), &resolved_named_args);
                     result
                 } else {
+                    scope.add_error(self.into());
+                    FluentValue::Error
+                }
+            }
+            ast::InlineExpression::MessageReference { id, attribute } => {
+                if let Some(msg) = scope.bundle.get_entry_message(id.name) {
+                    if let Some(attr) = attribute {
+                        msg.attributes
+                            .iter()
+                            .find_map(|a| {
+                                if a.id.name == attr.name {
+                                    Some(scope.track_resolve(&a.value))
+                                } else {
+                                    None
+                                }
+                            })
+                            .unwrap_or_else(|| {
+                                scope.add_error(self.into());
+                                FluentValue::Error
+                            })
+                    } else {
+                        msg.value
+                            .as_ref()
+                            .map(|value| scope.track_resolve(value))
+                            .unwrap_or_else(|| {
+                                scope.add_error(ResolverError::NoValue(id.name.to_string()));
+                                FluentValue::Error
+                            })
+                    }
+                } else {
+                    scope.add_error(self.into());
                     FluentValue::Error
                 }
             }
-            _ => {
-                let mut result = String::new();
-                self.write(&mut result, scope).expect("Failed to write");
-                result.into()
+            ast::InlineExpression::TermReference {
+                id,
+                attribute,
+                arguments,
+            } => {
+                let (_, resolved_named_args) = scope.get_arguments(arguments.as_ref());
+
+                scope.local_args = Some(resolved_named_args);
+                let result = scope
+                    .bundle
+                    .get_entry_term(id.name)
+                    .and_then(|term| {
+                        if let Some(attr) = attribute {
+                            term.attributes.iter().find_map(|a| {
+                                if a.id.name == attr.name {
+                                    Some(scope.track_resolve(&a.value))
+                                } else {
+                                    None
+                                }
+                            })
+                        } else {
+                            Some(scope.track_resolve(&term.value))
+                        }
+                    })
+                    .unwrap_or_else(|| {
+                        scope.add_error(self.into());
+                        FluentValue::Error
+                    });
+                scope.local_args = None;
+                result
             }
+            ast::InlineExpression::Placeable { expression } => expression.resolve(scope),
         }
     }
 }
diff --git a/fluent-bundle/src/resolver/pattern.rs b/fluent-bundle/src/resolver/pattern.rs
@@ -91,13 +91,33 @@ impl<'bundle> ResolveValue<'bundle> for ast::Pattern<&'bundle str> {
     {
         let len = self.elements.len();
 
+        // more than 1 element means concatenation, which is more efficient to write to a String
+        // 1 element often means just a message reference that can be passed back as a Cow::Borrowed
         if len == 1 {
-            if let ast::PatternElement::TextElement { value } = self.elements[0] {
-                return scope
-                    .bundle
-                    .transform
-                    .map_or_else(|| value.into(), |transform| transform(value).into());
-            }
+            match &self.elements[0] {
+                &ast::PatternElement::TextElement { value } => {
+                    return scope
+                        .bundle
+                        .transform
+                        .map_or_else(|| value.into(), |transform| transform(value).into());
+                }
+                ast::PatternElement::Placeable { expression } => {
+                    let before = scope.placeables;
+                    scope.placeables += 1;
+
+                    let res = scope.maybe_track_resolve(self, expression);
+                    if !matches!(res, FluentValue::Error) {
+                        return res;
+                    }
+
+                    // when hitting an error, reset scope state and format using writer below to write error information
+                    scope.placeables = before;
+                    scope.dirty = false;
+                    if let Some(err) = &mut scope.errors {
+                        err.pop();
+                    }
+                }
+            };
         }
 
         let mut result = String::new();
diff --git a/fluent-bundle/src/resolver/scope.rs b/fluent-bundle/src/resolver/scope.rs
@@ -102,6 +102,49 @@ impl<'bundle, 'ast, 'args, 'errors, R, M> Scope<'bundle, 'ast, 'args, 'errors, R
         }
     }
 
+    /// Similar to [`Scope::maybe_track`], but resolves to a value
+    /// instead of writing to a Writer instance.
+    pub fn maybe_track_resolve(
+        &mut self,
+        pattern: &'ast ast::Pattern<&'bundle str>,
+        exp: &'ast ast::Expression<&'bundle str>,
+    ) -> FluentValue<'bundle>
+    where
+        R: Borrow<FluentResource>,
+        M: MemoizerKind,
+    {
+        if self.travelled.is_empty() {
+            self.travelled.push(pattern);
+        }
+        let res = exp.resolve(self);
+        if self.dirty {
+            FluentValue::Error
+        } else {
+            res
+        }
+    }
+
+    /// Similar to [`Scope::track`], but resolves to a value
+    /// instead of writing to a Writer instance.
+    pub fn track_resolve(
+        &mut self,
+        pattern: &'ast ast::Pattern<&'bundle str>,
+    ) -> FluentValue<'bundle>
+    where
+        R: Borrow<FluentResource>,
+        M: MemoizerKind,
+    {
+        if self.travelled.contains(&pattern) {
+            self.add_error(ResolverError::Cyclic);
+            FluentValue::Error
+        } else {
+            self.travelled.push(pattern);
+            let result = pattern.resolve(self);
+            self.travelled.pop();
+            result
+        }
+    }
+
     pub fn write_ref_error<W>(
         &mut self,
         w: &mut W,
diff --git a/fluent-bundle/src/types/mod.rs b/fluent-bundle/src/types/mod.rs
@@ -185,9 +185,9 @@ impl<'source> FluentValue<'source> {
         M: MemoizerKind,
     {
         match (self, other) {
-            (&FluentValue::String(ref a), &FluentValue::String(ref b)) => a == b,
-            (&FluentValue::Number(ref a), &FluentValue::Number(ref b)) => a == b,
-            (&FluentValue::String(ref a), &FluentValue::Number(ref b)) => {
+            (FluentValue::String(a), FluentValue::String(b)) => a == b,
+            (FluentValue::Number(a), FluentValue::Number(b)) => a == b,
+            (FluentValue::String(a), FluentValue::Number(b)) => {
                 let cat = match a.as_ref() {
                     "zero" => PluralCategory::ZERO,
                     "one" => PluralCategory::ONE,
diff --git a/fluent-bundle/tests/custom_types.rs b/fluent-bundle/tests/custom_types.rs
@@ -40,9 +40,9 @@ fn fluent_custom_type() {
 
     let sv = FluentValue::from("foo");
 
-    assert_eq!(dt == dt2, true);
-    assert_eq!(dt == dt3, false);
-    assert_eq!(dt == sv, false);
+    assert!(dt == dt2);
+    assert!(dt != dt3);
+    assert!(dt != sv);
 }
 
 #[test]
@@ -146,7 +146,7 @@ key-ref = Hello { DATETIME($date, dateStyle: "full") } World
     bundle.set_use_isolating(false);
 
     bundle
-        .add_function("DATETIME", |positional, named| match positional.get(0) {
+        .add_function("DATETIME", |positional, named| match positional.first() {
             Some(FluentValue::Custom(custom)) => {
                 if let Some(that) = custom.as_ref().as_any().downcast_ref::<DateTime>() {
                     let mut dt = that.clone();
@@ -202,7 +202,7 @@ key-num-explicit = Hello { NUMBER(5, minimumFractionDigits: 2) } World
     bundle.set_use_isolating(false);
 
     bundle
-        .add_function("NUMBER", |positional, named| match positional.get(0) {
+        .add_function("NUMBER", |positional, named| match positional.first() {
             Some(FluentValue::Number(n)) => {
                 let mut num = n.clone();
                 num.options.merge(named);
diff --git a/fluent-bundle/tests/function.rs b/fluent-bundle/tests/function.rs
@@ -24,7 +24,7 @@ liked-count2 = { NUMBER($num) ->
     let mut bundle = FluentBundle::default();
 
     bundle
-        .add_function("NUMBER", |positional, named| match positional.get(0) {
+        .add_function("NUMBER", |positional, named| match positional.first() {
             Some(FluentValue::Number(n)) => {
                 let mut num = n.clone();
                 num.options.merge(named);
diff --git a/fluent-bundle/tests/resolver_fixtures.rs b/fluent-bundle/tests/resolver_fixtures.rs
@@ -155,10 +155,10 @@ fn create_bundle(
                         .into()
                 }),
                 "IDENTITY" => bundle.add_function(f.as_str(), |args, _name_args| {
-                    args.get(0).cloned().unwrap_or(FluentValue::Error)
+                    args.first().cloned().unwrap_or(FluentValue::Error)
                 }),
                 "NUMBER" => bundle.add_function(f.as_str(), |args, _name_args| {
-                    args.get(0).expect("Argument must be passed").clone()
+                    args.first().expect("Argument must be passed").clone()
                 }),
                 _ => unimplemented!("No such function."),
             };
@@ -242,7 +242,7 @@ fn test_test(test: &Test, defaults: &Option<TestDefaults>, mut scope: Scope) {
                 .get(bundle_name)
                 .expect("Failed to retrieve bundle.")
         } else if bundles.len() == 1 {
-            let name = bundles.keys().into_iter().last().unwrap();
+            let name = bundles.keys().last().unwrap();
             bundles.get(name).expect("Failed to retrieve bundle.")
         } else {
             panic!();
diff --git a/fluent-bundle/tests/types_test.rs b/fluent-bundle/tests/types_test.rs
diff --git a/fluent-fallback/src/localization.rs b/fluent-fallback/src/localization.rs
diff --git a/fluent-fallback/tests/localization_test.rs b/fluent-fallback/tests/localization_test.rs
diff --git a/fluent-syntax/benches/parser.rs b/fluent-syntax/benches/parser.rs
diff --git a/fluent-syntax/src/serializer.rs b/fluent-syntax/src/serializer.rs
diff --git a/fluent-syntax/tests/parser_fixtures.rs b/fluent-syntax/tests/parser_fixtures.rs

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ fn main() {`
`22`	`22`	`// Test for a function that accepts unnamed positional arguments`
`23`	`23`	`bundle`
`24`	`24`	`.add_function("MEANING_OF_LIFE", \|args, _named_args\| {`
`25`		`- if let Some(arg0) = args.get(0) {`
	`25`	`+ if let Some(arg0) = args.first() {`
`26`	`26`	`if *arg0 == 42.into() {`
`27`	`27`	`return "The answer to life, the universe, and everything".into();`
`28`	`28`	`}`