Fix broken request wrappers in generated gRPC code (#75) - #82
Conversation
The request-wrapper template ranges over .Requests ([]ServiceRequest) but
referenced the loop variable directly as `{{ $request }}`. text/template
falls back to fmt's struct format, so it rendered `{GetThingRequest
GetThingRequest}` — producing `type {GetThingRequest GetThingRequest}Wrapper`
and `*{GetThingRequest GetThingRequest}`, which do not compile
("syntax error: unexpected {, expected name/type").
Use `{{ $request.Request }}` at every site in the request-wrapper block
(type, embedded field, all receiver methods, and the Bind reflect call).
Adds the first test for the wrap package, asserting the request wrappers
render as `<Type>Wrapper` for multiple request types and that no struct
literal leaks into the output.
Fixes gofr-dev#75
Umang01-hash
left a comment
There was a problem hiding this comment.
Verified locally — nice fix. Confirmed the bug: on main the generated request wrapper doesn't even parse as Go ({GetThingRequest GetThingRequest}Wrapper → expected 'IDENT', found '{'), and with this change it parses cleanly. Every {{ $request }} site is updated to {{ $request.Request }}, and the regression test asserts no struct-literal leaks. Build + go test ./wrap green.
Since this fixes non-compiling output (any proto with >1 request type), I'd merge this first.
Heads up: #81 also adds wrap/template_test.go with the same createTestContext helper, so whichever of the two lands second will need a quick rebase to combine the test file (one createTestContext). LGTM.
aryanmehrotra
left a comment
There was a problem hiding this comment.
LGTM! Tested locally end-to-end with protoc and go build. Clean fix for #75.
The upstream gofr-dev#82 merge left testify/assert and gofr.dev/pkg/* in separate groups, but gci is configured [standard, default, localmodule] with go.mod module = gofr.dev/cli/gofr — so gofr.dev/pkg/* is 'default', same group as testify. Matches the pattern in main_test.go.
Objective
Fixes #75.
gofr wrap grpc servergenerates broken Go: every request-wrapper type/method renders as a struct literal (e.g.{GetThingRequest GetThingRequest}Wrapper) instead ofGetThingRequestWrapper, sorequest_gofr.goand its use sites don't compile (syntax error: unexpected {, expected name).Root cause
The request-wrapper block ranges over
.Requests([]ServiceRequest) but referenced the loop variable directly:text/templatefalls back tofmt's default struct format{field1 field2}, so{{ $request }}renders as{GetThingRequest GetThingRequest}.Fix
Use
{{ $request.Request }}at every site in the block — the type, the embedded field, all five receiver methods, and thereflect.ValueOf(h.…)call inBind. One-line-per-site template change; no behavior change beyond emitting the correct identifier.Tests
Adds
wrap/template_test.go(first test coverage for thewrappackage): rendersgenerateGoFrRequestWrapperfor multiple request types and asserts each becomes<Type>Wrapper(type,Context,Bind, embedded field, reflect call) and that no struct literal leaks into the output.Test plan
go test ./...passes (incl. the newwraptest).main.go, andgo build ./...succeeds —request_gofr.gonow containstype GetThingRequestWrapper struct/*GetThingRequest/reflect.ValueOf(h.GetThingRequest).gofmt/go vet ./...clean.Docs
No documentation change needed — the README only lists
wrap grpcas a feature and doesn't reference the generated wrapper internals; this is a code-generation correctness fix.Note
The same one-line fix is also present, bundled with unrelated naming changes, in the still-open #81. This PR isolates it as the focused fix for #75 so it can merge on its own.