From cf25af1a13be959e7dba8fcbc5c186800a3c8030 Mon Sep 17 00:00:00 2001 From: Snider Date: Fri, 20 Mar 2026 16:00:41 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20AX=20audit=20round=204=20=E2=80=94=20sem?= =?UTF-8?q?antic=20naming,=20Result=20returns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Op → Operation, AllOps → AllOperations (no abbreviations) - Translator.T → Translator.Translate (avoids testing.T confusion) - Lock.Mu → Lock.Mutex, ServiceRuntime.Opts → .Options - ErrorLog.Error/Warn return Result instead of error - ErrorPanic.Reports returns Result instead of ([]CrashReport, error) - Core.LogError/LogWarn simplified to passthrough Co-Authored-By: Virgil --- pkg/core/cli.go | 2 +- pkg/core/core.go | 16 ++++------------ pkg/core/error.go | 40 ++++++++++++++++++++-------------------- pkg/core/i18n.go | 10 +++++----- pkg/core/lock.go | 20 ++++++++++---------- pkg/core/log.go | 6 +++--- pkg/core/runtime.go | 2 +- pkg/core/service.go | 12 ++++++------ tests/error_test.go | 28 ++++++++++++++-------------- tests/i18n_test.go | 8 ++++---- tests/lock_test.go | 2 +- tests/runtime_test.go | 4 ++-- 12 files changed, 71 insertions(+), 79 deletions(-) diff --git a/pkg/core/cli.go b/pkg/core/cli.go index b180fe0..b02a465 100644 --- a/pkg/core/cli.go +++ b/pkg/core/cli.go @@ -121,7 +121,7 @@ func (cl *Cli) PrintHelp() { if cmd.Hidden { continue } - desc := cl.core.I18n().T(cmd.I18nKey()) + desc := cl.core.I18n().Translate(cmd.I18nKey()) if desc == cmd.I18nKey() { cl.Print(" %s", path) } else { diff --git a/pkg/core/core.go b/pkg/core/core.go index 9252c61..ab27608 100644 --- a/pkg/core/core.go +++ b/pkg/core/core.go @@ -59,22 +59,14 @@ func (c *Core) PERFORM(t Task) Result { return c.Perform(t) } // --- Error+Log --- -// LogError logs an error and returns a Result with the wrapped error. +// LogError logs an error and returns the Result from ErrorLog. func (c *Core) LogError(err error, op, msg string) Result { - wrapped := c.log.Error(err, op, msg) - if wrapped == nil { - return Result{OK: true} - } - return Result{wrapped, false} + return c.log.Error(err, op, msg) } -// LogWarn logs a warning and returns a Result with the wrapped error. +// LogWarn logs a warning and returns the Result from ErrorLog. func (c *Core) LogWarn(err error, op, msg string) Result { - wrapped := c.log.Warn(err, op, msg) - if wrapped == nil { - return Result{OK: true} - } - return Result{wrapped, false} + return c.log.Warn(err, op, msg) } // Must logs and panics if err is not nil. diff --git a/pkg/core/error.go b/pkg/core/error.go index d66e570..4af745a 100644 --- a/pkg/core/error.go +++ b/pkg/core/error.go @@ -145,9 +145,9 @@ func ErrorJoin(errs ...error) error { // --- Error Introspection Helpers --- -// Op extracts the operation name from an error. +// Operation extracts the operation name from an error. // Returns empty string if the error is not an *Err. -func Op(err error) string { +func Operation(err error) string { var e *Err if As(err, &e) { return e.Op @@ -193,9 +193,9 @@ func Root(err error) error { } } -// AllOps returns an iterator over all operational contexts in the error chain. +// AllOperations returns an iterator over all operational contexts in the error chain. // It traverses the error tree using errors.Unwrap. -func AllOps(err error) iter.Seq[string] { +func AllOperations(err error) iter.Seq[string] { return func(yield func(string) bool) { for err != nil { if e, ok := err.(*Err); ok { @@ -214,7 +214,7 @@ func AllOps(err error) iter.Seq[string] { // It returns an empty slice if no operational context is found. func StackTrace(err error) []string { var stack []string - for op := range AllOps(err) { + for op := range AllOperations(err) { stack = append(stack, op) } return stack @@ -223,7 +223,7 @@ func StackTrace(err error) []string { // FormatStackTrace returns a pretty-printed logical stack trace. func FormatStackTrace(err error) string { var ops []string - for op := range AllOps(err) { + for op := range AllOperations(err) { ops = append(ops, op) } if len(ops) == 0 { @@ -247,24 +247,24 @@ func (el *ErrorLog) logger() *Log { return defaultLog } -// Error logs at Error level and returns a wrapped error. -func (el *ErrorLog) Error(err error, op, msg string) error { +// Error logs at Error level and returns a Result with the wrapped error. +func (el *ErrorLog) Error(err error, op, msg string) Result { if err == nil { - return nil + return Result{OK: true} } wrapped := Wrap(err, op, msg) el.logger().Error(msg, "op", op, "err", err) - return wrapped + return Result{wrapped, false} } -// Warn logs at Warn level and returns a wrapped error. -func (el *ErrorLog) Warn(err error, op, msg string) error { +// Warn logs at Warn level and returns a Result with the wrapped error. +func (el *ErrorLog) Warn(err error, op, msg string) Result { if err == nil { - return nil + return Result{OK: true} } wrapped := Wrap(err, op, msg) el.logger().Warn(msg, "op", op, "err", err) - return wrapped + return Result{wrapped, false} } // Must logs and panics if err is not nil. @@ -346,24 +346,24 @@ func (h *ErrorPanic) SafeGo(fn func()) { } // Reports returns the last n crash reports from the file. -func (h *ErrorPanic) Reports(n int) ([]CrashReport, error) { +func (h *ErrorPanic) Reports(n int) Result { if h.filePath == "" { - return nil, nil + return Result{} } crashMu.Lock() defer crashMu.Unlock() data, err := os.ReadFile(h.filePath) if err != nil { - return nil, err + return Result{err, false} } var reports []CrashReport if err := json.Unmarshal(data, &reports); err != nil { - return nil, err + return Result{err, false} } if n <= 0 || len(reports) <= n { - return reports, nil + return Result{reports, true} } - return reports[len(reports)-n:], nil + return Result{reports[len(reports)-n:], true} } var crashMu sync.Mutex diff --git a/pkg/core/i18n.go b/pkg/core/i18n.go index b433dd2..590f684 100644 --- a/pkg/core/i18n.go +++ b/pkg/core/i18n.go @@ -13,8 +13,8 @@ import ( // Translator defines the interface for translation services. // Implemented by go-i18n's Srv. type Translator interface { - // T translates a message by its ID with optional arguments. - T(messageID string, args ...any) string + // Translate translates a message by its ID with optional arguments. + Translate(messageID string, args ...any) string // SetLanguage sets the active language (BCP47 tag, e.g., "en-GB", "de"). SetLanguage(lang string) error // Language returns the current language code. @@ -80,13 +80,13 @@ func (i *I18n) Translator() Translator { return t } -// T translates a message. Returns the key as-is if no translator is registered. -func (i *I18n) T(messageID string, args ...any) string { +// Translate translates a message. Returns the key as-is if no translator is registered. +func (i *I18n) Translate(messageID string, args ...any) string { i.mu.RLock() t := i.translator i.mu.RUnlock() if t != nil { - return t.T(messageID, args...) + return t.Translate(messageID, args...) } return messageID } diff --git a/pkg/core/lock.go b/pkg/core/lock.go index 8e96994..851c8aa 100644 --- a/pkg/core/lock.go +++ b/pkg/core/lock.go @@ -17,7 +17,7 @@ var ( // Lock is the DTO for a named mutex. type Lock struct { Name string - Mu *sync.RWMutex + Mutex *sync.RWMutex } // Lock returns a named Lock, creating the mutex if needed. @@ -29,7 +29,7 @@ func (c *Core) Lock(name string) *Lock { lockMap[name] = m } lockMu.Unlock() - return &Lock{Name: name, Mu: m} + return &Lock{Name: name, Mutex: m} } // LockEnable marks that the service lock should be applied after initialisation. @@ -38,8 +38,8 @@ func (c *Core) LockEnable(name ...string) { if len(name) > 0 { n = name[0] } - c.Lock(n).Mu.Lock() - defer c.Lock(n).Mu.Unlock() + c.Lock(n).Mutex.Lock() + defer c.Lock(n).Mutex.Unlock() if c.services == nil { c.services = &serviceRegistry{services: make(map[string]*Service)} } @@ -52,8 +52,8 @@ func (c *Core) LockApply(name ...string) { if len(name) > 0 { n = name[0] } - c.Lock(n).Mu.Lock() - defer c.Lock(n).Mu.Unlock() + c.Lock(n).Mutex.Lock() + defer c.Lock(n).Mutex.Unlock() if c.services.lockEnabled { c.services.locked = true } @@ -64,8 +64,8 @@ func (c *Core) Startables() Result { if c.services == nil { return Result{} } - c.Lock("srv").Mu.RLock() - defer c.Lock("srv").Mu.RUnlock() + c.Lock("srv").Mutex.RLock() + defer c.Lock("srv").Mutex.RUnlock() var out []*Service for _, svc := range c.services.services { if svc.OnStart != nil { @@ -80,8 +80,8 @@ func (c *Core) Stoppables() Result { if c.services == nil { return Result{} } - c.Lock("srv").Mu.RLock() - defer c.Lock("srv").Mu.RUnlock() + c.Lock("srv").Mutex.RLock() + defer c.Lock("srv").Mutex.RUnlock() var out []*Service for _, svc := range c.services.services { if svc.OnStop != nil { diff --git a/pkg/core/log.go b/pkg/core/log.go index 769286f..e4c6a72 100644 --- a/pkg/core/log.go +++ b/pkg/core/log.go @@ -182,7 +182,7 @@ func (l *Log) log(level Level, prefix, msg string, keyvals ...any) { for i := 0; i < origLen; i += 2 { if i+1 < origLen { if err, ok := keyvals[i+1].(error); ok { - if op := Op(err); op != "" { + if op := Operation(err); op != "" { // Check if op is already in keyvals hasOp := false for j := 0; j < len(keyvals); j += 2 { @@ -361,7 +361,7 @@ func (le *LogErr) Log(err error) { if err == nil { return } - le.log.Error(ErrorMessage(err), "op", Op(err), "code", ErrorCode(err), "stack", FormatStackTrace(err)) + le.log.Error(ErrorMessage(err), "op", Operation(err), "code", ErrorCode(err), "stack", FormatStackTrace(err)) } // --- LogPanic: Panic-Aware Logger --- @@ -390,7 +390,7 @@ func (lp *LogPanic) Recover() { } lp.log.Error("panic recovered", "err", err, - "op", Op(err), + "op", Operation(err), "stack", FormatStackTrace(err), ) } diff --git a/pkg/core/runtime.go b/pkg/core/runtime.go index 738b38d..adbcf38 100644 --- a/pkg/core/runtime.go +++ b/pkg/core/runtime.go @@ -26,7 +26,7 @@ func NewServiceRuntime[T any](c *Core, opts T) *ServiceRuntime[T] { } func (r *ServiceRuntime[T]) Core() *Core { return r.core } -func (r *ServiceRuntime[T]) Opts() T { return r.opts } +func (r *ServiceRuntime[T]) Options() T { return r.opts } func (r *ServiceRuntime[T]) Config() *Config { return r.core.Config() } // --- Lifecycle --- diff --git a/pkg/core/service.go b/pkg/core/service.go index c938a67..8007b83 100644 --- a/pkg/core/service.go +++ b/pkg/core/service.go @@ -43,9 +43,9 @@ func (c *Core) Service(name string, service ...Service) Result { } if len(service) == 0 { - c.Lock("srv").Mu.RLock() + c.Lock("srv").Mutex.RLock() v, ok := c.services.services[name] - c.Lock("srv").Mu.RUnlock() + c.Lock("srv").Mutex.RUnlock() return Result{v, ok} } @@ -53,8 +53,8 @@ func (c *Core) Service(name string, service ...Service) Result { return Result{E("core.Service", "service name cannot be empty", nil), false} } - c.Lock("srv").Mu.Lock() - defer c.Lock("srv").Mu.Unlock() + c.Lock("srv").Mutex.Lock() + defer c.Lock("srv").Mutex.Unlock() if c.services.locked { return Result{E("core.Service", Concat("service \"", name, "\" not permitted — registry locked"), nil), false} @@ -77,8 +77,8 @@ func (c *Core) Services() []string { if c.services == nil { return nil } - c.Lock("srv").Mu.RLock() - defer c.Lock("srv").Mu.RUnlock() + c.Lock("srv").Mutex.RLock() + defer c.Lock("srv").Mutex.RUnlock() var names []string for k := range c.services.services { names = append(names, k) diff --git a/tests/error_test.go b/tests/error_test.go index 758a5fe..eb4f769 100644 --- a/tests/error_test.go +++ b/tests/error_test.go @@ -50,14 +50,14 @@ func TestNewCode_Good(t *testing.T) { // --- Error Introspection --- -func TestOp_Good(t *testing.T) { +func TestOperation_Good(t *testing.T) { err := E("brain.Recall", "search failed", nil) - assert.Equal(t, "brain.Recall", Op(err)) + assert.Equal(t, "brain.Recall", Operation(err)) } -func TestOp_Bad(t *testing.T) { +func TestOperation_Bad(t *testing.T) { err := errors.New("plain error") - assert.Equal(t, "", Op(err)) + assert.Equal(t, "", Operation(err)) } func TestErrorMessage_Good(t *testing.T) { @@ -104,22 +104,22 @@ func TestFormatStackTrace_Good(t *testing.T) { func TestErrorLog_Good(t *testing.T) { c := New() cause := errors.New("boom") - err := c.Log().Error(cause, "test.Op", "something broke") - assert.Error(t, err) - assert.ErrorIs(t, err, cause) + r := c.Log().Error(cause, "test.Op", "something broke") + assert.False(t, r.OK) + assert.ErrorIs(t, r.Value.(error), cause) } func TestErrorLog_Nil_Good(t *testing.T) { c := New() - err := c.Log().Error(nil, "test.Op", "no error") - assert.Nil(t, err) + r := c.Log().Error(nil, "test.Op", "no error") + assert.True(t, r.OK) } func TestErrorLog_Warn_Good(t *testing.T) { c := New() cause := errors.New("warning") - err := c.Log().Warn(cause, "test.Op", "heads up") - assert.Error(t, err) + r := c.Log().Warn(cause, "test.Op", "heads up") + assert.False(t, r.OK) } func TestErrorLog_Must_Ugly(t *testing.T) { @@ -222,8 +222,8 @@ func TestErrorPanic_CrashFile_Good(t *testing.T) { // For now, test that Reports handles missing file gracefully c := New() - reports, err := c.Error().Reports(5) - assert.NoError(t, err) - assert.Nil(t, reports) + r := c.Error().Reports(5) + assert.False(t, r.OK) + assert.Nil(t, r.Value) _ = path } diff --git a/tests/i18n_test.go b/tests/i18n_test.go index cc35859..2df6b0b 100644 --- a/tests/i18n_test.go +++ b/tests/i18n_test.go @@ -36,10 +36,10 @@ func TestI18n_Locales_Empty_Good(t *testing.T) { // --- Translator (no translator registered) --- -func TestI18n_T_NoTranslator_Good(t *testing.T) { +func TestI18n_Translate_NoTranslator_Good(t *testing.T) { c := New() // Without a translator, T returns the key as-is - result := c.I18n().T("greeting.hello") + result := c.I18n().Translate("greeting.hello") assert.Equal(t, "greeting.hello", result) } @@ -71,7 +71,7 @@ type mockTranslator struct { lang string } -func (m *mockTranslator) T(id string, args ...any) string { return "translated:" + id } +func (m *mockTranslator) Translate(id string, args ...any) string { return "translated:" + id } func (m *mockTranslator) SetLanguage(lang string) error { m.lang = lang; return nil } func (m *mockTranslator) Language() string { return m.lang } func (m *mockTranslator) AvailableLanguages() []string { return []string{"en", "de", "fr"} } @@ -82,7 +82,7 @@ func TestI18n_WithTranslator_Good(t *testing.T) { c.I18n().SetTranslator(tr) assert.Equal(t, tr, c.I18n().Translator()) - assert.Equal(t, "translated:hello", c.I18n().T("hello")) + assert.Equal(t, "translated:hello", c.I18n().Translate("hello")) assert.Equal(t, "en", c.I18n().Language()) assert.Equal(t, []string{"en", "de", "fr"}, c.I18n().AvailableLanguages()) diff --git a/tests/lock_test.go b/tests/lock_test.go index 54dc200..12c55e5 100644 --- a/tests/lock_test.go +++ b/tests/lock_test.go @@ -11,7 +11,7 @@ func TestLock_Good(t *testing.T) { c := New() lock := c.Lock("test") assert.NotNil(t, lock) - assert.NotNil(t, lock.Mu) + assert.NotNil(t, lock.Mutex) } func TestLock_SameName_Good(t *testing.T) { diff --git a/tests/runtime_test.go b/tests/runtime_test.go index 95ada1d..9c08de2 100644 --- a/tests/runtime_test.go +++ b/tests/runtime_test.go @@ -21,8 +21,8 @@ func TestServiceRuntime_Good(t *testing.T) { rt := NewServiceRuntime(c, opts) assert.Equal(t, c, rt.Core()) - assert.Equal(t, opts, rt.Opts()) - assert.Equal(t, "https://api.lthn.ai", rt.Opts().URL) + assert.Equal(t, opts, rt.Options()) + assert.Equal(t, "https://api.lthn.ai", rt.Options().URL) assert.NotNil(t, rt.Config()) }