From ececd1b122c741567a80acd8d60ccd6356fa5323 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?=
 <bjorn.erik.pedersen@gmail.com>
Date: Sun, 2 May 2021 14:06:16 +0200
Subject: [PATCH] langs/i18n: Fix warning regression in i18n

Fix this by

1. Making sure that only numerical values are treated as plural counts
2. Making sure that `i18n.pluralFormNotFoundError` is not logged as a warning if `other` resolved.

Note that 2. isn't a new problem, but became visible because of the plural improvements in Hugo `0.83.0`.

Fixes #8492
---
 langs/i18n/i18n.go      | 24 ++++++++++++++++++++----
 langs/i18n/i18n_test.go | 25 +++++++++++++++++++++----
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/langs/i18n/i18n.go b/langs/i18n/i18n.go
index 73e54d034..83d581a74 100644
--- a/langs/i18n/i18n.go
+++ b/langs/i18n/i18n.go
@@ -14,6 +14,7 @@
 package i18n
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 
@@ -89,10 +90,22 @@ func (t Translator) initFuncs(bndl *i18n.Bundle) {
 				PluralCount:  pluralCount,
 			})
 
-			if err == nil && currentLang == translatedLang {
+			sameLang := currentLang == translatedLang
+
+			if err == nil && sameLang {
 				return translated
 			}
 
+			if err != nil && sameLang && translated != "" {
+				// See #8492
+				// TODO(bep) this needs to be improved/fixed upstream,
+				// but currently we get an error even if the fallback to
+				// "other" succeeds.
+				if fmt.Sprintf("%T", err) == "i18n.pluralFormNotFoundError" {
+					return translated
+				}
+			}
+
 			if _, ok := err.(*i18n.MessageNotFoundErr); !ok {
 				t.logger.Warnf("Failed to get translated string for language %q and ID %q: %s", currentLangStr, translationID, err)
 			}
@@ -120,9 +133,12 @@ func (c intCount) Count() int {
 const countFieldName = "Count"
 
 // getPluralCount gets the plural count as a string (floats) or an integer.
+// If v is nil, nil is returned.
 func getPluralCount(v interface{}) interface{} {
 	if v == nil {
-		return 0
+		// i18n called without any argument, make sure it does not
+		// get any plural count.
+		return nil
 	}
 
 	switch v := v.(type) {
@@ -171,11 +187,11 @@ func toPluralCountValue(in interface{}) interface{} {
 			return in
 		}
 		// A non-numeric value.
-		return 0
+		return nil
 	default:
 		if i, err := cast.ToIntE(in); err == nil {
 			return i
 		}
-		return 0
+		return nil
 	}
 }
diff --git a/langs/i18n/i18n_test.go b/langs/i18n/i18n_test.go
index 9618d3adf..18c122010 100644
--- a/langs/i18n/i18n_test.go
+++ b/langs/i18n/i18n_test.go
@@ -358,6 +358,20 @@ other = "{{ . }} hours"`,
 				{Key: "2", Value: "2 hours"},
 			},
 		},
+		{
+			name: "Other only",
+			lang: "en",
+			id:   "hour",
+			templ: `
+[hour]
+other = "{{ with . }}{{ . }}{{ end }} hours"`,
+			variants: []types.KeyValue{
+				{Key: 1, Value: "1 hours"},
+				{Key: "1", Value: "1 hours"},
+				{Key: 2, Value: "2 hours"},
+				{Key: nil, Value: " hours"},
+			},
+		},
 		{
 			name: "Polish",
 			lang: "pl",
@@ -381,6 +395,7 @@ other = "{{ . }} miesiąca"
 
 		c.Run(test.name, func(c *qt.C) {
 			cfg := getConfig()
+			cfg.Set("enableMissingTranslationPlaceholders", true)
 			fs := hugofs.NewMem(cfg)
 
 			err := afero.WriteFile(fs.Source, filepath.Join("i18n", test.lang+".toml"), []byte(test.templ), 0755)
@@ -388,6 +403,7 @@ other = "{{ . }} miesiąca"
 
 			tp := NewTranslationProvider()
 			depsCfg := newDepsConfig(tp, cfg, fs)
+			depsCfg.Logger = loggers.NewWarningLogger()
 			d, err := deps.New(depsCfg)
 			c.Assert(err, qt.IsNil)
 			c.Assert(d.LoadResources(), qt.IsNil)
@@ -396,6 +412,7 @@ other = "{{ . }} miesiąca"
 
 			for _, variant := range test.variants {
 				c.Assert(f(test.id, variant.Key), qt.Equals, variant.Value, qt.Commentf("input: %v", variant.Key))
+				c.Assert(int(depsCfg.Logger.LogCounters().WarnCounter.Count()), qt.Equals, 0)
 			}
 
 		})
@@ -434,12 +451,12 @@ func TestGetPluralCount(t *testing.T) {
 	c.Assert(getPluralCount(map[string]interface{}{"Count": "32.5"}), qt.Equals, "32.5")
 	c.Assert(getPluralCount(map[string]interface{}{"count": 32}), qt.Equals, 32)
 	c.Assert(getPluralCount(map[string]interface{}{"Count": "32"}), qt.Equals, "32")
-	c.Assert(getPluralCount(map[string]interface{}{"Counts": 32}), qt.Equals, 0)
-	c.Assert(getPluralCount("foo"), qt.Equals, 0)
+	c.Assert(getPluralCount(map[string]interface{}{"Counts": 32}), qt.Equals, nil)
+	c.Assert(getPluralCount("foo"), qt.Equals, nil)
 	c.Assert(getPluralCount(countField{Count: 22}), qt.Equals, 22)
 	c.Assert(getPluralCount(countField{Count: 1.5}), qt.Equals, "1.5")
 	c.Assert(getPluralCount(&countField{Count: 22}), qt.Equals, 22)
-	c.Assert(getPluralCount(noCountField{Counts: 23}), qt.Equals, 0)
+	c.Assert(getPluralCount(noCountField{Counts: 23}), qt.Equals, nil)
 	c.Assert(getPluralCount(countMethod{}), qt.Equals, "32.5")
 	c.Assert(getPluralCount(&countMethod{}), qt.Equals, "32.5")
 
@@ -448,7 +465,7 @@ func TestGetPluralCount(t *testing.T) {
 	c.Assert(getPluralCount(1234.0), qt.Equals, "1234.0")
 	c.Assert(getPluralCount("1234"), qt.Equals, "1234")
 	c.Assert(getPluralCount("0.5"), qt.Equals, "0.5")
-	c.Assert(getPluralCount(nil), qt.Equals, 0)
+	c.Assert(getPluralCount(nil), qt.Equals, nil)
 }
 
 func prepareTranslationProvider(t testing.TB, test i18nTest, cfg config.Provider) *TranslationProvider {