Skip to content

Commit

Permalink
Improve error reporting in fluent-fallback
Browse files Browse the repository at this point in the history
  • Loading branch information
zbraniecki committed Apr 10, 2021
1 parent e9b8687 commit 8e17ee6
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 40 deletions.
19 changes: 10 additions & 9 deletions fluent-bundle/src/resolver/pattern.rs
Expand Up @@ -49,15 +49,16 @@ impl<'p> WriteValue for ast::Pattern<&'p str> {

let needs_isolation = scope.bundle.use_isolating
&& len > 1
&& !matches!(expression, ast::Expression::Inline(
ast::InlineExpression::MessageReference { .. },
)
| ast::Expression::Inline(
ast::InlineExpression::TermReference { .. },
)
| ast::Expression::Inline(
ast::InlineExpression::StringLiteral { .. },
));
&& !matches!(
expression,
ast::Expression::Inline(ast::InlineExpression::MessageReference { .. },)
| ast::Expression::Inline(
ast::InlineExpression::TermReference { .. },
)
| ast::Expression::Inline(
ast::InlineExpression::StringLiteral { .. },
)
);
if needs_isolation {
w.write_char('\u{2068}')?;
}
Expand Down
4 changes: 2 additions & 2 deletions fluent-bundle/tests/custom_types.rs
Expand Up @@ -11,7 +11,7 @@ fn fluent_custom_type() {
#[derive(Debug, PartialEq)]
struct DateTime {
epoch: usize,
};
}

impl DateTime {
pub fn new(epoch: usize) -> Self {
Expand Down Expand Up @@ -108,7 +108,7 @@ fn fluent_date_time_builtin() {
struct DateTime {
epoch: usize,
options: DateTimeOptions,
};
}

impl DateTime {
pub fn new(epoch: usize, options: DateTimeOptions) -> Self {
Expand Down
34 changes: 18 additions & 16 deletions fluent-fallback/src/errors.rs
@@ -1,12 +1,17 @@
use fluent_bundle::FluentError;
use std::error::Error;
use unic_langid::LanguageIdentifier;

#[derive(Debug, PartialEq)]
pub enum LocalizationError {
Bundle {
id: Option<String>,
error: FluentError,
},
Resolver {
id: String,
locale: LanguageIdentifier,
errors: Vec<FluentError>,
},
MissingMessage {
id: String,
},
Expand All @@ -16,29 +21,26 @@ pub enum LocalizationError {
SyncRequestInAsyncMode,
}

impl<I: ToString> From<(I, FluentError)> for LocalizationError {
fn from(pieces: (I, FluentError)) -> Self {
Self::Bundle {
id: Some(pieces.0.to_string()),
error: pieces.1,
}
}
}

impl From<FluentError> for LocalizationError {
fn from(error: FluentError) -> Self {
Self::Bundle { id: None, error }
Self::Bundle { error }
}
}

impl std::fmt::Display for LocalizationError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Bundle {
id: Some(id),
error,
} => write!(f, "Bundle {} error: {}", id, error),
Self::Bundle { id: None, error } => write!(f, "Bundle error: {}", error),
Self::Bundle { error } => write!(f, "Bundle error: {}", error),
Self::Resolver { id, locale, errors } => {
let errors: Vec<String> = errors.iter().map(|err| err.to_string()).collect();
write!(
f,
"[resolver] errors in {}/{}: {}",
id,
locale.to_string(),
errors.join(", ")
)
}
Self::MissingMessage { id } => write!(f, "Missing message: {}", id),
Self::MissingValue { id } => write!(f, "Missing value in message: {}", id),
Self::SyncRequestInAsyncMode => {
Expand Down
62 changes: 49 additions & 13 deletions fluent-fallback/src/localization.rs
Expand Up @@ -274,7 +274,6 @@ where
keys: &'l [L10nKey<'l>],
errors: &mut Vec<LocalizationError>,
) -> Vec<Option<L10nMessage<'l>>> {
let mut format_errors = vec![];
let mut result: Vec<Option<L10nMessage>> = Vec::with_capacity(keys.len());

for _ in 0..keys.len() {
Expand All @@ -295,10 +294,19 @@ where
.zip(&mut result)
.filter(|(_, cell)| cell.is_none())
{
let mut format_errors = vec![];
let msg = Self::format_message_from_bundle(bundle, key, &mut format_errors);

if msg.is_none() {
has_missing = true;
} else {
if !format_errors.is_empty() {
errors.push(LocalizationError::Resolver {
id: key.id.to_string(),
locale: bundle.locales.get(0).cloned().unwrap(),
errors: format_errors,
});
}
}

*cell = msg;
Expand All @@ -308,7 +316,6 @@ where
break;
}
}
errors.extend(format_errors.into_iter().map(Into::into));

if !is_complete {
for (key, _) in keys
Expand All @@ -330,7 +337,6 @@ where
keys: &'l [L10nKey<'l>],
errors: &mut Vec<LocalizationError>,
) -> Vec<Option<Cow<'l, str>>> {
let mut format_errors = vec![];
let mut result: Vec<Option<Cow<'l, str>>> = Vec::with_capacity(keys.len());

for _ in 0..keys.len() {
Expand All @@ -354,11 +360,19 @@ where
{
if let Some(msg) = bundle.get_message(&key.id) {
if let Some(value) = msg.value() {
let mut format_errors = vec![];
*cell = Some(bundle.format_pattern(
value,
key.args.as_ref(),
&mut format_errors,
));
if !format_errors.is_empty() {
errors.push(LocalizationError::Resolver {
id: key.id.to_string(),
locale: bundle.locales.get(0).cloned().unwrap(),
errors: format_errors,
});
}
} else {
errors.push(LocalizationError::MissingValue {
id: key.id.to_string(),
Expand All @@ -374,8 +388,6 @@ where
}
}

errors.extend(format_errors.into_iter().map(Into::into));

if !is_complete {
for (key, _) in keys
.iter()
Expand Down Expand Up @@ -407,7 +419,13 @@ where
if let Some(value) = msg.value() {
let mut format_errors = vec![];
let result = bundle.format_pattern(value, args, &mut format_errors);
errors.extend(format_errors.into_iter().map(Into::into));
if !format_errors.is_empty() {
errors.push(LocalizationError::Resolver {
id: id.to_string(),
locale: bundle.locales.get(0).cloned().unwrap(),
errors: format_errors,
});
}
return Some(result);
} else {
errors.push(LocalizationError::MissingValue { id: id.to_string() });
Expand All @@ -430,8 +448,6 @@ where
keys: &'l [L10nKey<'l>],
errors: &mut Vec<LocalizationError>,
) -> Vec<Option<L10nMessage<'l>>> {
let mut format_errors = vec![];

let mut result: Vec<Option<L10nMessage>> = Vec::with_capacity(keys.len());

for _ in 0..keys.len() {
Expand All @@ -454,10 +470,20 @@ where
.zip(&mut result)
.filter(|(_, cell)| cell.is_none())
{
let mut format_errors = vec![];

let msg = Self::format_message_from_bundle(bundle, key, &mut format_errors);

if msg.is_none() {
has_missing = true;
} else {
if !format_errors.is_empty() {
errors.push(LocalizationError::Resolver {
id: key.id.to_string(),
locale: bundle.locales.get(0).cloned().unwrap(),
errors: format_errors,
});
}
}

*cell = msg;
Expand All @@ -467,7 +493,6 @@ where
break;
}
}
errors.extend(format_errors.into_iter().map(Into::into));

if !is_complete {
for (key, _) in keys
Expand All @@ -489,8 +514,6 @@ where
keys: &'l [L10nKey<'l>],
errors: &mut Vec<LocalizationError>,
) -> Vec<Option<Cow<'l, str>>> {
let mut format_errors = vec![];

let mut result: Vec<Option<Cow<'l, str>>> = Vec::with_capacity(keys.len());

for _ in 0..keys.len() {
Expand All @@ -515,11 +538,19 @@ where
{
if let Some(msg) = bundle.get_message(&key.id) {
if let Some(value) = msg.value() {
let mut format_errors = vec![];
*cell = Some(bundle.format_pattern(
value,
key.args.as_ref(),
&mut format_errors,
));
if !format_errors.is_empty() {
errors.push(LocalizationError::Resolver {
id: key.id.to_string(),
locale: bundle.locales.get(0).cloned().unwrap(),
errors: format_errors,
});
}
} else {
errors.push(LocalizationError::MissingValue {
id: key.id.to_string(),
Expand All @@ -535,7 +566,6 @@ where
break;
}
}
errors.extend(format_errors.into_iter().map(Into::into));

if !is_complete {
for (key, _) in keys
Expand Down Expand Up @@ -570,7 +600,13 @@ where
if let Some(value) = msg.value() {
let mut format_errors = vec![];
let result = bundle.format_pattern(value, args, &mut format_errors);
errors.extend(format_errors.into_iter().map(Into::into));
if !format_errors.is_empty() {
errors.push(LocalizationError::Resolver {
id: id.to_string(),
locale: bundle.locales.get(0).cloned().unwrap(),
errors: format_errors,
});
}
return Some(result);
} else {
errors.push(LocalizationError::MissingValue { id: id.to_string() });
Expand Down

0 comments on commit 8e17ee6

Please sign in to comment.