Skip to content

fix: resolve variable name conflicts in GoRPC client generation#80

Merged
franklinkim merged 9 commits intomainfrom
fix/go-generator-naming-conflicts
Nov 7, 2025
Merged

fix: resolve variable name conflicts in GoRPC client generation#80
franklinkim merged 9 commits intomainfrom
fix/go-generator-naming-conflicts

Conversation

@quwe66
Copy link
Copy Markdown
Member

@quwe66 quwe66 commented Oct 28, 2025

The GoRPC client generator was creating variable name conflicts when service method parameters had the same name as internally generated variables. The generator uses fixed variable names like 'req' for request structs, but if a method parameter was also named 'req', it would create a naming conflict in the generated code.

Example of the conflict:

func (tsc *ServiceGoRPCClient) GetUser(req int) (name string, clientErr error) {
    req := ServiceGetUserRequest{Id: req}  // ← CONFLICT!
    //     ↑ variable name    ↑ parameter name
}

Changes:

  • Change parameter naming from field names to indexed arguments (arg_0, arg_1, etc.)
  • Add context filtering to prevent context.Context from being included in GoRPC requests
  • Add isContext() method to Value type for context detection
  • Ensure parameter names never conflict with internal generator variable names

This ensures that:

  • Generated GoRPC client code compiles without variable name conflicts
  • Method parameters use unique names that don't clash with internal variables
  • Context parameters are properly filtered from request structs
  • Generated code is robust regardless of service method parameter names

Fixes compilation errors when service methods have parameters with names that conflict with internally generated variable names (like 'req', 'response', etc.).

quwe66 and others added 3 commits October 28, 2025 15:03
The GoRPC client generator was creating variable name conflicts when service method
parameters had the same name as internally generated variables. The generator uses
fixed variable names like 'req' for request structs, but if a method parameter was
also named 'req', it would create a naming conflict in the generated code.
Another change fixes the problem with a function definition having context.Context as one of its arguments, the generated code would have the context argument twice then.
Copy link
Copy Markdown
Member

@franklinkim franklinkim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some more vars that need to be replaced and I'll handle the context a bit different... hang on

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 7, 2025

Codecov Report

❌ Patch coverage is 2.24057% with 1658 lines in your changes missing coverage. Please review.
✅ Project coverage is 2.11%. Comparing base (49be3bc) to head (9535198).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
example/basic/service/gorpcclient_gen.go 0.00% 385 Missing ⚠️
example/basic/service/gotsrpcclient_gen.go 0.00% 326 Missing ⚠️
example/basic/service/gotsrpc_gen.go 0.00% 261 Missing ⚠️
example/context/service/gotsrpc_gen.go 0.00% 127 Missing ⚠️
...xample/errors/service/backend/gotsrpcclient_gen.go 0.00% 60 Missing ⚠️
example/context/service/gorpc_gen.go 17.46% 52 Missing ⚠️
example/context/service/gorpcclient_gen.go 0.00% 51 Missing ⚠️
example/nullable/service/gorpcclient_gen.go 0.00% 48 Missing ⚠️
example/context/service/gotsrpcclient_gen.go 0.00% 44 Missing ⚠️
example/nullable/service/gotsrpcclient_gen.go 0.00% 40 Missing ⚠️
... and 30 more
Additional details and impacted files
@@           Coverage Diff            @@
##            main     #80      +/-   ##
========================================
+ Coverage   1.87%   2.11%   +0.23%     
========================================
  Files         62      69       +7     
  Lines       6815    7181     +366     
========================================
+ Hits         128     152      +24     
- Misses      6669    7011     +342     
  Partials      18      18              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@franklinkim
Copy link
Copy Markdown
Member

@quwe66 renamed more vars that could conflict.

Also, while I was on it:

i took your Context a bit further. You can do this now:

type Service interface {
  Hello(ctx context.Context, args string) string
}
client.hello("foomo").then((err) => {
  console.log(err);
});

Which will automatically pass the r.Context() through.

Errors can now be passed to the frontend

type Service interface {
  TypedError(ctx context.Context, msg string) error
  CustomError(ctx context.Context, msg string) error
  WrappedError(ctx context.Context, msg string) error
  StandardError(ctx context.Context, msg string) error
}
// {"m":"something went wrong: ups","p":"errors","t":"*errors.errorString","d":{}}
client.standardError("ups").then((err) => {
  console.log(JSON.stringify(err));
});

// {"m":"something","p":"github.com/pkg/errors","t":"*errors.fundamental","d":{}}
client.typedError("ups").then((err) => {
  console.log(JSON.stringify(err));
});

// {"m":"ups","p":"github.com/pkg/errors","t":"*errors.withMessage","d":{},"c":{"m":"something","p":"github.com/pkg/errors","t":"*errors.fundamental","d":{}}}
client.wrappedError("ups").then((err) => {
  console.log(JSON.stringify(err));
});

// {"m":"ups","p":"github.com/foomo/gotsrpc/v2/example/context/service","t":"*service.MyError","d":{"payload":"ups"},"c":{"m":"something","p":"github.com/pkg/errors","t":"*errors.fundamental","d":{}}}
client.customError("ups").then((err) => {
  console.log(JSON.stringify(err));
});

@franklinkim franklinkim merged commit 31d2c58 into main Nov 7, 2025
3 checks passed
@franklinkim franklinkim deleted the fix/go-generator-naming-conflicts branch November 7, 2025 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants