From 4be54154e29f7ad804d0fd36dc71ede8ab808999 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 12 May 2025 12:03:11 +0100 Subject: [PATCH 1/7] fix: `%x` formatting of `ids.ID` and `ShortID` --- ids/format.go | 40 ++++++++++++++++++++++++++++++++ ids/format_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 ids/format.go create mode 100644 ids/format_test.go diff --git a/ids/format.go b/ids/format.go new file mode 100644 index 000000000000..38a30bf9632f --- /dev/null +++ b/ids/format.go @@ -0,0 +1,40 @@ +package ids + +import "fmt" + +var _ = []fmt.Formatter{ID{}, ShortID{}} + +// Format implements the [fmt.Formatter] interface. +func (id ID) Format(s fmt.State, verb rune) { + format(s, verb, id) +} + +// Format implements the [fmt.Formatter] interface. +func (id ShortID) Format(s fmt.State, verb rune) { + format(s, verb, id) +} + +// format implements the [fmt.Formatter] interface for [ID] and [ShortID]. +func format[T interface { + String() string + Hex() string +}](s fmt.State, verb rune, id T) { + switch verb { + case 'x': + if s.Flag('#') { + s.Write([]byte("0x")) + } + s.Write([]byte(id.Hex())) + + case 'q': + str := id.String() + buf := make([]byte, len(str)+2) + buf[0] = '"' + buf[len(buf)-1] = '"' + copy(buf[1:], str) + s.Write(buf) + + default: + s.Write([]byte(id.String())) + } +} diff --git a/ids/format_test.go b/ids/format_test.go new file mode 100644 index 000000000000..ab2f23429bd8 --- /dev/null +++ b/ids/format_test.go @@ -0,0 +1,57 @@ +package ids + +import ( + "fmt" + "testing" +) + +func TestFormat(t *testing.T) { + type ( + idInterface interface { + String() string + Hex() string + } + test struct { + id idInterface + want map[string]string // format -> output + } + ) + makeTestCase := func(id idInterface) test { + return test{ + id: id, + want: map[string]string{ + "%v": id.String(), + "%s": id.String(), + "%q": `"` + id.String() + `"`, + "%x": id.Hex(), + "%#x": `0x` + id.Hex(), + }, + } + } + + tests := []test{ + makeTestCase(ID{}), + makeTestCase(GenerateTestID()), + makeTestCase(GenerateTestID()), + makeTestCase(ShortID{}), + makeTestCase(GenerateTestShortID()), + makeTestCase(GenerateTestShortID()), + } + + for _, tt := range tests { + t.Run(tt.id.String(), func(t *testing.T) { + for format, want := range tt.want { + if got := fmt.Sprintf(format, tt.id); got != want { + t.Errorf("fmt.Sprintf(%q, %T) got %q; want %q", format, tt.id, got, want) + } + } + }) + } +} + +func BenchmarkFormat(b *testing.B) { + // %q uses a []byte so this is just to demonstrate that it's on the stack + // otherwise someone, not naming any names, might want to "fix" it. + _ = fmt.Sprintf("%q", ID{}) + _ = fmt.Sprintf("%q", ShortID{}) +} From 7a734ad036a1d48e7fdba6a1ae6acdaa2d8e2d11 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 12 May 2025 12:40:25 +0100 Subject: [PATCH 2/7] chore: placate the linter --- ids/format.go | 8 ++++---- ids/format_test.go | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ids/format.go b/ids/format.go index 38a30bf9632f..5253915927b1 100644 --- a/ids/format.go +++ b/ids/format.go @@ -22,9 +22,9 @@ func format[T interface { switch verb { case 'x': if s.Flag('#') { - s.Write([]byte("0x")) + s.Write([]byte("0x")) //nolint:errcheck // [fmt.Formatter] doesn't allow for returning errors, and the implementation of [fmt.State] always returns nil on Write() } - s.Write([]byte(id.Hex())) + s.Write([]byte(id.Hex())) //nolint:errcheck // See above case 'q': str := id.String() @@ -32,9 +32,9 @@ func format[T interface { buf[0] = '"' buf[len(buf)-1] = '"' copy(buf[1:], str) - s.Write(buf) + s.Write(buf) //nolint:errcheck // See above default: - s.Write([]byte(id.String())) + s.Write([]byte(id.String())) //nolint:errcheck // See above } } diff --git a/ids/format_test.go b/ids/format_test.go index ab2f23429bd8..f4be27a73d5f 100644 --- a/ids/format_test.go +++ b/ids/format_test.go @@ -42,6 +42,7 @@ func TestFormat(t *testing.T) { t.Run(tt.id.String(), func(t *testing.T) { for format, want := range tt.want { if got := fmt.Sprintf(format, tt.id); got != want { + //nolint:forbidigo // require.Equal() is inappropriate as it unnecessarily stops future tests and `assert.Equal()` isn't allowed t.Errorf("fmt.Sprintf(%q, %T) got %q; want %q", format, tt.id, got, want) } } @@ -49,7 +50,7 @@ func TestFormat(t *testing.T) { } } -func BenchmarkFormat(b *testing.B) { +func BenchmarkFormat(*testing.B) { // %q uses a []byte so this is just to demonstrate that it's on the stack // otherwise someone, not naming any names, might want to "fix" it. _ = fmt.Sprintf("%q", ID{}) From 0d49c500b8d4fedc056b9fb1beac916f06caa0ae Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 12 May 2025 12:43:28 +0100 Subject: [PATCH 3/7] chore: copyright headers --- ids/format.go | 3 +++ ids/format_test.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/ids/format.go b/ids/format.go index 5253915927b1..c0ed26dee2cb 100644 --- a/ids/format.go +++ b/ids/format.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package ids import "fmt" diff --git a/ids/format_test.go b/ids/format_test.go index f4be27a73d5f..58b2eb51c3cb 100644 --- a/ids/format_test.go +++ b/ids/format_test.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package ids import ( From 70de15a75ed70ffd0ed3e0e9f55dd58d835817dd Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 12 May 2025 13:31:27 +0100 Subject: [PATCH 4/7] chore: use currently required header --- ids/format.go | 2 +- ids/format_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ids/format.go b/ids/format.go index c0ed26dee2cb..0225be576477 100644 --- a/ids/format.go +++ b/ids/format.go @@ -1,4 +1,4 @@ -// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. package ids diff --git a/ids/format_test.go b/ids/format_test.go index 58b2eb51c3cb..301f77739f76 100644 --- a/ids/format_test.go +++ b/ids/format_test.go @@ -1,4 +1,4 @@ -// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. package ids From e6a7c333f223df2a77217687ca126a5922845c5d Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 12 May 2025 17:42:42 +0100 Subject: [PATCH 5/7] refactor: only define `idForFormatting` interface once --- ids/format.go | 9 +++++++-- ids/format_test.go | 16 +++++----------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ids/format.go b/ids/format.go index 0225be576477..5346167744a6 100644 --- a/ids/format.go +++ b/ids/format.go @@ -17,10 +17,15 @@ func (id ShortID) Format(s fmt.State, verb rune) { format(s, verb, id) } -// format implements the [fmt.Formatter] interface for [ID] and [ShortID]. -func format[T interface { +type idForFormatting interface { String() string Hex() string +} + +// format implements the [fmt.Formatter] interface for [ID] and [ShortID]. +func format[T interface { + idForFormatting + ID | ShortID }](s fmt.State, verb rune, id T) { switch verb { case 'x': diff --git a/ids/format_test.go b/ids/format_test.go index 301f77739f76..3615a4e34a5d 100644 --- a/ids/format_test.go +++ b/ids/format_test.go @@ -9,17 +9,11 @@ import ( ) func TestFormat(t *testing.T) { - type ( - idInterface interface { - String() string - Hex() string - } - test struct { - id idInterface - want map[string]string // format -> output - } - ) - makeTestCase := func(id idInterface) test { + type test struct { + id idForFormatting + want map[string]string // format -> output + } + makeTestCase := func(id idForFormatting) test { return test{ id: id, want: map[string]string{ From 6489323ccb6f5c1c40b565ea30ecddc4ce89fec5 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 12 May 2025 18:07:52 +0100 Subject: [PATCH 6/7] test: use `require` --- ids/format_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ids/format_test.go b/ids/format_test.go index 3615a4e34a5d..1038ceb342a4 100644 --- a/ids/format_test.go +++ b/ids/format_test.go @@ -6,6 +6,8 @@ package ids import ( "fmt" "testing" + + "github.com/stretchr/testify/require" ) func TestFormat(t *testing.T) { @@ -38,10 +40,7 @@ func TestFormat(t *testing.T) { for _, tt := range tests { t.Run(tt.id.String(), func(t *testing.T) { for format, want := range tt.want { - if got := fmt.Sprintf(format, tt.id); got != want { - //nolint:forbidigo // require.Equal() is inappropriate as it unnecessarily stops future tests and `assert.Equal()` isn't allowed - t.Errorf("fmt.Sprintf(%q, %T) got %q; want %q", format, tt.id, got, want) - } + require.Equalf(t, want, fmt.Sprintf(format, tt.id), "fmt.Sprintf(%q, %T)", format, tt.id) } }) } From 5e841fa7042956cdb957ffb289d2e9b043bd7fac Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 12 May 2025 18:16:58 +0100 Subject: [PATCH 7/7] chore: remove benchmark --- ids/format_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ids/format_test.go b/ids/format_test.go index 1038ceb342a4..07837f6b9442 100644 --- a/ids/format_test.go +++ b/ids/format_test.go @@ -45,10 +45,3 @@ func TestFormat(t *testing.T) { }) } } - -func BenchmarkFormat(*testing.B) { - // %q uses a []byte so this is just to demonstrate that it's on the stack - // otherwise someone, not naming any names, might want to "fix" it. - _ = fmt.Sprintf("%q", ID{}) - _ = fmt.Sprintf("%q", ShortID{}) -}