From: mejrs <> Date: Thu, 5 Jan 2023 02:39:07 +0000 (+0100) Subject: Improve fluent error messages X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=0b5d6ae5dbbbe6b05a85bdcccc8aedbb96feedf4;p=rust.git Improve fluent error messages --- diff --git a/compiler/rustc_errors/src/error.rs b/compiler/rustc_errors/src/error.rs new file mode 100644 index 00000000000..e1c8dcd3bd3 --- /dev/null +++ b/compiler/rustc_errors/src/error.rs @@ -0,0 +1,134 @@ +use rustc_error_messages::{ + fluent_bundle::resolver::errors::{ReferenceKind, ResolverError}, + FluentArgs, FluentError, +}; +use std::borrow::Cow; +use std::error::Error; +use std::fmt; + +#[derive(Debug)] +pub enum TranslateError<'args> { + One { + id: &'args Cow<'args, str>, + args: &'args FluentArgs<'args>, + kind: TranslateErrorKind<'args>, + }, + Two { + primary: Box>, + fallback: Box>, + }, +} + +impl<'args> TranslateError<'args> { + pub fn message(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { + Self::One { id, args, kind: TranslateErrorKind::MessageMissing } + } + pub fn primary(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { + Self::One { id, args, kind: TranslateErrorKind::PrimaryBundleMissing } + } + pub fn attribute( + id: &'args Cow<'args, str>, + args: &'args FluentArgs<'args>, + attr: &'args str, + ) -> Self { + Self::One { id, args, kind: TranslateErrorKind::AttributeMissing { attr } } + } + pub fn value(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { + Self::One { id, args, kind: TranslateErrorKind::ValueMissing } + } + + pub fn fluent( + id: &'args Cow<'args, str>, + args: &'args FluentArgs<'args>, + errs: Vec, + ) -> Self { + Self::One { id, args, kind: TranslateErrorKind::Fluent { errs } } + } + + pub fn and(self, fallback: TranslateError<'args>) -> TranslateError<'args> { + Self::Two { primary: Box::new(self), fallback: Box::new(fallback) } + } +} + +#[derive(Debug)] +pub enum TranslateErrorKind<'args> { + MessageMissing, + PrimaryBundleMissing, + AttributeMissing { attr: &'args str }, + ValueMissing, + Fluent { errs: Vec }, +} + +impl fmt::Display for TranslateError<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use TranslateErrorKind::*; + + match self { + Self::One { id, args, kind } => { + writeln!(f, "\nfailed while formatting fluent string `{id}`: ")?; + match kind { + MessageMissing => writeln!(f, "message was missing")?, + PrimaryBundleMissing => writeln!(f, "the primary bundle was missing")?, + AttributeMissing { attr } => writeln!(f, "the attribute `{attr}` was missing")?, + ValueMissing => writeln!(f, "the value was missing")?, + Fluent { errs } => { + for err in errs { + match err { + FluentError::ResolverError(ResolverError::Reference( + ReferenceKind::Message { id, .. } + | ReferenceKind::Variable { id, .. }, + )) => { + if args.iter().any(|(arg_id, _)| arg_id == id) { + writeln!( + f, + "argument `{id}` exists but was not referenced correctly" + )?; + writeln!(f, "help: try using `{{${id}}}` instead")?; + } else { + writeln!( + f, + "the fluent string has an argument `{id}` that was not found." + )?; + let vars: Vec<&str> = + args.iter().map(|(a, _v)| a).collect(); + match &*vars { + [] => writeln!(f, "help: no arguments are available")?, + [one] => writeln!( + f, + "help: the argument `{one}` is available" + )?, + [first, middle @ .., last] => { + write!(f, "help: the arguments `{first}`")?; + for a in middle { + write!(f, ", `{a}`")?; + } + writeln!(f, " and `{last}` are available")?; + } + } + } + } + _ => writeln!(f, "{err}")?, + } + } + } + } + } + // If someone cares about primary bundles, they'll probably notice it's missing + // regardless or will be using `debug_assertions` + // so we skip the arm below this one to avoid confusing the regular user. + Self::Two { primary: box Self::One { kind: PrimaryBundleMissing, .. }, fallback } => { + fmt::Display::fmt(fallback, f)?; + } + Self::Two { primary, fallback } => { + writeln!( + f, + "first, fluent formatting using the primary bundle failed:\n {primary}\n \ + while attempting to recover by using the fallback bundle instead, another error occurred:\n{fallback}" + )?; + } + } + Ok(()) + } +} + +impl Error for TranslateError<'_> {} diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 136c360201e..6f411c5174a 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -11,6 +11,10 @@ #![feature(never_type)] #![feature(result_option_inspect)] #![feature(rustc_attrs)] +#![feature(yeet_expr)] +#![feature(try_blocks)] +#![feature(box_patterns)] +#![feature(error_reporter)] #![allow(incomplete_features)] #[macro_use] @@ -55,6 +59,7 @@ mod diagnostic_builder; mod diagnostic_impls; pub mod emitter; +pub mod error; pub mod json; mod lock; pub mod registry; diff --git a/compiler/rustc_errors/src/translation.rs b/compiler/rustc_errors/src/translation.rs index afd660ff1bf..26c86cdaf45 100644 --- a/compiler/rustc_errors/src/translation.rs +++ b/compiler/rustc_errors/src/translation.rs @@ -1,11 +1,10 @@ +use crate::error::TranslateError; use crate::snippet::Style; use crate::{DiagnosticArg, DiagnosticMessage, FluentBundle}; use rustc_data_structures::sync::Lrc; -use rustc_error_messages::{ - fluent_bundle::resolver::errors::{ReferenceKind, ResolverError}, - FluentArgs, FluentError, -}; +use rustc_error_messages::FluentArgs; use std::borrow::Cow; +use std::error::Report; /// Convert diagnostic arguments (a rustc internal type that exists to implement /// `Encodable`/`Decodable`) into `FluentArgs` which is necessary to perform translation. @@ -63,75 +62,50 @@ fn translate_message<'a>( } DiagnosticMessage::FluentIdentifier(identifier, attr) => (identifier, attr), }; + let translate_with_bundle = + |bundle: &'a FluentBundle| -> Result, TranslateError<'_>> { + let message = bundle + .get_message(identifier) + .ok_or(TranslateError::message(identifier, args))?; + let value = match attr { + Some(attr) => message + .get_attribute(attr) + .ok_or(TranslateError::attribute(identifier, args, attr))? + .value(), + None => message.value().ok_or(TranslateError::value(identifier, args))?, + }; + debug!(?message, ?value); - let translate_with_bundle = |bundle: &'a FluentBundle| -> Option<(Cow<'_, str>, Vec<_>)> { - let message = bundle.get_message(identifier)?; - let value = match attr { - Some(attr) => message.get_attribute(attr)?.value(), - None => message.value()?, + let mut errs = vec![]; + let translated = bundle.format_pattern(value, Some(args), &mut errs); + debug!(?translated, ?errs); + if errs.is_empty() { + Ok(translated) + } else { + Err(TranslateError::fluent(identifier, args, errs)) + } }; - debug!(?message, ?value); - - let mut errs = vec![]; - let translated = bundle.format_pattern(value, Some(args), &mut errs); - debug!(?translated, ?errs); - Some((translated, errs)) - }; - self.fluent_bundle() - .and_then(|bundle| translate_with_bundle(bundle)) - // If `translate_with_bundle` returns `None` with the primary bundle, this is likely - // just that the primary bundle doesn't contain the message being translated, so - // proceed to the fallback bundle. - // - // However, when errors are produced from translation, then that means the translation - // is broken (e.g. `{$foo}` exists in a translation but `foo` isn't provided). - // - // In debug builds, assert so that compiler devs can spot the broken translation and - // fix it.. - .inspect(|(_, errs)| { - debug_assert!( - errs.is_empty(), - "identifier: {:?}, attr: {:?}, args: {:?}, errors: {:?}", - identifier, - attr, - args, - errs - ); - }) - // ..otherwise, for end users, an error about this wouldn't be useful or actionable, so - // just hide it and try with the fallback bundle. - .filter(|(_, errs)| errs.is_empty()) - .or_else(|| translate_with_bundle(self.fallback_fluent_bundle())) - .map(|(translated, errs)| { - // Always bail out for errors with the fallback bundle. + let ret: Result, TranslateError<'_>> = try { + match self.fluent_bundle().map(|b| translate_with_bundle(b)) { + // The primary bundle was present and translation succeeded + Some(Ok(t)) => t, - let mut help_messages = vec![]; + // Always yeet out for errors on debug + Some(Err(primary)) if cfg!(debug_assertions) => do yeet primary, - if !errs.is_empty() { - for error in &errs { - match error { - FluentError::ResolverError(ResolverError::Reference( - ReferenceKind::Message { id, .. }, - )) if args.iter().any(|(arg_id, _)| arg_id == id) => { - help_messages.push(format!("Argument `{id}` exists but was not referenced correctly. Try using `{{${id}}}` instead")); - } - _ => {} - } - } + // If `translate_with_bundle` returns `Err` with the primary bundle, this is likely + // just that the primary bundle doesn't contain the message being translated or + // something else went wrong) so proceed to the fallback bundle. + Some(Err(primary)) => translate_with_bundle(self.fallback_fluent_bundle()) + .map_err(|fallback| primary.and(fallback))?, - panic!( - "Encountered errors while formatting message for `{identifier}`\n\ - help: {}\n\ - attr: `{attr:?}`\n\ - args: `{args:?}`\n\ - errors: `{errs:?}`", - help_messages.join("\nhelp: ") - ); - } - - translated - }) + // The primary bundle is missing, proceed to the fallback bundle + None => translate_with_bundle(self.fallback_fluent_bundle()) + .map_err(|fallback| TranslateError::primary(identifier, args).and(fallback))?, + } + }; + ret.map_err(Report::new) .expect("failed to find message in primary or fallback fluent bundles") } }