fix(search): accept nil SearchOptions in every search method
What does this MR do?
Every SearchService method builds its query with &searchOptions{SearchOptions: *opt, ...}, and only UsersByGroup checks opt for nil first, replacing it with empty options as several list methods elsewhere in the library do (in workitems.go and achievements.go, for example). In the other 24 methods a nil options pointer panics with a nil pointer dereference before any request is built: client.Search.MergeRequestsByProject(1, "fix", nil), for example, panics at search.go line 171. The search API requires only scope and search, which every method fills in from its own arguments.
This adds an unexported newSearchOptions helper that copies the options only when they are set, uses it in all 25 methods, and drops the nil check of UsersByGroup that the helper now covers. The //nolint:goconst directives on the Issues and MergeRequests lines go too: with the change in place, nolintlint reports both as unused.
It comes from maintaining gitlab-mcp-server, an MCP server built on this library whose search tools call these methods.
How this relates to !3065 and !3135
!3065 (mine) adds the client option WithNilOptionsOmitted(), which decides what NewRequestToURL does with a nil options pointer that reaches it. It cannot help here: these methods panic at the dereference before a request is built, and after this change they always hand the request builder a non-nil value, so the option has no effect on them either way. The two touch different files and can land in either order.
!3135 fixes the same kind of panic in CreateGroup and UpdateGroup with a check in each method. Here all 25 methods share one query struct, so the check lives in one helper rather than in the three lines UsersByGroup had, copied into the other 24. If you prefer the inline form, I can switch to it; the test stays the same.
Is this a breaking change?
No. No exported name, type or signature changes. A search call with nil options, which panicked, now sends a request with only scope and search. A call with options sends the same query as before, which the new test checks for all 25 methods.
How was this tested?
TestSearchService_SearchOptions runs each of the 25 methods twice against an httptest server: with nil options, asserting that the query is exactly scope and search, and with options, asserting that page, per_page, ref and search_type are sent beside them.
- On
mainwithout the fix, the 24 nil subtests fail withruntime error: invalid memory address or nil pointer dereference, each at its method's*optline, and the other 26 pass (UsersByGroupwith nil options, and all 25 with options). - With the fix, all 50 pass.
- Dropping the copy of the options from the helper fails the 25 "with options" subtests, and dropping its nil check fails the 25 nil ones.
go test work -race(themake testtarget) passes on Go 1.26.6 and 1.27.2 (tests:unitruns Go 1.26 and 1.27). golangci-lint 2.14.0, run the waymake lintruns it, reports 0 issues;gofumpt -l(0.8.0) lists nothing; and the mock mockgen generates forSearchServiceInterfaceis unchanged, since no exported signature moved.
Test output without and with the fix
Without the fix, at 95be8ccf (one of the 24 failures, paths shortened):
=== NAME TestSearchService_SearchOptions/MergeRequestsByProject/nil_options
search_test.go:827:
Error: func (assert.PanicTestFunc)(0xc34d60) should not panic
Panic value: runtime error: invalid memory address or nil pointer dereference
...
gitlab.com/gitlab-org/api/client-go/v3.(*SearchService).MergeRequestsByProject(...)
search.go:171 +0x120With the fix:
--- PASS: TestSearchService_SearchOptions (0.00s)
--- PASS: TestSearchService_SearchOptions/MergeRequestsByProject/with_options (0.00s)
--- PASS: TestSearchService_SearchOptions/MergeRequestsByProject/nil_options (0.00s)
ok gitlab.com/gitlab-org/api/client-go/v3 0.029sgitlab-mcp-server keeps a record of what it finds and contributes in the projects it depends on, and in sibling projects, in upstream-bugs.md.