Rollup merge of rust-lang#100366 - davidtwco:translation-never-fail, r=compiler-errors

errors: don't fail on broken primary translations

If a primary bundle doesn't contain a message then the fallback bundle is used. However, if the primary bundle's message is broken (e.g. it refers to a interpolated variable that the compiler isn't providing) then this would just result in a compiler panic. While there aren't any primary bundles right now, this is the type of issue that could come up once translation is further along.

r? ```@compiler-errors``` (since this comes out of a in-person discussion we had at RustConf)

Author: Dylan-DPC

compiler/rustc_errors/src/emitter.rs

@@ -273,40 +273,58 @@ pub trait Emitter {
             DiagnosticMessage::FluentIdentifier(identifier, attr) => (identifier, attr),
         };
 
-        let bundle = match self.fluent_bundle() {
-            Some(bundle) if bundle.has_message(&identifier) => bundle,
-            _ => self.fallback_fluent_bundle(),
-        };
+        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()?,
+            };
+            debug!(?message, ?value);
 
-        let message = bundle.get_message(&identifier).expect("missing diagnostic in fluent bundle");
-        let value = match attr {
-            Some(attr) => {
-                if let Some(attr) = message.get_attribute(attr) {
-                    attr.value()
-                } else {
-                    panic!("missing attribute `{attr}` in fluent message `{identifier}`")
-                }
-            }
-            None => {
-                if let Some(value) = message.value() {
-                    value
-                } else {
-                    panic!("missing value in fluent message `{identifier}`")
-                }
-            }
+            let mut errs = vec![];
+            let translated = bundle.format_pattern(value, Some(&args), &mut errs);
+            debug!(?translated, ?errs);
+            Some((translated, errs))
         };
 
-        let mut err = vec![];
-        let translated = bundle.format_pattern(value, Some(&args), &mut err);
-        trace!(?translated, ?err);
-        debug_assert!(
-            err.is_empty(),
-            "identifier: {:?}, args: {:?}, errors: {:?}",
-            identifier,
-            args,
-            err
-        );
-        translated
+        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.
+                assert!(
+                    errs.is_empty(),
+                    "identifier: {:?}, attr: {:?}, args: {:?}, errors: {:?}",
+                    identifier,
+                    attr,
+                    args,
+                    errs
+                );
+                translated
+            })
+            .expect("failed to find message in primary or fallback fluent bundles")
     }
 
     /// Formats the substitutions of the primary_span

compiler/rustc_errors/src/lib.rs

@@ -6,9 +6,10 @@
 #![feature(drain_filter)]
 #![feature(if_let_guard)]
 #![cfg_attr(bootstrap, feature(let_chains))]
+#![feature(adt_const_params)]
 #![feature(let_else)]
 #![feature(never_type)]
-#![feature(adt_const_params)]
+#![feature(result_option_inspect)]
 #![feature(rustc_attrs)]
 #![allow(incomplete_features)]
 #![allow(rustc::potential_query_instability)]

src/test/run-make/translation/Makefile

@@ -9,16 +9,29 @@ FAKEROOT=$(TMPDIR)/fakeroot
 
 all: normal custom sysroot
 
-normal: basic-translation.rs
+# Check that the test works normally, using the built-in fallback bundle.
+normal: test.rs
 	$(RUSTC) $< 2>&1 | grep "struct literal body without path"
 
-custom: basic-translation.rs basic-translation.ftl
-	$(RUSTC) $< -Ztranslate-additional-ftl=$(CURDIR)/basic-translation.ftl 2>&1 | grep "this is a test message"
+# Check that a primary bundle can be loaded and will be preferentially used
+# where possible.
+custom: test.rs working.ftl
+	$(RUSTC) $< -Ztranslate-additional-ftl=$(CURDIR)/working.ftl 2>&1 | grep "this is a test message"
+
+# Check that a primary bundle with a broken message (e.g. a interpolated
+# variable is missing) will use the fallback bundle.
+missing: test.rs missing.ftl
+	$(RUSTC) $< -Ztranslate-additional-ftl=$(CURDIR)/missing.ftl 2>&1 | grep "struct literal body without path"
+
+# Check that a primary bundle without the desired message will use the fallback
+# bundle.
+broken: test.rs broken.ftl
+	$(RUSTC) $< -Ztranslate-additional-ftl=$(CURDIR)/broken.ftl 2>&1 | grep "struct literal body without path"
 
 # Check that a locale can be loaded from the sysroot given a language
 # identifier by making a local copy of the sysroot and adding the custom locale
 # to it.
-sysroot: basic-translation.rs basic-translation.ftl
+sysroot: test.rs working.ftl
 	mkdir $(FAKEROOT)
 	ln -s $(SYSROOT)/* $(FAKEROOT)
 	rm -f $(FAKEROOT)/lib
@@ -31,7 +44,7 @@ sysroot: basic-translation.rs basic-translation.ftl
 	mkdir $(FAKEROOT)/lib/rustlib/src
 	ln -s $(SYSROOT)/lib/rustlib/src/* $(FAKEROOT)/lib/rustlib/src
 	mkdir -p $(FAKEROOT)/share/locale/zh-CN/
-	ln -s $(CURDIR)/basic-translation.ftl $(FAKEROOT)/share/locale/zh-CN/basic-translation.ftl
+	ln -s $(CURDIR)/working.ftl $(FAKEROOT)/share/locale/zh-CN/basic-translation.ftl
 	$(RUSTC) $< --sysroot $(FAKEROOT) -Ztranslate-lang=zh-CN 2>&1 | grep "this is a test message"
 
 # Check that the compiler errors out when the sysroot requested cannot be
@@ -43,7 +56,7 @@ sysroot-missing:
 # Check that the compiler errors out when the sysroot requested cannot be
 # found. This test might start failing if there actually exists a Klingon
 # translation of rustc's error messages.
-sysroot-invalid: basic-translation.rs basic-translation.ftl
+sysroot-invalid: test.rs working.ftl
 	mkdir $(FAKEROOT)
 	ln -s $(SYSROOT)/* $(FAKEROOT)
 	rm -f $(FAKEROOT)/lib

src/test/run-make/translation/broken.ftl

@@ -0,0 +1,3 @@
+# `foo` isn't provided by this diagnostic so it is expected that the fallback message is used.
+parser-struct-literal-body-without-path = this is a {$foo} message
+    .suggestion = this is a test suggestion

src/test/run-make/translation/missing.ftl

@@ -0,0 +1,3 @@
+# `parser-struct-literal-body-without-path` isn't provided by this resource at all, so the
+# fallback should be used.
+foo = bar