From 2eebd34cd50c1486024da06d24415781af3a0a54 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 10 Aug 2022 11:48:25 +0100 Subject: [PATCH] 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. Signed-off-by: David Wood --- compiler/rustc_errors/src/emitter.rs | 80 ++++++++++++------- compiler/rustc_errors/src/lib.rs | 3 +- src/test/run-make/translation/Makefile | 25 ++++-- src/test/run-make/translation/broken.ftl | 3 + src/test/run-make/translation/missing.ftl | 3 + .../{basic-translation.rs => test.rs} | 0 .../{basic-translation.ftl => working.ftl} | 0 7 files changed, 76 insertions(+), 38 deletions(-) create mode 100644 src/test/run-make/translation/broken.ftl create mode 100644 src/test/run-make/translation/missing.ftl rename src/test/run-make/translation/{basic-translation.rs => test.rs} (100%) rename src/test/run-make/translation/{basic-translation.ftl => working.ftl} (100%) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 61d953cd6f1cc..753e2f07c042e 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/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 diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 15c1858023de7..f83e972efd56e 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/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)] diff --git a/src/test/run-make/translation/Makefile b/src/test/run-make/translation/Makefile index bfff75e7acb08..20e86c7f9a072 100644 --- a/src/test/run-make/translation/Makefile +++ b/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 diff --git a/src/test/run-make/translation/broken.ftl b/src/test/run-make/translation/broken.ftl new file mode 100644 index 0000000000000..1482dd2824aed --- /dev/null +++ b/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 diff --git a/src/test/run-make/translation/missing.ftl b/src/test/run-make/translation/missing.ftl new file mode 100644 index 0000000000000..43076b1d6ae79 --- /dev/null +++ b/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 diff --git a/src/test/run-make/translation/basic-translation.rs b/src/test/run-make/translation/test.rs similarity index 100% rename from src/test/run-make/translation/basic-translation.rs rename to src/test/run-make/translation/test.rs diff --git a/src/test/run-make/translation/basic-translation.ftl b/src/test/run-make/translation/working.ftl similarity index 100% rename from src/test/run-make/translation/basic-translation.ftl rename to src/test/run-make/translation/working.ftl