Delay merging fields until they're used

Also fix a newly introduced logic issue in the merge making the tests pass.

Author: bep

.github/workflows/test.yml

@@ -18,9 +18,6 @@ jobs:
     - name: Install staticcheck
       run: go install honnef.co/go/tools/cmd/staticcheck@latest
       shell: bash
-    - name: Install golint
-      run: go install golang.org/x/lint/golint@latest
-      shell: bash
     - name: Update PATH
       run: echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
       shell: bash
@@ -34,7 +31,5 @@ jobs:
       run: go vet ./...
     - name: Staticcheck
       run: staticcheck ./...
-    - name: Lint
-      run: golint ./...
     - name: Test
       run: go test -race ./...

entry.go

@@ -18,22 +18,26 @@ var Now = time.Now
 type EntryFields struct {
 	Logger *Logger `json:"-"`
 	Fields Fields  `json:"-"`
-	start  time.Time
 }
 
 // Entry holds a Entry with a Message, Timestamp and a Level.
 // This is what is actually logged.
 type Entry struct {
 	*EntryFields
-	FieldsUnique Fields    `json:"-"`
-	Level        Level     `json:"level"`
-	Timestamp    time.Time `json:"timestamp"`
-	Message      string    `json:"message"`
+	Level     Level     `json:"level"`
+	Timestamp time.Time `json:"timestamp"`
+	Message   string    `json:"message"`
+}
+
+// FieldsDistinct returns a list of fields with duplicate names removed,
+// keeping the last.
+func (e *EntryFields) FieldsDistinct() Fields {
+	return e.distinctFieldsLastByName()
 }
 
 func (e Entry) MarshalJSON() ([]byte, error) {
 	fields := make(map[string]any)
-	for _, f := range e.FieldsUnique {
+	for _, f := range e.FieldsDistinct() {
 		fields[f.Name] = f.Value
 	}
 
@@ -155,8 +159,9 @@ func (e *EntryFields) Fatalf(msg string, v ...any) {
 	e.Fatal(fmt.Sprintf(msg, v...))
 }
 
-// mergedFields returns the fields list collapsed into a single slice.
-func (e *EntryFields) mergedFields() Fields {
+// distinctFieldsLastByName returns the fields with duplicate names removed,
+// keeping the rightmost field (last) with a given name.
+func (e *EntryFields) distinctFieldsLastByName() Fields {
 	fields := make(Fields, 0, len(e.Fields))
 	for i := len(e.Fields) - 1; i >= 0; i-- {
 		f := e.Fields[i]
@@ -168,7 +173,10 @@ func (e *EntryFields) mergedFields() Fields {
 			}
 		}
 		if !seen {
-			fields = append(fields, f)
+			// Insert first.
+			fields = append(fields, Field{})
+			copy(fields[1:], fields[:])
+			fields[0] = f
 		}
 	}
 
@@ -182,10 +190,9 @@ func (e *EntryFields) mergedFields() Fields {
 // finalize returns a copy of the Entry with Fields merged.
 func (e *EntryFields) finalize(level Level, msg string) *Entry {
 	return &Entry{
-		EntryFields:  e,
-		FieldsUnique: e.mergedFields(),
-		Level:        level,
-		Message:      msg,
-		Timestamp:    Now(),
+		EntryFields: e,
+		Level:       level,
+		Message:     msg,
+		Timestamp:   Now(),
 	}
 }

entry_test.go

@@ -13,13 +13,13 @@ func TestEntry_WithFields(t *testing.T) {
 	qt.Assert(t, a.Fields, qt.IsNil)
 
 	b := a.WithFields(Fields{{"foo", "bar"}})
-	qt.Assert(t, a.mergedFields(), qt.IsNil)
+	qt.Assert(t, a.distinctFieldsLastByName(), qt.IsNil)
 
 	c := a.WithFields(Fields{{"foo", "hello"}, {"bar", "world"}})
 	d := c.WithFields(Fields{{"baz", "jazz"}})
-	qt.Assert(t, b.mergedFields(), qt.DeepEquals, Fields{{"foo", "bar"}})
-	qt.Assert(t, c.mergedFields(), qt.DeepEquals, Fields{{"foo", "hello"}, {"bar", "world"}})
-	qt.Assert(t, d.mergedFields(), qt.DeepEquals, Fields{{"foo", "hello"}, {"bar", "world"}, {"baz", "jazz"}})
+	qt.Assert(t, b.distinctFieldsLastByName(), qt.DeepEquals, Fields{{"foo", "bar"}})
+	qt.Assert(t, c.distinctFieldsLastByName(), qt.DeepEquals, Fields{{"foo", "hello"}, {"bar", "world"}})
+	qt.Assert(t, d.distinctFieldsLastByName(), qt.DeepEquals, Fields{{"foo", "hello"}, {"bar", "world"}, {"baz", "jazz"}})
 
 	e := c.finalize(InfoLevel, "upload")
 	qt.Assert(t, "upload", qt.Equals, e.Message)
@@ -31,24 +31,24 @@ func TestEntry_WithFields(t *testing.T) {
 func TestEntry_WithField(t *testing.T) {
 	a := NewEntry(nil)
 	b := a.WithField("foo", "baz").WithField("foo", "bar")
-	qt.Assert(t, a.mergedFields(), qt.IsNil)
-	qt.Assert(t, b.mergedFields(), qt.DeepEquals, Fields{{"foo", "bar"}})
+	qt.Assert(t, a.distinctFieldsLastByName(), qt.IsNil)
+	qt.Assert(t, b.distinctFieldsLastByName(), qt.DeepEquals, Fields{{"foo", "bar"}})
 }
 
 func TestEntry_WithError(t *testing.T) {
 	a := NewEntry(nil)
 	b := a.WithError(fmt.Errorf("boom"))
-	qt.Assert(t, a.mergedFields(), qt.IsNil)
-	qt.Assert(t, b.mergedFields(), qt.DeepEquals, Fields{{"error", "boom"}})
+	qt.Assert(t, a.distinctFieldsLastByName(), qt.IsNil)
+	qt.Assert(t, b.distinctFieldsLastByName(), qt.DeepEquals, Fields{{"error", "boom"}})
 }
 
 func TestEntry_WithError_fields(t *testing.T) {
 	a := NewEntry(nil)
 	b := a.WithError(errFields("boom"))
-	qt.Assert(t, a.mergedFields(), qt.IsNil)
+	qt.Assert(t, a.distinctFieldsLastByName(), qt.IsNil)
 	qt.Assert(t,
 
-		b.mergedFields(), qt.DeepEquals, Fields{
+		b.distinctFieldsLastByName(), qt.DeepEquals, Fields{
 			{"error", "boom"},
 			{"reason", "timeout"},
 		})
@@ -58,14 +58,14 @@ func TestEntry_WithError_fields(t *testing.T) {
 func TestEntry_WithError_nil(t *testing.T) {
 	a := NewEntry(nil)
 	b := a.WithError(nil)
-	qt.Assert(t, a.mergedFields(), qt.IsNil)
-	qt.Assert(t, b.mergedFields(), qt.IsNil)
+	qt.Assert(t, a.distinctFieldsLastByName(), qt.IsNil)
+	qt.Assert(t, b.distinctFieldsLastByName(), qt.IsNil)
 }
 
 func TestEntry_WithDuration(t *testing.T) {
 	a := NewEntry(nil)
 	b := a.WithDuration(time.Second * 2)
-	qt.Assert(t, b.mergedFields(), qt.DeepEquals, Fields{{"duration", int64(2000)}})
+	qt.Assert(t, b.distinctFieldsLastByName(), qt.DeepEquals, Fields{{"duration", int64(2000)}})
 }
 
 type errFields string

handlers/cli/cli.go

@@ -67,7 +67,7 @@ func (h *Handler) HandleLog(e *log.Entry) error {
 
 	color.Fprintf(h.Writer, "%s %-25s", bold.Sprintf("%*s", h.Padding+1, level), e.Message)
 
-	for _, field := range e.FieldsUnique {
+	for _, field := range e.FieldsDistinct() {
 		if field.Name == "source" {
 			continue
 		}

handlers/text/text.go

@@ -69,7 +69,7 @@ func (h *Handler) HandleLog(e *log.Entry) error {
 	ts := time.Since(start) / time.Second
 	fmt.Fprintf(h.Writer, "\033[%dm%6s\033[0m[%04d] %-25s", color, level, ts, e.Message)
 
-	for _, f := range e.FieldsUnique {
+	for _, f := range e.FieldsDistinct() {
 		fmt.Fprintf(h.Writer, " \033[%dm%s\033[0m=%v", color, f.Name, f.Value)
 	}