slog: TextHandler checks long strings, quotes non-printing runes

TextHandler checks the full length of all strings for whether
they need to be quoted, because the cost of doing so (when done
quickly) is significantly less than the cost of quoting.

Also, it quotes strings containing non-printing characters.

Change-Id: I2bb3a66f4fc1f60b0fc58acc53a2f5b973522cc7
Reviewed-on: https://go-review.googlesource.com/c/exp/+/429838
Reviewed-by: Alan Donovan <[email protected]>
Run-TryBot: Jonathan Amsterdam <[email protected]>

Author: jba

slog/text_handler.go

@@ -9,9 +9,9 @@ import (
 	"fmt"
 	"io"
 	"strconv"
-	"strings"
 	"time"
 	"unicode"
+	"unicode/utf8"
 
 	"golang.org/x/exp/slog/internal/buffer"
 )
@@ -69,8 +69,8 @@ func (h *TextHandler) With(attrs []Attr) Handler {
 // If a value implements [encoding.TextMarshaler], the result of MarshalText is
 // written. Otherwise, the result of fmt.Sprint is written.
 //
-// Keys and values are quoted if they are long or contain Unicode space
-// characters, '"' or '='.
+// Keys and values are quoted if they contain Unicode space characters,
+// non-printing characters, '"' or '='.
 //
 // Each call to Handle results in a single serialized call to
 // io.Writer.Write.
@@ -95,19 +95,10 @@ func (a *textAppender) appendString(s string) {
 	}
 }
 
-func needsQuoting(s string) bool {
-	return len(s) > maxCheckQuoteSize ||
-		strings.IndexFunc(s, func(r rune) bool {
-			return unicode.IsSpace(r) || r == '"' || r == '='
-		}) >= 0
-}
-
 func (a *textAppender) appendStart() {}
 func (a *textAppender) appendEnd()   {}
 func (a *textAppender) appendSep()   { a.buf().WriteByte(' ') }
 
-const maxCheckQuoteSize = 80
-
 func (a *textAppender) appendTime(t time.Time) error {
 	*a.buf() = appendTimeRFC3339Millis(*a.buf(), t)
 	return nil
@@ -144,3 +135,36 @@ func (ap *textAppender) appendAttrValue(a Attr) error {
 	}
 	return nil
 }
+
+func needsQuoting(s string) bool {
+	for i := 0; i < len(s); {
+		b := s[i]
+		if b < utf8.RuneSelf {
+			if needsQuotingSet[b] {
+				return true
+			}
+			i++
+			continue
+		}
+		r, size := utf8.DecodeRuneInString(s[i:])
+		if r == utf8.RuneError || unicode.IsSpace(r) || !unicode.IsPrint(r) {
+			return true
+		}
+		i += size
+	}
+	return false
+}
+
+var needsQuotingSet = [utf8.RuneSelf]bool{
+	'"': true,
+	'=': true,
+}
+
+func init() {
+	for i := 0; i < utf8.RuneSelf; i++ {
+		r := rune(i)
+		if unicode.IsSpace(r) || !unicode.IsPrint(r) {
+			needsQuotingSet[i] = true
+		}
+	}
+}

slog/text_handler_test.go

@@ -132,3 +132,23 @@ func TestTextHandlerAlloc(t *testing.T) {
 	h := NewTextHandler(io.Discard)
 	wantAllocs(t, 0, func() { h.Handle(r) })
 }
+
+func TestNeedsQuoting(t *testing.T) {
+	for _, test := range []struct {
+		in   string
+		want bool
+	}{
+		{"", false},
+		{"ab", false},
+		{"a=b", true},
+		{`"ab"`, true},
+		{"\a\b", true},
+		{"a\tb", true},
+		{"µåπ", false},
+	} {
+		got := needsQuoting(test.in)
+		if got != test.want {
+			t.Errorf("%q: got %t, want %t", test.in, got, test.want)
+		}
+	}
+}