Improve fluent error messages

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<TranslateError<'args>>,
+        fallback: Box<TranslateError<'args>>,
+    },
+}
+
+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<FluentError>,
+    ) -> 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<FluentError> },
+}
+
+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<'_> {}

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;
 mod diagnostic_builder;
 mod diagnostic_impls;
 pub mod emitter;
+pub mod error;
 pub mod json;
 mod lock;
 pub mod registry;

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 @@ pub trait Translate {
             }
             DiagnosticMessage::FluentIdentifier(identifier, attr) => (identifier, attr),
         };
+        let translate_with_bundle =
+            |bundle: &'a FluentBundle| -> Result<Cow<'_, str>, 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<Cow<'_, str>, 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")
     }
 }