Fix FluentBundle::format_pattern lifetimes (#264)

* FluentValue::into_owned

return a longer living FluentValue, necessary for returning FluentValue from FluentArgs that will outlives FluentArgs

* fix lifetimes

* add tests

* cleaning

Author: MathieuTricoire

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>(
+    pub fn format_pattern<'bundle, 'args>(
         &'bundle self,
-        pattern: &'bundle ast::Pattern<&str>,
-        args: Option<&'bundle FluentArgs>,
+        pattern: &'bundle ast::Pattern<&'bundle str>,
+        args: Option<&'args FluentArgs>,
         errors: &mut Vec<FluentError>,
     ) -> Cow<'bundle, str>
     where

fluent-bundle/src/resolver/expression.rs

@@ -11,11 +11,11 @@ use crate::resolver::{ResolveValue, ResolverError};
 use crate::resource::FluentResource;
 use crate::types::FluentValue;
 
-impl<'p> WriteValue for ast::Expression<&'p str> {
-    fn write<'scope, 'errors, W, R, M>(
-        &'scope self,
+impl<'bundle> WriteValue<'bundle> for ast::Expression<&'bundle str> {
+    fn write<'ast, 'args, 'errors, W, R, M>(
+        &'ast self,
         w: &mut W,
-        scope: &mut Scope<'scope, 'errors, R, M>,
+        scope: &mut Scope<'bundle, 'ast, 'args, 'errors, R, M>,
     ) -> fmt::Result
     where
         W: fmt::Write,

fluent-bundle/src/resolver/inline_expression.rs

@@ -12,11 +12,11 @@ use crate::memoizer::MemoizerKind;
 use crate::resource::FluentResource;
 use crate::types::FluentValue;
 
-impl<'p> WriteValue for ast::InlineExpression<&'p str> {
-    fn write<'scope, 'errors, W, R, M>(
-        &'scope self,
+impl<'bundle> WriteValue<'bundle> for ast::InlineExpression<&'bundle str> {
+    fn write<'ast, 'args, 'errors, W, R, M>(
+        &'ast self,
         w: &mut W,
-        scope: &mut Scope<'scope, 'errors, R, M>,
+        scope: &mut Scope<'bundle, 'ast, 'args, 'errors, R, M>,
     ) -> fmt::Result
     where
         W: fmt::Write,
@@ -147,11 +147,11 @@ impl<'p> WriteValue for ast::InlineExpression<&'p str> {
     }
 }
 
-impl<'p> ResolveValue for ast::InlineExpression<&'p str> {
-    fn resolve<'source, 'errors, R, M>(
-        &'source self,
-        scope: &mut Scope<'source, 'errors, R, M>,
-    ) -> FluentValue<'source>
+impl<'bundle> ResolveValue<'bundle> for ast::InlineExpression<&'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,
@@ -160,16 +160,18 @@ impl<'p> ResolveValue for ast::InlineExpression<&'p str> {
             Self::StringLiteral { value } => unescape_unicode_to_string(value).into(),
             Self::NumberLiteral { value } => FluentValue::try_number(*value),
             Self::VariableReference { id } => {
-                let args = scope.local_args.as_ref().or(scope.args);
-
-                if let Some(arg) = args.and_then(|args| args.get(id.name)) {
-                    arg.clone()
-                } else {
-                    if scope.local_args.is_none() {
-                        scope.add_error(self.into());
+                if let Some(local_args) = &scope.local_args {
+                    if let Some(arg) = local_args.get(id.name) {
+                        return arg.clone();
                     }
-                    FluentValue::Error
+                } else if let Some(arg) = scope.args.and_then(|args| args.get(id.name)) {
+                    return arg.into_owned();
+                }
+
+                if scope.local_args.is_none() {
+                    scope.add_error(self.into());
                 }
+                FluentValue::Error
             }
             Self::FunctionReference { id, arguments } => {
                 let (resolved_positional_args, resolved_named_args) =

fluent-bundle/src/resolver/mod.rs

@@ -15,21 +15,21 @@ use crate::resource::FluentResource;
 use crate::types::FluentValue;
 
 // Converts an AST node to a `FluentValue`.
-pub(crate) trait ResolveValue {
-    fn resolve<'source, 'errors, R, M>(
-        &'source self,
-        scope: &mut Scope<'source, 'errors, R, M>,
-    ) -> FluentValue<'source>
+pub(crate) trait ResolveValue<'bundle> {
+    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;
 }
 
-pub(crate) trait WriteValue {
-    fn write<'source, 'errors, W, R, M>(
-        &'source self,
+pub(crate) trait WriteValue<'bundle> {
+    fn write<'ast, 'args, 'errors, W, R, M>(
+        &'ast self,
         w: &mut W,
-        scope: &mut Scope<'source, 'errors, R, M>,
+        scope: &mut Scope<'bundle, 'ast, 'args, 'errors, R, M>,
     ) -> fmt::Result
     where
         W: fmt::Write,

fluent-bundle/src/resolver/pattern.rs

@@ -13,11 +13,11 @@ use crate::types::FluentValue;
 
 const MAX_PLACEABLES: u8 = 100;
 
-impl<'p> WriteValue for ast::Pattern<&'p str> {
-    fn write<'scope, 'errors, W, R, M>(
-        &'scope self,
+impl<'bundle> WriteValue<'bundle> for ast::Pattern<&'bundle str> {
+    fn write<'ast, 'args, 'errors, W, R, M>(
+        &'ast self,
         w: &mut W,
-        scope: &mut Scope<'scope, 'errors, R, M>,
+        scope: &mut Scope<'bundle, 'ast, 'args, 'errors, R, M>,
     ) -> fmt::Result
     where
         W: fmt::Write,
@@ -80,11 +80,11 @@ impl<'p> WriteValue for ast::Pattern<&'p str> {
     }
 }
 
-impl<'p> ResolveValue for ast::Pattern<&'p str> {
-    fn resolve<'source, 'errors, R, M>(
-        &'source self,
-        scope: &mut Scope<'source, 'errors, R, M>,
-    ) -> FluentValue<'source>
+impl<'bundle> ResolveValue<'bundle> for ast::Pattern<&'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,

fluent-bundle/src/resolver/scope.rs

@@ -8,28 +8,28 @@ use std::borrow::Borrow;
 use std::fmt;
 
 /// State for a single `ResolveValue::to_value` call.
-pub struct Scope<'scope, 'errors, R, M> {
+pub struct Scope<'bundle, 'ast, 'args, 'errors, R, M> {
     /// The current `FluentBundle` instance.
-    pub bundle: &'scope FluentBundle<R, M>,
+    pub bundle: &'bundle FluentBundle<R, M>,
     /// The current arguments passed by the developer.
-    pub(super) args: Option<&'scope FluentArgs<'scope>>,
+    pub(super) args: Option<&'args FluentArgs<'args>>,
     /// Local args
-    pub(super) local_args: Option<FluentArgs<'scope>>,
+    pub(super) local_args: Option<FluentArgs<'bundle>>,
     /// The running count of resolved placeables. Used to detect the Billion
     /// Laughs and Quadratic Blowup attacks.
     pub(super) placeables: u8,
     /// Tracks hashes to prevent infinite recursion.
-    travelled: smallvec::SmallVec<[&'scope ast::Pattern<&'scope str>; 2]>,
+    travelled: smallvec::SmallVec<[&'ast ast::Pattern<&'bundle str>; 2]>,
     /// Track errors accumulated during resolving.
     pub errors: Option<&'errors mut Vec<FluentError>>,
     /// Makes the resolver bail.
     pub dirty: bool,
 }
 
-impl<'scope, 'errors, R, M> Scope<'scope, 'errors, R, M> {
+impl<'bundle, 'ast, 'args, 'errors, R, M> Scope<'bundle, 'ast, 'args, 'errors, R, M> {
     pub fn new(
-        bundle: &'scope FluentBundle<R, M>,
-        args: Option<&'scope FluentArgs>,
+        bundle: &'bundle FluentBundle<R, M>,
+        args: Option<&'args FluentArgs>,
         errors: Option<&'errors mut Vec<FluentError>>,
     ) -> Self {
         Scope {
@@ -58,8 +58,8 @@ impl<'scope, 'errors, R, M> Scope<'scope, 'errors, R, M> {
     pub fn maybe_track<W>(
         &mut self,
         w: &mut W,
-        pattern: &'scope ast::Pattern<&str>,
-        exp: &'scope ast::Expression<&str>,
+        pattern: &'ast ast::Pattern<&'bundle str>,
+        exp: &'ast ast::Expression<&'bundle str>,
     ) -> fmt::Result
     where
         R: Borrow<FluentResource>,
@@ -82,8 +82,8 @@ impl<'scope, 'errors, R, M> Scope<'scope, 'errors, R, M> {
     pub fn track<W>(
         &mut self,
         w: &mut W,
-        pattern: &'scope ast::Pattern<&str>,
-        exp: &ast::InlineExpression<&str>,
+        pattern: &'ast ast::Pattern<&'bundle str>,
+        exp: &'ast ast::InlineExpression<&'bundle str>,
     ) -> fmt::Result
     where
         R: Borrow<FluentResource>,
@@ -119,8 +119,8 @@ impl<'scope, 'errors, R, M> Scope<'scope, 'errors, R, M> {
 
     pub fn get_arguments(
         &mut self,
-        arguments: Option<&'scope ast::CallArguments<&'scope str>>,
-    ) -> (Vec<FluentValue<'scope>>, FluentArgs<'scope>)
+        arguments: Option<&'ast ast::CallArguments<&'bundle str>>,
+    ) -> (Vec<FluentValue<'bundle>>, FluentArgs<'bundle>)
     where
         R: Borrow<FluentResource>,
         M: MemoizerKind,

fluent-bundle/src/types/mod.rs

@@ -181,6 +181,16 @@ impl<'source> FluentValue<'source> {
             FluentValue::None => "".into(),
         }
     }
+
+    pub fn into_owned<'a>(&self) -> FluentValue<'a> {
+        match self {
+            FluentValue::String(str) => FluentValue::String(Cow::from(str.to_string())),
+            FluentValue::Number(s) => FluentValue::Number(s.clone()),
+            FluentValue::Custom(s) => FluentValue::Custom(s.duplicate()),
+            FluentValue::Error => FluentValue::Error,
+            FluentValue::None => FluentValue::None,
+        }
+    }
 }
 
 impl<'source> From<String> for FluentValue<'source> {

fluent-bundle/tests/bundle.rs

@@ -1,14 +1,13 @@
-use fluent_bundle::{FluentBundle, FluentResource};
-use unic_langid::LanguageIdentifier;
+use fluent_bundle::{FluentArgs, FluentBundle, FluentResource};
+use std::borrow::Cow;
+use unic_langid::langid;
 
 #[test]
 fn add_resource_override() {
     let res = FluentResource::try_new("key = Value".to_string()).unwrap();
     let res2 = FluentResource::try_new("key = Value 2".to_string()).unwrap();
 
-    let en_us: LanguageIdentifier = "en-US"
-        .parse()
-        .expect("Failed to parse a language identifier");
+    let en_us = langid!("en-US");
     let mut bundle = FluentBundle::new(vec![en_us]);
 
     bundle.add_resource(&res).expect("Failed to add a resource");
@@ -19,19 +18,75 @@ fn add_resource_override() {
 
     let value = bundle
         .get_message("key")
-        .expect("Failed to retireve a message")
+        .expect("Failed to retrieve a message")
         .value()
-        .expect("Failed to retireve a value of a message");
+        .expect("Failed to retrieve a value of a message");
     assert_eq!(bundle.format_pattern(value, None, &mut errors), "Value");
 
     bundle.add_resource_overriding(&res2);
 
     let value = bundle
         .get_message("key")
-        .expect("Failed to retireve a message")
+        .expect("Failed to retrieve a message")
         .value()
-        .expect("Failed to retireve a value of a message");
+        .expect("Failed to retrieve a value of a message");
     assert_eq!(bundle.format_pattern(value, None, &mut errors), "Value 2");
 
     assert!(errors.is_empty());
 }
+
+#[test]
+fn borrowed_plain_message() {
+    let res = FluentResource::try_new("key = Value".to_string()).unwrap();
+    let en_us = langid!("en-US");
+
+    let mut bundle = FluentBundle::new(vec![en_us]);
+    bundle.add_resource(&res).expect("Failed to add a resource");
+
+    let mut errors = vec![];
+
+    let formatted_pattern = {
+        let value = bundle
+            .get_message("key")
+            .expect("Failed to retrieve a message")
+            .value()
+            .expect("Failed to retrieve a value of a message");
+
+        bundle.format_pattern(value, None, &mut errors)
+    };
+
+    fn is_borrowed(cow: Cow<'_, str>) -> bool {
+        match cow {
+            Cow::Borrowed(_) => true,
+            _ => false,
+        }
+    }
+    assert_eq!(formatted_pattern, "Value");
+    assert!(is_borrowed(formatted_pattern));
+}
+
+#[test]
+fn arguments_outlive_formatted_pattern() {
+    let res = FluentResource::try_new("key = { $variable }".to_string()).unwrap();
+    let en_us = langid!("en-US");
+
+    let mut bundle = FluentBundle::new(vec![en_us]);
+    bundle.add_resource(&res).expect("Failed to add a resource");
+
+    let mut errors = vec![];
+
+    let formatted_pattern = {
+        let value = bundle
+            .get_message("key")
+            .expect("Failed to retrieve a message")
+            .value()
+            .expect("Failed to retrieve a value of a message");
+
+        let mut args = FluentArgs::new();
+        args.set("variable", "Variable");
+
+        bundle.format_pattern(value, Some(&args), &mut errors)
+    };
+
+    assert_eq!(formatted_pattern, "Variable");
+}