slog: rewrite commonHandler

Reorganize and simplify commonHandler.Handle.

- Simplify the appender interface by removing all state from it.

- Introduce a handleState struct to track state. In particular, we have
  to track whether or not we have written the first key, to know when to
  write a separator. This fixes a bug in the previous version.

Also, improve commonHandler tests to check preformatted Attrs.

Change-Id: Ia87eee200cea1617ff5bae486e8a52eab7aadc5d
Reviewed-on: https://go-review.googlesource.com/c/exp/+/432184
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
diff --git a/slog/handler.go b/slog/handler.go
index 5ab9fbf..2661ac1 100644
--- a/slog/handler.go
+++ b/slog/handler.go
@@ -93,9 +93,9 @@
 }
 
 type commonHandler struct {
-	newAppender       func(*buffer.Buffer) appender
 	opts              HandlerOptions
-	attrs             []Attr
+	app               appender
+	attrSep           byte // char separating attrs from each other
 	preformattedAttrs []byte
 	mu                sync.Mutex
 	w                 io.Writer
@@ -109,132 +109,162 @@
 
 func (h *commonHandler) with(as []Attr) *commonHandler {
 	h2 := &commonHandler{
-		newAppender:       h.newAppender,
+		app:               h.app,
+		attrSep:           h.attrSep,
 		opts:              h.opts,
-		attrs:             concat(h.attrs, as),
 		preformattedAttrs: h.preformattedAttrs,
 		w:                 h.w,
 	}
-	if h.opts.ReplaceAttr != nil {
-		for i, p := range h2.attrs[len(h.attrs):] {
-			h2.attrs[i] = h.opts.ReplaceAttr(p)
-		}
-	}
-
 	// Pre-format the attributes as an optimization.
-	app := h2.newAppender((*buffer.Buffer)(&h2.preformattedAttrs))
-	for _, p := range h2.attrs[len(h.attrs):] {
-		appendAttr(app, p)
+	state := handleState{
+		h2,
+		(*buffer.Buffer)(&h2.preformattedAttrs),
+		false,
+	}
+	for _, a := range as {
+		state.appendAttr(a)
 	}
 	return h2
 }
 
 func (h *commonHandler) handle(r Record) error {
-	buf := buffer.New()
-	defer buf.Free()
-	app := h.newAppender(buf)
 	rep := h.opts.ReplaceAttr
-	replace := func(a Attr) {
-		a = rep(a)
-		if a.Key() != "" {
-			app.appendKey(a.Key())
-			if err := app.appendAttrValue(a); err != nil {
-				app.appendString(fmt.Sprintf("!ERROR:%v", err))
-			}
-		}
-	}
-
-	app.appendStart()
+	state := handleState{h, buffer.New(), false}
+	defer state.buf.Free()
+	h.app.appendStart(state.buf)
+	// time
 	if !r.Time().IsZero() {
 		key := "time"
 		val := r.Time().Round(0) // strip monotonic to match Attr behavior
 		if rep == nil {
-			app.appendKey(key)
-			if err := app.appendTime(val); err != nil {
-				return err
-			}
+			state.appendKey(key)
+			state.appendTime(val)
 		} else {
-			replace(Time(key, val))
+			state.appendAttr(Time(key, val))
 		}
-		app.appendSep()
 	}
+	// level
 	if r.Level() != 0 {
 		key := "level"
 		val := r.Level()
 		if rep == nil {
-			app.appendKey(key)
-			app.appendString(val.String())
+			state.appendKey(key)
+			state.appendString(val.String())
 		} else {
-			replace(Any(key, val))
+			state.appendAttr(Any(key, val))
 		}
-		app.appendSep()
 	}
+	// source
 	if h.opts.AddSource {
 		file, line := r.SourceLine()
 		if file != "" {
 			key := "source"
 			if rep == nil {
-				app.appendKey(key)
-				app.appendSource(file, line)
+				state.appendKey(key)
+				h.app.appendSource(state.buf, file, line)
 			} else {
 				buf := buffer.New()
-				buf.WriteString(file)
+				buf.WriteString(file) // TODO: escape?
 				buf.WriteByte(':')
 				itoa((*[]byte)(buf), line, -1)
 				s := buf.String()
 				buf.Free()
-				replace(String(key, s))
+				state.appendAttr(String(key, s))
 			}
-			app.appendSep()
 		}
 	}
+	// message
 	key := "msg"
 	val := r.Message()
 	if rep == nil {
-		app.appendKey(key)
-		app.appendString(val)
+		state.appendKey(key)
+		state.appendString(val)
 	} else {
-		replace(String(key, val))
+		state.appendAttr(String(key, val))
 	}
-	*buf = append(*buf, h.preformattedAttrs...)
+	// preformatted Attrs
+	if len(h.preformattedAttrs) > 0 {
+		state.appendSep()
+		state.buf.Write(h.preformattedAttrs)
+	}
+	// Attrs in Record
 	r.Attrs(func(a Attr) {
-		if rep != nil {
-			a = rep(a)
-		}
-		appendAttr(app, a)
+		state.appendAttr(a)
 	})
-	app.appendEnd()
-	buf.WriteByte('\n')
+	h.app.appendEnd(state.buf)
+	state.buf.WriteByte('\n')
 
 	h.mu.Lock()
 	defer h.mu.Unlock()
-	_, err := h.w.Write(*buf)
+	_, err := h.w.Write(*state.buf)
 	return err
 }
 
-func appendAttr(app appender, a Attr) {
-	if a.Key() != "" {
-		app.appendSep()
-		app.appendKey(a.Key())
-		if err := app.appendAttrValue(a); err != nil {
-			app.appendString(fmt.Sprintf("!ERROR:%v", err))
-		}
+// handleState holds state for a single call to commonHandler.handle.
+// The initial value of sep determines whether to emit a separator
+// before the next key, after which it stays true.
+type handleState struct {
+	h   *commonHandler
+	buf *buffer.Buffer
+	sep bool // Append separator before next Attr?
+}
+
+// appendAttr appends the Attr's key and value using app.
+// If sep is true, it also prepends a separator.
+// It handles replacement and checking for an empty key.
+// It sets sep to true if it actually did the append (if the key was non-empty
+// after replacement).
+func (s *handleState) appendAttr(a Attr) {
+	if rep := s.h.opts.ReplaceAttr; rep != nil {
+		a = rep(a)
+	}
+	if a.Key() == "" {
+		return
+	}
+	s.appendKey(a.Key())
+	s.appendAttrValue(a)
+}
+
+func (s *handleState) appendError(err error) {
+	s.appendString(fmt.Sprintf("!ERROR:%v", err))
+}
+
+type appender interface {
+	appendStart(*buffer.Buffer)                 // start of output
+	appendEnd(*buffer.Buffer)                   // end of output
+	appendKey(*buffer.Buffer, string)           // append key and key-value separator
+	appendString(*buffer.Buffer, string)        // append a string
+	appendSource(*buffer.Buffer, string, int)   // append a filename and line
+	appendTime(*buffer.Buffer, time.Time) error // append a time
+	appendAttrValue(*buffer.Buffer, Attr) error // append Attr's value (but not key)
+}
+
+func (s *handleState) appendSep() {
+	if s.sep {
+		s.buf.WriteByte(s.h.attrSep)
 	}
 }
 
-// An appender appends keys and values to a buffer.
-// TextHandler and JSONHandler both implement it.
-// It factors out the format-specific parts of the job.
-// Other than growing the buffer, none of the methods should allocate.
-type appender interface {
-	appendStart()                       // start a sequence of Attrs
-	appendEnd()                         // end a sequence of Attrs
-	appendSep()                         // separate one Attr from the next
-	appendKey(key string)               // append a key
-	appendString(string)                // append a string that may need to be escaped
-	appendTime(time.Time) error         // append a time
-	appendSource(file string, line int) // append file:line
-	appendAttrValue(a Attr) error       // append the Attr's value (but not its key)
+func (s *handleState) appendKey(key string) {
+	s.appendSep()
+	s.h.app.appendKey(s.buf, key)
+	s.sep = true
+}
+
+func (s *handleState) appendString(str string) {
+	s.h.app.appendString(s.buf, str)
+}
+
+func (s *handleState) appendAttrValue(a Attr) {
+	if err := s.h.app.appendAttrValue(s.buf, a); err != nil {
+		s.appendError(err)
+	}
+}
+
+func (s *handleState) appendTime(t time.Time) {
+	if err := s.h.app.appendTime(s.buf, t); err != nil {
+		s.appendError(err)
+	}
 }
 
 // This takes half the time of Time.AppendFormat.
diff --git a/slog/handler_test.go b/slog/handler_test.go
index 9800ef7..f1ed37f 100644
--- a/slog/handler_test.go
+++ b/slog/handler_test.go
@@ -8,12 +8,9 @@
 
 import (
 	"bytes"
-	"fmt"
 	"strings"
 	"testing"
 	"time"
-
-	"golang.org/x/exp/slog/internal/buffer"
 )
 
 func TestDefaultWith(t *testing.T) {
@@ -33,11 +30,22 @@
 	}
 }
 
+// NOTE TO REVIEWER: Ignore this test. The next CL revamps it.
 func TestCommonHandle(t *testing.T) {
 	tm := time.Date(2022, 9, 18, 8, 26, 33, 0, time.UTC)
 	r := NewRecord(tm, InfoLevel, "message", 1)
 	r.AddAttrs(String("a", "one"), Int("b", 2), Any("", "ignore me"))
 
+	newHandler := func(replace func(Attr) Attr) *commonHandler {
+		return &commonHandler{
+			app:     textAppender{},
+			attrSep: ' ',
+			opts:    HandlerOptions{ReplaceAttr: replace},
+		}
+	}
+
+	removeAttr := func(a Attr) Attr { return Attr{} }
+
 	for _, test := range []struct {
 		name string
 		h    *commonHandler
@@ -45,33 +53,38 @@
 	}{
 		{
 			name: "basic",
-			h:    &commonHandler{},
-			want: "(time=2022-09-18T08:26:33.000Z;level=INFO;msg=message;a=one;b=2)",
+			h:    newHandler(nil),
+			want: "time=2022-09-18T08:26:33.000Z level=INFO msg=message a=one b=2",
 		},
 		{
 			name: "cap keys",
-			h: &commonHandler{
-				opts: HandlerOptions{ReplaceAttr: upperCaseKey},
-			},
-			want: "(TIME=2022-09-18T08:26:33.000Z;LEVEL=INFO;MSG=message;A=one;B=2)",
+			h:    newHandler(upperCaseKey),
+			want: "TIME=2022-09-18T08:26:33.000Z LEVEL=INFO MSG=message A=one B=2",
 		},
 		{
 			name: "remove all",
-			h: &commonHandler{
-				opts: HandlerOptions{
-					ReplaceAttr: func(a Attr) Attr { return Attr{} },
-				},
-			},
-			// TODO: fix this. The correct result is "()".
-			want: "(;;)",
+			h:    newHandler(removeAttr),
+			want: "",
+		},
+		{
+			name: "preformatted",
+			h:    newHandler(nil).with([]Attr{Int("pre", 3), String("x", "y")}),
+			want: "time=2022-09-18T08:26:33.000Z level=INFO msg=message pre=3 x=y a=one b=2",
+		},
+		{
+			name: "preformatted cap keys",
+			h:    newHandler(upperCaseKey).with([]Attr{Int("pre", 3), String("x", "y")}),
+			want: "TIME=2022-09-18T08:26:33.000Z LEVEL=INFO MSG=message PRE=3 X=y A=one B=2",
+		},
+		{
+			name: "preformatted remove all",
+			h:    newHandler(removeAttr).with([]Attr{Int("pre", 3), String("x", "y")}),
+			want: "",
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			var buf bytes.Buffer
 			test.h.w = &buf
-			test.h.newAppender = func(buf *buffer.Buffer) appender {
-				return &testAppender{buf}
-			}
 			if err := test.h.handle(r); err != nil {
 				t.Fatal(err)
 			}
@@ -87,41 +100,6 @@
 	return a.WithKey(strings.ToUpper(a.Key()))
 }
 
-type testAppender struct {
-	buf *buffer.Buffer
-}
-
-func (a *testAppender) appendStart() { a.buf.WriteByte('(') }
-func (a *testAppender) appendSep()   { a.buf.WriteByte(';') }
-func (a *testAppender) appendEnd()   { a.buf.WriteByte(')') }
-
-func (a *testAppender) appendKey(key string) {
-	a.appendString(key)
-	a.buf.WriteByte('=')
-}
-func (a *testAppender) appendString(s string) { a.buf.WriteString(s) }
-
-func (a *testAppender) appendTime(t time.Time) error {
-	*a.buf = appendTimeRFC3339Millis(*a.buf, t)
-	return nil
-}
-
-func (a *testAppender) appendSource(file string, line int) {
-	a.appendString(fmt.Sprintf("%s:%d", file, line))
-}
-
-func (a *testAppender) appendAttrValue(at Attr) error {
-	switch at.Kind() {
-	case StringKind:
-		a.appendString(at.str())
-	case TimeKind:
-		a.appendTime(at.Time())
-	default:
-		*a.buf = at.appendValue(*a.buf)
-	}
-	return nil
-}
-
 const rfc3339Millis = "2006-01-02T15:04:05.000Z07:00"
 
 func TestAppendTimeRFC3339(t *testing.T) {
diff --git a/slog/json_handler.go b/slog/json_handler.go
index b473011..844b6a3 100644
--- a/slog/json_handler.go
+++ b/slog/json_handler.go
@@ -32,17 +32,14 @@
 func (opts HandlerOptions) NewJSONHandler(w io.Writer) *JSONHandler {
 	return &JSONHandler{
 		&commonHandler{
-			newAppender: newJSONAppender,
-			w:           w,
-			opts:        opts,
+			app:     jsonAppender{},
+			attrSep: ',',
+			w:       w,
+			opts:    opts,
 		},
 	}
 }
 
-func newJSONAppender(buf *buffer.Buffer) appender {
-	return (*jsonAppender)(buf)
-}
-
 // With returns a new JSONHandler whose attributes consists
 // of h's attributes followed by attrs.
 func (h *JSONHandler) With(attrs []Attr) Handler {
@@ -53,7 +50,7 @@
 //
 // If the Record's time is zero, the time is omitted.
 // Otherwise, the key is "time"
-// and the value is output in RFC3339 format with millisecond precision.
+// and the value is output as with json.Marshal.
 //
 // If the Record's level is zero, the level is omitted.
 // Otherwise, the key is "level"
@@ -79,77 +76,74 @@
 	return h.commonHandler.handle(r)
 }
 
-type jsonAppender buffer.Buffer
+type jsonAppender struct{}
 
-func (a *jsonAppender) buf() *buffer.Buffer { return (*buffer.Buffer)(a) }
+func (jsonAppender) appendStart(buf *buffer.Buffer) { buf.WriteByte('{') }
+func (jsonAppender) appendEnd(buf *buffer.Buffer)   { buf.WriteByte('}') }
 
-func (a *jsonAppender) appendKey(key string) {
-	a.appendString(key)
-	a.buf().WriteByte(':')
+func (a jsonAppender) appendKey(buf *buffer.Buffer, key string) {
+	a.appendString(buf, key)
+	buf.WriteByte(':')
 }
 
-func (a *jsonAppender) appendString(s string) {
-	*a.buf() = appendQuotedJSONString(*a.buf(), s)
+func (jsonAppender) appendString(buf *buffer.Buffer, s string) {
+	*buf = appendQuotedJSONString(*buf, s)
 }
 
-func (a *jsonAppender) appendStart() { a.buf().WriteByte('{') }
-func (a *jsonAppender) appendEnd()   { a.buf().WriteByte('}') }
-func (a *jsonAppender) appendSep()   { a.buf().WriteByte(',') }
+func (jsonAppender) appendSource(buf *buffer.Buffer, file string, line int) {
+	buf.WriteByte('"')
+	*buf = appendJSONString(*buf, file)
+	buf.WriteByte(':')
+	itoa((*[]byte)(buf), line, -1)
+	buf.WriteByte('"')
+}
 
-func (a *jsonAppender) appendTime(t time.Time) error {
+func (jsonAppender) appendTime(buf *buffer.Buffer, t time.Time) error {
 	b, err := t.MarshalJSON()
 	if err != nil {
 		return err
 	}
-	a.buf().Write(b)
+	buf.Write(b)
 	return nil
 }
 
-func (a *jsonAppender) appendSource(file string, line int) {
-	a.buf().WriteByte('"')
-	*a.buf() = appendJSONString(*a.buf(), file)
-	a.buf().WriteByte(':')
-	itoa((*[]byte)(a), line, -1)
-	a.buf().WriteByte('"')
-}
-
-func (ap *jsonAppender) appendAttrValue(a Attr) error {
+func (app jsonAppender) appendAttrValue(buf *buffer.Buffer, a Attr) error {
 	switch a.Kind() {
 	case StringKind:
-		ap.appendString(a.str())
+		app.appendString(buf, a.str())
 	case Int64Kind:
-		*ap.buf() = strconv.AppendInt(*ap.buf(), a.Int64(), 10)
+		*buf = strconv.AppendInt(*buf, a.Int64(), 10)
 	case Uint64Kind:
-		*ap.buf() = strconv.AppendUint(*ap.buf(), a.Uint64(), 10)
+		*buf = strconv.AppendUint(*buf, a.Uint64(), 10)
 	case Float64Kind:
 		f := a.Float64()
 		// json.Marshal fails on special floats, so handle them here.
 		switch {
 		case math.IsInf(f, 1):
-			ap.buf().WriteString(`"+Inf"`)
+			buf.WriteString(`"+Inf"`)
 		case math.IsInf(f, -1):
-			ap.buf().WriteString(`"-Inf"`)
+			buf.WriteString(`"-Inf"`)
 		case math.IsNaN(f):
-			ap.buf().WriteString(`"NaN"`)
+			buf.WriteString(`"NaN"`)
 		default:
 			// json.Marshal is funny about floats; it doesn't
 			// always match strconv.AppendFloat. So just call it.
 			// That's expensive, but floats are rare.
-			if err := ap.appendJSONMarshal(f); err != nil {
+			if err := appendJSONMarshal(buf, f); err != nil {
 				return err
 			}
 		}
 	case BoolKind:
-		*ap.buf() = strconv.AppendBool(*ap.buf(), a.Bool())
+		*buf = strconv.AppendBool(*buf, a.Bool())
 	case DurationKind:
 		// Do what json.Marshal does.
-		*ap.buf() = strconv.AppendInt(*ap.buf(), int64(a.Duration()), 10)
+		*buf = strconv.AppendInt(*buf, int64(a.Duration()), 10)
 	case TimeKind:
-		if err := ap.appendTime(a.Time()); err != nil {
+		if err := app.appendTime(buf, a.Time()); err != nil {
 			return err
 		}
 	case AnyKind:
-		if err := ap.appendJSONMarshal(a.Value()); err != nil {
+		if err := appendJSONMarshal(buf, a.Value()); err != nil {
 			return err
 		}
 	default:
@@ -158,12 +152,12 @@
 	return nil
 }
 
-func (a *jsonAppender) appendJSONMarshal(v any) error {
+func appendJSONMarshal(buf *buffer.Buffer, v any) error {
 	b, err := json.Marshal(v)
 	if err != nil {
 		return err
 	}
-	a.buf().Write(b)
+	buf.Write(b)
 	return nil
 }
 
diff --git a/slog/json_handler_test.go b/slog/json_handler_test.go
index 2d47b66..5785d86 100644
--- a/slog/json_handler_test.go
+++ b/slog/json_handler_test.go
@@ -54,8 +54,7 @@
 
 func TestJSONAppendSource(t *testing.T) {
 	var buf []byte
-	app := newJSONAppender((*buffer.Buffer)(&buf))
-	app.appendSource("file.go", 23)
+	(jsonAppender{}).appendSource((*buffer.Buffer)(&buf), "file.go", 23)
 	got := string(buf)
 	want := `"file.go:23"`
 	if got != want {
@@ -78,9 +77,7 @@
 }
 
 func TestJSONAppendAttrValue(t *testing.T) {
-	var buf []byte
-	app := newJSONAppender((*buffer.Buffer)(&buf))
-	// On most values, appendAttrValue should agree with json.Marshal.
+	// On most values, jsonAppendAttrValue should agree with json.Marshal.
 	for _, value := range []any{
 		"hello",
 		`"[{escape}]"`,
@@ -95,12 +92,12 @@
 		testTime,
 		jsonMarshaler{"xyz"},
 	} {
+		var buf []byte
 		attr := Any("", value)
-		if err := app.appendAttrValue(attr); err != nil {
+		if err := (jsonAppender{}).appendAttrValue((*buffer.Buffer)(&buf), attr); err != nil {
 			t.Fatal(err)
 		}
 		got := string(buf)
-		buf = nil
 		b, err := json.Marshal(value)
 		if err != nil {
 			t.Fatal(err)
@@ -114,8 +111,6 @@
 
 func TestJSONAppendAttrValueSpecial(t *testing.T) {
 	// Attr values that render differently from json.Marshal.
-	var buf []byte
-	app := newJSONAppender((*buffer.Buffer)(&buf))
 	for _, test := range []struct {
 		value any
 		want  string
@@ -125,12 +120,13 @@
 		{math.Inf(-1), `"-Inf"`},
 		{WarnLevel, `"WARN"`},
 	} {
+		var buf []byte
+
 		attr := Any("", test.value)
-		if err := app.appendAttrValue(attr); err != nil {
+		if err := (jsonAppender{}).appendAttrValue((*buffer.Buffer)(&buf), attr); err != nil {
 			t.Fatal(err)
 		}
 		got := string(buf)
-		buf = nil
 		if got != test.want {
 			t.Errorf("%v: got %s, want %s", test.value, got, test.want)
 		}
diff --git a/slog/text_handler.go b/slog/text_handler.go
index b67a650..de18fec 100644
--- a/slog/text_handler.go
+++ b/slog/text_handler.go
@@ -32,11 +32,10 @@
 func (opts HandlerOptions) NewTextHandler(w io.Writer) *TextHandler {
 	return &TextHandler{
 		&commonHandler{
-			newAppender: func(buf *buffer.Buffer) appender {
-				return (*textAppender)(buf)
-			},
-			w:    w,
-			opts: opts,
+			app:     textAppender{},
+			attrSep: ' ',
+			w:       w,
+			opts:    opts,
 		},
 	}
 }
@@ -78,65 +77,59 @@
 	return h.commonHandler.handle(r)
 }
 
-type textAppender buffer.Buffer
+type textAppender struct{}
 
-func (a *textAppender) buf() *buffer.Buffer { return (*buffer.Buffer)(a) }
+func (textAppender) appendStart(*buffer.Buffer) {}
 
-func (a *textAppender) appendKey(key string) {
-	a.appendString(key)
-	a.buf().WriteByte('=')
+func (textAppender) appendEnd(*buffer.Buffer) {}
+
+func (a textAppender) appendKey(buf *buffer.Buffer, key string) {
+	a.appendString(buf, key)
+	buf.WriteByte('=')
 }
 
-func (a *textAppender) appendString(s string) {
+func (textAppender) appendString(buf *buffer.Buffer, s string) {
 	if needsQuoting(s) {
-		*a.buf() = strconv.AppendQuote(*a.buf(), s)
+		*buf = strconv.AppendQuote(*buf, s)
 	} else {
-		a.buf().WriteString(s)
+		buf.WriteString(s)
 	}
 }
 
-func (a *textAppender) appendStart() {}
-func (a *textAppender) appendEnd()   {}
-func (a *textAppender) appendSep()   { a.buf().WriteByte(' ') }
-
-func (a *textAppender) appendTime(t time.Time) error {
-	*a.buf() = appendTimeRFC3339Millis(*a.buf(), t)
+func (textAppender) appendTime(buf *buffer.Buffer, t time.Time) error {
+	*buf = appendTimeRFC3339Millis(*buf, t)
 	return nil
 }
 
-func (a *textAppender) appendSource(file string, line int) {
+func (a textAppender) appendSource(buf *buffer.Buffer, file string, line int) {
 	if needsQuoting(file) {
-		a.appendString(file + ":" + strconv.Itoa(line))
+		a.appendString(buf, file+":"+strconv.Itoa(line))
 	} else {
 		// common case: no quoting needed.
-		a.appendString(file)
-		a.buf().WriteByte(':')
-		itoa((*[]byte)(a.buf()), line, -1)
+		a.appendString(buf, file)
+		buf.WriteByte(':')
+		itoa((*[]byte)(buf), line, -1)
 	}
 }
-
-func (ap *textAppender) appendAttrValue(a Attr) error {
+func (app textAppender) appendAttrValue(buf *buffer.Buffer, a Attr) error {
 	switch a.Kind() {
 	case StringKind:
-		ap.appendString(a.str())
+		app.appendString(buf, a.str())
 	case TimeKind:
-		if err := ap.appendTime(a.Time()); err != nil {
-			return err
-		}
+		_ = app.appendTime(buf, a.Time())
 	case AnyKind:
 		if tm, ok := a.any.(encoding.TextMarshaler); ok {
 			data, err := tm.MarshalText()
 			if err != nil {
-				ap.appendString("!ERROR:" + err.Error())
-			} else {
-				// TODO: avoid the conversion to string.
-				ap.appendString(string(data))
+				return err
 			}
+			// TODO: avoid the conversion to string.
+			app.appendString(buf, string(data))
 			return nil
 		}
-		ap.appendString(fmt.Sprint(a.Value()))
+		app.appendString(buf, fmt.Sprint(a.Value()))
 	default:
-		*ap.buf() = a.appendValue(*ap.buf())
+		*buf = a.appendValue(*buf)
 	}
 	return nil
 }
diff --git a/slog/text_handler_test.go b/slog/text_handler_test.go
index f9f4f30..7836393 100644
--- a/slog/text_handler_test.go
+++ b/slog/text_handler_test.go
@@ -114,8 +114,6 @@
 }
 
 func TestTextAppendSource(t *testing.T) {
-	var buf []byte
-	app := (*textAppender)((*buffer.Buffer)(&buf))
 	for _, test := range []struct {
 		file string
 		want string
@@ -124,12 +122,13 @@
 		{"a b.go", `"a b.go:1"`},
 		{`C:\windows\b.go`, `C:\windows\b.go:1`},
 	} {
-		app.appendSource(test.file, 1)
+		var buf []byte
+		(textAppender{}).appendSource((*buffer.Buffer)(&buf), test.file, 1)
 		got := string(buf)
 		if got != test.want {
 			t.Errorf("%s:\ngot  %s\nwant %s", test.file, got, test.want)
 		}
-		buf = buf[:0]
+
 	}
 }