From 7ecf331bd60f7219261782d2c1859621d5ee6e99 Mon Sep 17 00:00:00 2001 From: Nik <73077675+tmzane@users.noreply.github.com> Date: Tue, 10 Jun 2025 22:08:59 +0300 Subject: [PATCH] feat(builder): don't panic (like fmt) --- builder.go | 28 ++++------------------------ builder_test.go | 30 +++++++++++++++--------------- 2 files changed, 19 insertions(+), 39 deletions(-) diff --git a/builder.go b/builder.go index 6db2bae..55fe895 100644 --- a/builder.go +++ b/builder.go @@ -41,20 +41,7 @@ func (b *Builder) Appendf(format string, args ...any) { } // Query returns the query string. -// If the query is invalid, e.g. too few/many arguments are given or different placeholders are used, Query panics. -func (b *Builder) Query() string { - query := b.query.String() - if strings.Contains(query, "%!") { - // fmt silently recovers panics and writes them to the output. - // We want panics to be loud, so we find and rethrow them. - // See also https://github.com/golang/go/issues/28150. - panic(fmt.Sprintf("queries: bad query: %s", query)) - } - if b.placeholder == -1 { - panic("queries: different placeholders used") - } - return query -} +func (b *Builder) Query() string { return b.query.String() } // Args returns the query arguments. func (b *Builder) Args() []any { return b.args } @@ -72,7 +59,7 @@ func (a argument) Format(s fmt.State, verb rune) { a.builder.placeholder = verb } if a.builder.placeholder != verb { - a.builder.placeholder = -1 + panic("unexpected placeholder") } default: format := fmt.FormatString(s, verb) @@ -104,29 +91,22 @@ func (a argument) writePlaceholder(w io.Writer, verb rune) { func (a argument) writeSlice(w io.Writer, verb rune) { slice := reflect.ValueOf(a.value) if slice.Kind() != reflect.Slice { - panic("queries: %+ argument must be a slice") + panic("non-slice argument") } if slice.Len() == 0 { // TODO: revisit. - // Unlike other errors produced by Builder, - // which are all the result of a programmer's mistake, - // this one may be caused by user input, so panicking is not an option here. // "WHERE IN (NULL)" will always result in an empty result set, // which may be undesirable in some situations. fmt.Fprint(w, "NULL") return } - args := reflect.ValueOf(a.builder.args) - for i := range slice.Len() { if i > 0 { fmt.Fprint(w, ", ") } a.writePlaceholder(w, verb) - args = reflect.Append(args, slice.Index(i)) + a.builder.args = append(a.builder.args, slice.Index(i).Interface()) } - - a.builder.args = args.Interface().([]any) } diff --git a/builder_test.go b/builder_test.go index 85e8366..aaf623a 100644 --- a/builder_test.go +++ b/builder_test.go @@ -40,11 +40,11 @@ func TestBuilder_dialects(t *testing.T) { }, } - for name, tt := range tests { + for name, test := range tests { t.Run(name, func(t *testing.T) { var qb queries.Builder - qb.Appendf(tt.format, 1, 2, 3) - assert.Equal[E](t, qb.Query(), tt.query) + qb.Appendf(test.format, 1, 2, 3) + assert.Equal[E](t, qb.Query(), test.query) assert.Equal[E](t, qb.Args(), []any{1, 2, 3}) }) } @@ -68,46 +68,46 @@ func TestBuilder_sliceArgument(t *testing.T) { func TestBuilder_badQuery(t *testing.T) { tests := map[string]struct { - appendf func(*queries.Builder) - panicMsg string + appendf func(*queries.Builder) + query string }{ "wrong verb": { appendf: func(qb *queries.Builder) { qb.Appendf("SELECT %d FROM tbl", "foo") }, - panicMsg: "queries: bad query: SELECT %!d(string=foo) FROM tbl", + query: "SELECT %!d(string=foo) FROM tbl", }, "too few arguments": { appendf: func(qb *queries.Builder) { qb.Appendf("SELECT %s FROM tbl") }, - panicMsg: "queries: bad query: SELECT %!s(MISSING) FROM tbl", + query: "SELECT %!s(MISSING) FROM tbl", }, "too many arguments": { appendf: func(qb *queries.Builder) { qb.Appendf("SELECT %s FROM tbl", "foo", "bar") }, - panicMsg: "queries: bad query: SELECT foo FROM tbl%!(EXTRA queries.argument=bar)", + query: "SELECT foo FROM tbl%!(EXTRA queries.argument=bar)", }, - "different placeholders": { + "unexpected placeholder": { appendf: func(qb *queries.Builder) { - qb.Appendf("SELECT * FROM tbl WHERE foo = %? AND bar = %$ AND baz = %@", 1, 2, 3) + qb.Appendf("SELECT * FROM tbl WHERE foo = %? AND bar = %$", 1, 2) }, - panicMsg: "queries: different placeholders used", + query: "SELECT * FROM tbl WHERE foo = ? AND bar = %!$(PANIC=Format method: unexpected placeholder)", }, "non-slice argument": { appendf: func(qb *queries.Builder) { qb.Appendf("SELECT * FROM tbl WHERE foo IN (%+$)", 1) }, - panicMsg: "queries: bad query: SELECT * FROM tbl WHERE foo IN (%!$(PANIC=Format method: queries: %+ argument must be a slice))", + query: "SELECT * FROM tbl WHERE foo IN (%!$(PANIC=Format method: non-slice argument))", }, } - for name, tt := range tests { + for name, test := range tests { t.Run(name, func(t *testing.T) { var qb queries.Builder - tt.appendf(&qb) - assert.Panics[E](t, func() { qb.Query() }, tt.panicMsg) + test.appendf(&qb) + assert.Equal[E](t, qb.Query(), test.query) }) } }