From 1d6d891932d231b905e724ee31ce8c6d60f24a9c Mon Sep 17 00:00:00 2001
From: Snider <snider@host.uk.com>
Date: Tue, 17 Mar 2026 14:20:29 +0000
Subject: [PATCH] fix(coderabbit): address review findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. core_service.go: Return error when SetLanguage fails instead of
   silently discarding it.
2. grammar.go/service.go: Add MergeGrammarData() and use it in AddLoader
   and loadJSON so multiple loaders merge grammar entries instead of
   replacing the entire struct per language.
3. service.go: Document that package-level AddLoader is superseded when
   NewCoreService replaces the default — by design, not a bug.
4. service.go: Use CompareAndSwap in Init() to prevent TOCTOU race where
   a concurrent SetDefault could be overwritten.

Co-Authored-By: Virgil <virgil@lethean.io>
---
 core_service.go |  5 ++++-
 grammar.go      | 17 ++++++++++++++++-
 service.go      | 15 +++++++++++----
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/core_service.go b/core_service.go
index f10980e..026b85e 100644
--- a/core_service.go
+++ b/core_service.go
@@ -4,6 +4,7 @@ package i18n
 
 import (
 	"context"
+	"fmt"
 	"io/fs"
 	"sync"
 
@@ -65,7 +66,9 @@ func NewCoreService(opts ServiceOptions) func(*core.Core) (any, error) {
 		}
 
 		if opts.Language != "" {
-			_ = svc.SetLanguage(opts.Language)
+			if langErr := svc.SetLanguage(opts.Language); langErr != nil {
+				return nil, fmt.Errorf("i18n: invalid language %q: %w", opts.Language, langErr)
+			}
 		}
 
 		svc.SetMode(opts.Mode)
diff --git a/grammar.go b/grammar.go
index bb3037b..3a97674 100644
--- a/grammar.go
+++ b/grammar.go
@@ -14,13 +14,28 @@ func GetGrammarData(lang string) *GrammarData {
 	return grammarCache[lang]
 }
 
-// SetGrammarData sets the grammar data for a language.
+// SetGrammarData sets the grammar data for a language, replacing any existing data.
 func SetGrammarData(lang string, data *GrammarData) {
 	grammarCacheMu.Lock()
 	defer grammarCacheMu.Unlock()
 	grammarCache[lang] = data
 }
 
+// MergeGrammarData merges grammar data into the existing data for a language.
+// New entries are added; existing entries are overwritten per-key.
+func MergeGrammarData(lang string, data *GrammarData) {
+	grammarCacheMu.Lock()
+	defer grammarCacheMu.Unlock()
+	existing := grammarCache[lang]
+	if existing == nil {
+		grammarCache[lang] = data
+		return
+	}
+	maps.Copy(existing.Verbs, data.Verbs)
+	maps.Copy(existing.Nouns, data.Nouns)
+	maps.Copy(existing.Words, data.Words)
+}
+
 // IrregularVerbs returns a copy of the irregular verb forms map.
 func IrregularVerbs() map[string]VerbForms {
 	result := make(map[string]VerbForms, len(irregularVerbs))
diff --git a/service.go b/service.go
index 6a1c36c..e3641a0 100644
--- a/service.go
+++ b/service.go
@@ -139,7 +139,9 @@ func Init() error {
 		}
 		svc, err := New()
 		if err == nil {
-			defaultService.Store(svc)
+			// CAS prevents overwriting a concurrent SetDefault call that
+			// raced between the Load check above and this store.
+			defaultService.CompareAndSwap(nil, svc)
 		}
 		defaultErr = err
 	})
@@ -170,6 +172,10 @@ func SetDefault(s *Service) {
 //	//go:embed *.json
 //	var localeFS embed.FS
 //	func init() { i18n.AddLoader(i18n.NewFSLoader(localeFS, ".")) }
+//
+// Note: When using the Core framework, NewCoreService creates a fresh Service
+// and calls SetDefault, so init-time AddLoader calls are superseded. In that
+// context, packages should implement LocaleProvider instead.
 func AddLoader(loader Loader) {
 	svc := Default()
 	if svc == nil {
@@ -196,7 +202,7 @@ func (s *Service) loadJSON(lang string, data []byte) error {
 		s.messages[lang] = messages
 	}
 	if len(grammarData.Verbs) > 0 || len(grammarData.Nouns) > 0 || len(grammarData.Words) > 0 {
-		SetGrammarData(lang, grammarData)
+		MergeGrammarData(lang, grammarData)
 	}
 	return nil
 }
@@ -476,9 +482,10 @@ func (s *Service) AddLoader(loader Loader) error {
 			s.messages[lang][k] = v
 		}
 
-		// Merge grammar data into the global grammar store
+		// Merge grammar data into the global grammar store (merge, not replace,
+		// so that multiple loaders contribute entries for the same language).
 		if grammar != nil && (len(grammar.Verbs) > 0 || len(grammar.Nouns) > 0 || len(grammar.Words) > 0) {
-			SetGrammarData(lang, grammar)
+			MergeGrammarData(lang, grammar)
 		}
 
 		tag := language.Make(lang)
-- 
2.45.3