Bug 1685180 - Debug assert Fluent strings where replaced variables are not provided; r=nordzilla

Differential Revision: https://phabricator.services.mozilla.com/D161996

intl/l10n/rust/localization-ffi/src/lib.rs

@@ -205,6 +205,8 @@ impl LocalizationRc {
             } else {
                 ret_val.set_is_void(true);
             }
+            #[cfg(debug_assertions)]
+            debug_assert_variables_exist(&errors, &[id], |id| id.to_string());
             ret_err.extend(errors.into_iter().map(|err| err.to_string().into()));
             true
         } else {
@@ -236,6 +238,8 @@ impl LocalizationRc {
                     ret_val.push(void_string);
                 }
             }
+            #[cfg(debug_assertions)]
+            debug_assert_variables_exist(&errors, &keys, |key| key.id.to_string());
             ret_err.extend(errors.into_iter().map(|err| err.to_string().into()));
             true
         } else {
@@ -272,6 +276,8 @@ impl LocalizationRc {
                 });
             }
             assert_eq!(keys.len(), ret_val.len());
+            #[cfg(debug_assertions)]
+            debug_assert_variables_exist(&errors, &keys, |key| key.id.to_string());
             ret_err.extend(errors.into_iter().map(|err| err.to_string().into()));
             true
         } else {
@@ -306,6 +312,8 @@ impl LocalizationRc {
                 v.set_is_void(true);
                 v
             };
+            #[cfg(debug_assertions)]
+            debug_assert_variables_exist(&errors, &[id], |id| id.to_string());
             let errors = errors
                 .into_iter()
                 .map(|err| err.to_string().into())
@@ -348,6 +356,8 @@ impl LocalizationRc {
 
             assert_eq!(keys.len(), ret_val.len());
 
+            #[cfg(debug_assertions)]
+            debug_assert_variables_exist(&errors, &keys, |key| key.id.to_string());
             let errors = errors
                 .into_iter()
                 .map(|err| err.to_string().into())
@@ -399,6 +409,9 @@ impl LocalizationRc {
 
             assert_eq!(keys.len(), ret_val.len());
 
+            #[cfg(debug_assertions)]
+            debug_assert_variables_exist(&errors, &keys, |key| key.id.to_string());
+
             let errors = errors
                 .into_iter()
                 .map(|err| err.to_string().into())
@@ -571,3 +584,42 @@ pub extern "C" fn localization_is_sync(loc: &LocalizationRc) -> bool {
 pub extern "C" fn localization_on_change(loc: &LocalizationRc) {
     loc.on_change();
 }
+
+#[cfg(debug_assertions)]
+fn debug_assert_variables_exist<K, F>(
+    errors: &[fluent_fallback::LocalizationError],
+    keys: &[K],
+    to_string: F,
+) where
+    F: Fn(&K) -> String,
+{
+    for error in errors {
+        if let fluent_fallback::LocalizationError::Resolver { errors, .. } = error {
+            use fluent::{
+                resolver::{errors::ReferenceKind, ResolverError},
+                FluentError,
+            };
+            for error in errors {
+                if let FluentError::ResolverError(ResolverError::Reference(
+                    ReferenceKind::Variable { id },
+                )) = error
+                {
+                    // This error needs to be actionable for Firefox engineers to fix
+                    // their Fluent issues. It might be nicer to share the specific
+                    // message, but at this point we don't have that information.
+                    eprintln!(
+                        "Fluent error, the argument \"${}\" was not provided a value.",
+                        id
+                    );
+                    eprintln!("This error happened while formatting the following messages:");
+                    for key in keys {
+                        eprintln!("  {:?}", to_string(key))
+                    }
+
+                    // Panic with the slightly more cryptic ResolverError.
+                    panic!("{}", error.to_string());
+                }
+            }
+        }
+    }
+}

intl/l10n/test/mochitest/chrome.ini

@@ -1,3 +1,6 @@
 [localization/test_formatValue.html]
+skip-if = debug # Intentionally triggers a debug assert for missing Fluent arguments.
 [localization/test_formatValues.html]
-[localization/test_formatMessages.html]
+skip-if = debug # Intentionally triggers a debug assert for missing Fluent arguments.
+[localization/test_formatMessages.html]
+skip-if = debug # Intentionally triggers a debug assert for missing Fluent arguments.

intl/l10n/test/test_missing_variables.js

@@ -0,0 +1,28 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * The following test demonstrates crashing behavior.
+ */
+add_task(function test_missing_variables() {
+  const l10nReg = new L10nRegistry();
+
+  const fs = [
+    { path: "/localization/en-US/browser/test.ftl", source: "welcome-message = Welcome { $user }\n" }
+  ]
+  const locales = ["en-US"];
+  const source = L10nFileSource.createMock("test", "app", locales, "/localization/{locale}", fs);
+  l10nReg.registerSources([source]);
+  const l10n = new Localization(["/browser/test.ftl"], true, l10nReg, locales);
+
+  {
+    const [message] = l10n.formatValuesSync([{ id: "welcome-message", args: { user: "Greg" } }]);
+    equal(message, "Welcome Greg");
+  }
+
+  {
+    // This will crash in debug builds.
+    const [message] = l10n.formatValuesSync([{ id: "welcome-message" }]);
+    equal(message, "Welcome {$user}");
+  }
+});

intl/l10n/test/xpcshell.ini

@@ -8,4 +8,6 @@ head =
 [test_localization.js]
 [test_localization_sync.js]
 [test_messagecontext.js]
+[test_missing_variables.js]
+skip-if = debug # Intentionally triggers a debug assert for missing Fluent arguments.
 [test_pseudo.js]