Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FluentBundle::format_pattern lifetimes #264

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions fluent-bundle/src/bundle.rs
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions fluent-bundle/src/resolver/expression.rs
Expand Up @@ -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,
Expand Down
36 changes: 19 additions & 17 deletions fluent-bundle/src/resolver/inline_expression.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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>
gregtatum marked this conversation as resolved.
Show resolved Hide resolved
where
R: Borrow<FluentResource>,
M: MemoizerKind,
Expand All @@ -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);
gregtatum marked this conversation as resolved.
Show resolved Hide resolved

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
}
_ => {
let mut result = String::new();
Expand Down
18 changes: 9 additions & 9 deletions fluent-bundle/src/resolver/mod.rs
Expand Up @@ -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,
Expand Down
18 changes: 9 additions & 9 deletions fluent-bundle/src/resolver/pattern.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
28 changes: 14 additions & 14 deletions fluent-bundle/src/resolver/scope.rs
Expand Up @@ -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> {
gregtatum marked this conversation as resolved.
Show resolved Hide resolved
/// 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 {
Expand Down Expand Up @@ -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>,
Expand All @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions fluent-bundle/src/types/mod.rs
Expand Up @@ -181,6 +181,19 @@ 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) => {
let new_value: Box<dyn FluentType + Send> = s.duplicate();
gregtatum marked this conversation as resolved.
Show resolved Hide resolved
FluentValue::Custom(new_value)
}
FluentValue::Error => FluentValue::Error,
FluentValue::None => FluentValue::None,
}
}
}

impl<'source> From<String> for FluentValue<'source> {
Expand Down
73 changes: 64 additions & 9 deletions 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");
Expand All @@ -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");
gregtatum marked this conversation as resolved.
Show resolved Hide resolved
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 = {
gregtatum marked this conversation as resolved.
Show resolved Hide resolved
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");
}