fix(i18n): address remaining code review issues
- service.go: Simplify Default() to just call Init() (sync.Once handles idempotency) - service.go: Add nil check to SetDefault() with panic - service.go: Add documentation for ModeStrict panic behavior - loader.go: Add LanguagesErr() to expose directory scan errors - loader.go: Use path.Join instead of filepath.Join for fs.FS compatibility - transform.go: Add uint, uint8-64, int8, int16 support to type converters - grammar.go: Replace deprecated strings.Title with unicode-aware implementation - i18n_test.go: Add comprehensive concurrency tests with race detector Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
285a4582b0
commit
7f4682ab17
5 changed files with 158 additions and 6 deletions
|
|
@ -393,8 +393,21 @@ func isVowel(r rune) bool {
|
|||
}
|
||||
|
||||
// Title capitalizes the first letter of each word.
|
||||
// Uses unicode-aware casing for proper internationalization.
|
||||
// Word boundaries are defined as any non-letter character (matching strings.Title behavior).
|
||||
func Title(s string) string {
|
||||
return strings.Title(s) //nolint:staticcheck // strings.Title is fine for our use case
|
||||
var b strings.Builder
|
||||
b.Grow(len(s))
|
||||
prev := ' ' // Treat start of string as word boundary
|
||||
for _, r := range s {
|
||||
if !unicode.IsLetter(prev) && unicode.IsLetter(r) {
|
||||
b.WriteRune(unicode.ToUpper(r))
|
||||
} else {
|
||||
b.WriteRune(r)
|
||||
}
|
||||
prev = r
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// Quote wraps a string in double quotes.
|
||||
|
|
|
|||
|
|
@ -495,3 +495,83 @@ func TestNewWithFS(t *testing.T) {
|
|||
assert.True(t, svc.Debug())
|
||||
})
|
||||
}
|
||||
|
||||
func TestConcurrentTranslation(t *testing.T) {
|
||||
svc, err := New()
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Run("concurrent T calls", func(t *testing.T) {
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 100; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
result := svc.T("cmd.dev.short")
|
||||
assert.Equal(t, "Multi-repo development workflow", result)
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
})
|
||||
|
||||
t.Run("concurrent T with args", func(t *testing.T) {
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 100; i++ {
|
||||
wg.Add(1)
|
||||
go func(n int) {
|
||||
defer wg.Done()
|
||||
result := svc.T("i18n.count.file", n)
|
||||
if n == 1 {
|
||||
assert.Equal(t, "1 file", result)
|
||||
} else {
|
||||
assert.Contains(t, result, "files")
|
||||
}
|
||||
}(i)
|
||||
}
|
||||
wg.Wait()
|
||||
})
|
||||
|
||||
t.Run("concurrent read and write", func(t *testing.T) {
|
||||
var wg sync.WaitGroup
|
||||
|
||||
// Readers
|
||||
for i := 0; i < 50; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
_ = svc.T("cmd.dev.short")
|
||||
_ = svc.Language()
|
||||
_ = svc.Formality()
|
||||
}()
|
||||
}
|
||||
|
||||
// Writers
|
||||
for i := 0; i < 10; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
svc.SetFormality(FormalityNeutral)
|
||||
svc.SetDebug(false)
|
||||
}()
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
})
|
||||
}
|
||||
|
||||
func TestConcurrentDefault(t *testing.T) {
|
||||
// Reset for test
|
||||
defaultService.Store(nil)
|
||||
defaultOnce = sync.Once{}
|
||||
defaultErr = nil
|
||||
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 50; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
svc := Default()
|
||||
assert.NotNil(t, svc)
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import (
|
|||
"encoding/json"
|
||||
"fmt"
|
||||
"io/fs"
|
||||
"path/filepath"
|
||||
"path"
|
||||
"strings"
|
||||
"sync"
|
||||
)
|
||||
|
|
@ -18,6 +18,7 @@ type FSLoader struct {
|
|||
// Cache of available languages (populated on first Languages() call)
|
||||
languages []string
|
||||
langOnce sync.Once
|
||||
langErr error // Error from directory scan, if any
|
||||
}
|
||||
|
||||
// NewFSLoader creates a loader for the given filesystem and directory.
|
||||
|
|
@ -40,7 +41,7 @@ func (l *FSLoader) Load(lang string) (map[string]Message, *GrammarData, error) {
|
|||
var data []byte
|
||||
var err error
|
||||
for _, filename := range variants {
|
||||
filePath := filepath.Join(l.dir, filename)
|
||||
filePath := path.Join(l.dir, filename) // Use path.Join for fs.FS (forward slashes)
|
||||
data, err = fs.ReadFile(l.fsys, filePath)
|
||||
if err == nil {
|
||||
break
|
||||
|
|
@ -69,10 +70,12 @@ func (l *FSLoader) Load(lang string) (map[string]Message, *GrammarData, error) {
|
|||
|
||||
// Languages implements Loader.Languages - returns available language codes.
|
||||
// Thread-safe: uses sync.Once to ensure the directory is scanned only once.
|
||||
// Returns nil if the directory scan failed (check LanguagesErr for details).
|
||||
func (l *FSLoader) Languages() []string {
|
||||
l.langOnce.Do(func() {
|
||||
entries, err := fs.ReadDir(l.fsys, l.dir)
|
||||
if err != nil {
|
||||
l.langErr = fmt.Errorf("failed to read locale directory %q: %w", l.dir, err)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -90,6 +93,13 @@ func (l *FSLoader) Languages() []string {
|
|||
return l.languages
|
||||
}
|
||||
|
||||
// LanguagesErr returns any error that occurred during Languages() scan.
|
||||
// Returns nil if the scan succeeded.
|
||||
func (l *FSLoader) LanguagesErr() error {
|
||||
l.Languages() // Ensure scan has been attempted
|
||||
return l.langErr
|
||||
}
|
||||
|
||||
// Ensure FSLoader implements Loader at compile time.
|
||||
var _ Loader = (*FSLoader)(nil)
|
||||
|
||||
|
|
|
|||
|
|
@ -161,15 +161,17 @@ func Init() error {
|
|||
// Default returns the global i18n service, initializing if needed.
|
||||
// Thread-safe: can be called concurrently.
|
||||
func Default() *Service {
|
||||
if defaultService.Load() == nil {
|
||||
_ = Init()
|
||||
}
|
||||
_ = Init() // sync.Once handles idempotency
|
||||
return defaultService.Load()
|
||||
}
|
||||
|
||||
// SetDefault sets the global i18n service.
|
||||
// Thread-safe: can be called concurrently with Default().
|
||||
// Panics if s is nil.
|
||||
func SetDefault(s *Service) {
|
||||
if s == nil {
|
||||
panic("i18n: SetDefault called with nil service")
|
||||
}
|
||||
defaultService.Store(s)
|
||||
}
|
||||
|
||||
|
|
@ -490,9 +492,14 @@ func (s *Service) getEffectiveFormality(data any) Formality {
|
|||
|
||||
// handleMissingKey handles a missing translation key based on the current mode.
|
||||
// Must be called with s.mu.RLock held.
|
||||
//
|
||||
// In ModeStrict, this panics - use only in development/CI to catch missing keys.
|
||||
// In ModeCollect, this dispatches to OnMissingKey handler for logging/collection.
|
||||
// In ModeNormal (default), this returns the key as-is.
|
||||
func (s *Service) handleMissingKey(key string, args []any) string {
|
||||
switch s.mode {
|
||||
case ModeStrict:
|
||||
// WARNING: Panics! Use ModeStrict only in development/CI environments.
|
||||
panic(fmt.Sprintf("i18n: missing translation key %q", key))
|
||||
case ModeCollect:
|
||||
// Convert args to map for the action
|
||||
|
|
|
|||
|
|
@ -31,6 +31,20 @@ func toInt(v any) int {
|
|||
return int(n)
|
||||
case int32:
|
||||
return int(n)
|
||||
case int16:
|
||||
return int(n)
|
||||
case int8:
|
||||
return int(n)
|
||||
case uint:
|
||||
return int(n)
|
||||
case uint64:
|
||||
return int(n)
|
||||
case uint32:
|
||||
return int(n)
|
||||
case uint16:
|
||||
return int(n)
|
||||
case uint8:
|
||||
return int(n)
|
||||
case float64:
|
||||
return int(n)
|
||||
case float32:
|
||||
|
|
@ -51,6 +65,20 @@ func toInt64(v any) int64 {
|
|||
return n
|
||||
case int32:
|
||||
return int64(n)
|
||||
case int16:
|
||||
return int64(n)
|
||||
case int8:
|
||||
return int64(n)
|
||||
case uint:
|
||||
return int64(n)
|
||||
case uint64:
|
||||
return int64(n)
|
||||
case uint32:
|
||||
return int64(n)
|
||||
case uint16:
|
||||
return int64(n)
|
||||
case uint8:
|
||||
return int64(n)
|
||||
case float64:
|
||||
return int64(n)
|
||||
case float32:
|
||||
|
|
@ -75,6 +103,20 @@ func toFloat64(v any) float64 {
|
|||
return float64(n)
|
||||
case int32:
|
||||
return float64(n)
|
||||
case int16:
|
||||
return float64(n)
|
||||
case int8:
|
||||
return float64(n)
|
||||
case uint:
|
||||
return float64(n)
|
||||
case uint64:
|
||||
return float64(n)
|
||||
case uint32:
|
||||
return float64(n)
|
||||
case uint16:
|
||||
return float64(n)
|
||||
case uint8:
|
||||
return float64(n)
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue