[go] Fixes problems with conflicting parameters

[thiagoarrais]

OpenAPI allows parameters to bear the same name if they appear in different settings inside the same operation.

The code generated by the go-experimental go generator, though, can sometimes fail to compile due to parameters with the same name. Both the field in the api*Request struct and the setter method names can clash if there are multiple parameteres with the same name.

This change fixes that by namespacing all the param names in the generated code with their origin.

This may also affect the stable Go generator because it messes with setExportParameterName, even though I've tried to limit my changes to the internal parts of the generated code. Please review accordingly.

Update: this PR was written before go-experimental graduated to become the main go client and was edited accordingly.

PR checklist

• Read the contribution guidelines.
• If contributing template-only or documentation-only changes which will change sample output, build the project before.
• Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
• File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
• Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

cc @antihax @bvwells @grokify @kemokemo @bkabrda

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[thiagoarrais]

Rebased to current master. Any changes needed here?

[thiagoarrais]

@wing328 @bkabrda: friendly ping (not sure if I'm pinging the right people). Any thoughts?

[thiagoarrais]

Rebased. Bump?

[wing328]

@thiagoarrais thanks for the PR and sorry for taking so long to review it. I'll try to review later this week and let you know if I've any question/feedback.

Btw, is there a minimal spec to more easily reproduce the issue?

[thiagoarrais]

Yup. There is one in modules/openapi-generator/src/test/resources/3_0/conflictingParameter.yaml

[thiagoarrais]

As I understand go-experimental is now the main go generator? I've reviewed this and rebased accordingly, but I'm not 100% sure that no bug slipped away. Just let me know if you need anything.

[thiagoarrais]

By the way, this is a breaking change for some cases considering the old go-experimental code. But I guess this is as good a time as it gets to introduce breaking changes since (as I understand) we're already packaging the graduation of go-experimental into the 5.x major version change.

[wing328]

Thanks for the additional information. I'll try to review tomorrow or later this week.

[thiagoarrais]

Rebased to current master and fixed conflicts. Shippable and bitrise failures seem unrelated?

[thiagoarrais]

Rebased

[thiagoarrais]

Rebased to current master

[thiagoarrais]

Rebased to current master. Don't know if the circleci failure is related.

Friendly ping, @wing328: do you think this is mergeable?

[thiagoarrais]

Rebased to current master.

Just noticed that this is now partially solved in master. Somewhere between 8afb067 and 79395de a disambiguating suffix was added to parameter names. It makes the resulting client compilable for most cases. "Most" because it seems to assume that disambiguation only needs to happen for path parameters. But the same parameter name can happen in form, query, cookie or headers parameters.

Even though it is compilable for most cases, I think it can be yet a little more friendly (and this PR provides some of that). Here is the difference on the output from before and after this PR.

Before (master):

type ApiCreateRequest struct {
	id int32
	id2 *int32
	id2 *int32
	id2 *int32
	id2 *int32
        /* ... */
}

func (r ApiCreateRequest) Id2(id2 int32) ApiCreateRequest { /* ... */ }
func (r ApiCreateRequest) Id2(id2 int32) ApiCreateRequest { /* ... */ }
func (r ApiCreateRequest) Id2(id2 int32) ApiCreateRequest { /* ... */ }
func (r ApiCreateRequest) Id2(id2 int32) ApiCreateRequest { /* ... */ }

After (this PR as is):

type ApiCreateRequest struct {
	pathid int32
	headerid2 *int32
	cookieid2 *int32
	queryid2 *int32
	formid2 *int32
        /* ... */
}

func (r ApiCreateRequest) HeaderId2(id2 int32) ApiCreateRequest { /* ... */ }
func (r ApiCreateRequest) CookieId2(id2 int32) ApiCreateRequest { /* ... */ }
func (r ApiCreateRequest) QueryId2(id2 int32) ApiCreateRequest { /* ... */ }
func (r ApiCreateRequest) Id2(id2 int32) ApiCreateRequest { /* ... */ }

The version currently on master detects that parameter names conflict with the path parameter name and adds a disambiguating numerical suffix. This PR adds an easier to locate namespace preffix to the parameters (except for the form parameter, which I've assumed is the most common one and chosen to award with the no-preffix namespace).

We can make the resulting API even more clear by removing the suffix. I can add that to this PR if there is interest.

[wing328]

As discussed in #8762