slog: TextHandler.appendSource: quote entire value, fix test

If the filename value of the source attribute needs quoting, quote the entire "file:line". Previously we were just quoting the filename. Also, fix the test for source to accept quoted values. Change-Id: Ief560123633983ba6ab8679c52e8605714d9f193 Reviewed-on: https://go-review.googlesource.com/c/exp/+/430076 Run-TryBot: Jonathan Amsterdam <[email protected]> Reviewed-by: Bryan Mills <[email protected]> Reviewed-by: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
golang · Sep 15, 2022 · 82e28ad · 82e28ad
1 parent 0d63173
commit 82e28ad
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 10 deletions.
diff --git a/slog/text_handler.go b/slog/text_handler.go
@@ -105,9 +105,14 @@ func (a *textAppender) appendTime(t time.Time) error {
 }
 
 func (a *textAppender) appendSource(file string, line int) {
-	a.appendString(file)
-	a.buf().WriteByte(':')
-	itoa((*[]byte)(a.buf()), line, -1)
+	if needsQuoting(file) {
+		a.appendString(file + ":" + strconv.Itoa(line))
+	} else {
+		// common case: no quoting needed.
+		a.appendString(file)
+		a.buf().WriteByte(':')
+		itoa((*[]byte)(a.buf()), line, -1)
+	}
 }
 
 func (ap *textAppender) appendAttrValue(a Attr) error {

diff --git a/slog/text_handler_test.go b/slog/text_handler_test.go
@@ -13,6 +13,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"golang.org/x/exp/slog/internal/buffer"
 )
 
 var testTime = time.Date(2000, 1, 2, 3, 4, 5, 0, time.UTC)
@@ -89,21 +91,49 @@ func (t text) MarshalText() ([]byte, error) {
 	return []byte(fmt.Sprintf("text{%q}", t.s)), nil
 }
 
+func TestTextAppendSource(t *testing.T) {
+	var buf []byte
+	app := (*textAppender)((*buffer.Buffer)(&buf))
+	for _, test := range []struct {
+		file string
+		want string
+	}{
+		{"a/b.go", "a/b.go:1"},
+		{"a b.go", `"a b.go:1"`},
+		{`C:\windows\b.go`, `C:\windows\b.go:1`},
+	} {
+		app.appendSource(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]
+	}
+}
+
 func TestTextHandlerSource(t *testing.T) {
 	var buf bytes.Buffer
 	h := HandlerOptions{AddSource: true}.NewTextHandler(&buf)
 	r := MakeRecord(testTime, InfoLevel, "m", 2)
 	if err := h.Handle(r); err != nil {
 		t.Fatal(err)
 	}
-	got := buf.String()
-	wantRE := `source=([A-Z]:)?[^:]+text_handler_test\.go:\d+ msg`
-	matched, err := regexp.MatchString(wantRE, got)
-	if err != nil {
-		t.Fatal(err)
+	if got := buf.String(); !sourceRegexp.MatchString(got) {
+		t.Errorf("got\n%q\nwanted to match %s", got, sourceRegexp)
 	}
-	if !matched {
-		t.Errorf("got\n%q\nwanted to match %s", got, wantRE)
+}
+
+var sourceRegexp = regexp.MustCompile(`source="?([A-Z]:)?[^:]+text_handler_test\.go:\d+"? msg`)
+
+func TestSourceRegexp(t *testing.T) {
+	for _, s := range []string{
+		`source=/tmp/path/to/text_handler_test.go:23 msg=m`,
+		`source=C:\windows\path\text_handler_test.go:23 msg=m"`,
+		`source="/tmp/tmp.XcGZ9cG9Xb/with spaces/exp/slog/text_handler_test.go:95" msg=m`,
+	} {
+		if !sourceRegexp.MatchString(s) {
+			t.Errorf("failed to match %s", s)
+		}
 	}
 }