Fix Golang pattern validation with regex fails on commas#24349
Merged
Conversation
…ihan/openapi-generator into mlebihan-go_pattern_generation_20079
wing328
changed the title
wing328
marked this pull request as ready for review
July 19, 2026 17:40
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
7 issues found across 49 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/test/resources/3_0/issue_20079_go_regex_wrongly_translated.yaml">
<violation number="1" location="modules/openapi-generator/src/test/resources/3_0/issue_20079_go_regex_wrongly_translated.yaml:155">
P2: The phoneNumber regex anchors don't apply to both alternatives. `^` only anchors the hyphen-separated branch and `$` only anchors the space-separated branch, so `abc 519 888 6898` or `519-883-6898 xyz` would incorrectly match. Consider wrapping both alternatives in a group anchored at both ends: `^(([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\s[2-9][0-9]{2}\s[0-9]{4}))$`.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/go/model_oneof.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/go/model_oneof.mustache:83">
P3: `{{#oneOf}}` iteration in the "no match" else block generates duplicate Go code for types with multiple oneOf schemas. The template content only references `{{classname}}` (same every iteration), not `{{{.}}}` (current item), so each iteration produces an identical `if err != nil { return ... } else { return ... }` block. For types with 2+ oneOf items, all blocks after the first are unreachable dead code. This is a pre-existing pattern (original `{{#oneOf}}return fmt.Errorf(...){{/oneOf}}` had the same issue), but since this PR restructures the same section, consider removing the `{{#oneOf}}` wrapper entirely and keeping only the single non-iterated block with the `{{^oneOf}}` fallback.</violation>
</file>
<file name="samples/openapi3/client/petstore/go/go-petstore/model_one_of_primitive_types.go">
<violation number="1" location="samples/openapi3/client/petstore/go/go-petstore/model_one_of_primitive_types.go:88">
P1: The same `if err != nil` block is duplicated — the second block is unreachable dead code because the first block always returns (both the `if` and `else` branches end with `return`). Remove the duplicate block.</violation>
</file>
<file name="samples/openapi3/client/petstore/go/go-petstore/model_one_of_with_complex_type.go">
<violation number="1" location="samples/openapi3/client/petstore/go/go-petstore/model_one_of_with_complex_type.go:87">
P1: Dead/unreachable code: the first `if err != nil { return ... } else { return ... }` block covers both paths and always returns, so the second identical block can never execute. This looks like a copy-paste error during template generation — the old fallback return should have been replaced entirely, not duplicated below.</violation>
</file>
<file name="samples/client/others/go/issue_20079_go_regex_wrongly_translated/README.md">
<violation number="1" location="samples/client/others/go/issue_20079_go_regex_wrongly_translated/README.md:98">
P3: README documents a `PtrFloat` utility function that doesn't exist in the generated code. Users relying on the docs would get a compilation error. The actual helpers are `PtrFloat32` and `PtrFloat64` — consider removing `PtrFloat` from the list.</violation>
</file>
<file name="samples/client/others/go/oneof-anyof-required/model_object.go">
<violation number="1" location="samples/client/others/go/oneof-anyof-required/model_object.go:87">
P1: The oneOf error-handling block in the no-match branch is duplicated. The first `if err != nil` block always returns (via either branch), so the second identical block is unreachable dead code — it will never execute. Remove the duplicate block.</violation>
</file>
<file name="samples/client/others/go/issue_20079_go_regex_wrongly_translated/.openapi-generator/FILES">
<violation number="1" location="samples/client/others/go/issue_20079_go_regex_wrongly_translated/.openapi-generator/FILES:17">
P2: Generated test file `test/api_default_test.go` is not tracked in `.openapi-generator/FILES`. The file has the "Code generated by OpenAPI Generator" header and should be tracked to ensure proper cleanup during regeneration.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| return nil // exactly one match | ||
| } else { // no match | ||
| return fmt.Errorf("data failed to match schemas in oneOf(OneOfPrimitiveTypes)") | ||
| if err != nil { |
Contributor
There was a problem hiding this comment.
P1: The same if err != nil block is duplicated — the second block is unreachable dead code because the first block always returns (both the if and else branches end with return). Remove the duplicate block.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/go/go-petstore/model_one_of_primitive_types.go, line 88:
<comment>The same `if err != nil` block is duplicated — the second block is unreachable dead code because the first block always returns (both the `if` and `else` branches end with `return`). Remove the duplicate block.</comment>
<file context>
@@ -85,7 +85,16 @@ func (dst *OneOfPrimitiveTypes) UnmarshalJSON(data []byte) error {
return nil // exactly one match
} else { // no match
- return fmt.Errorf("data failed to match schemas in oneOf(OneOfPrimitiveTypes)")
+ if err != nil {
+ return fmt.Errorf("data failed to match schemas in oneOf(OneOfPrimitiveTypes): %v", err)
+ } else {
</file context>
| return nil // exactly one match | ||
| } else { // no match | ||
| return fmt.Errorf("data failed to match schemas in oneOf(OneOfWithComplexType)") | ||
| if err != nil { |
Contributor
There was a problem hiding this comment.
P1: Dead/unreachable code: the first if err != nil { return ... } else { return ... } block covers both paths and always returns, so the second identical block can never execute. This looks like a copy-paste error during template generation — the old fallback return should have been replaced entirely, not duplicated below.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/go/go-petstore/model_one_of_with_complex_type.go, line 87:
<comment>Dead/unreachable code: the first `if err != nil { return ... } else { return ... }` block covers both paths and always returns, so the second identical block can never execute. This looks like a copy-paste error during template generation — the old fallback return should have been replaced entirely, not duplicated below.</comment>
<file context>
@@ -84,7 +84,16 @@ func (dst *OneOfWithComplexType) UnmarshalJSON(data []byte) error {
return nil // exactly one match
} else { // no match
- return fmt.Errorf("data failed to match schemas in oneOf(OneOfWithComplexType)")
+ if err != nil {
+ return fmt.Errorf("data failed to match schemas in oneOf(OneOfWithComplexType): %v", err)
+ } else {
</file context>
| return nil // exactly one match | ||
| } else { // no match | ||
| return fmt.Errorf("data failed to match schemas in oneOf(Object)") | ||
| if err != nil { |
Contributor
There was a problem hiding this comment.
P1: The oneOf error-handling block in the no-match branch is duplicated. The first if err != nil block always returns (via either branch), so the second identical block is unreachable dead code — it will never execute. Remove the duplicate block.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/go/oneof-anyof-required/model_object.go, line 87:
<comment>The oneOf error-handling block in the no-match branch is duplicated. The first `if err != nil` block always returns (via either branch), so the second identical block is unreachable dead code — it will never execute. Remove the duplicate block.</comment>
<file context>
@@ -84,7 +84,16 @@ func (dst *Object) UnmarshalJSON(data []byte) error {
return nil // exactly one match
} else { // no match
- return fmt.Errorf("data failed to match schemas in oneOf(Object)")
+ if err != nil {
+ return fmt.Errorf("data failed to match schemas in oneOf(Object): %v", err)
+ } else {
</file context>
| non-matches: 888 6898, 5198886898, 519 883-6898" | ||
| type: string | ||
|
|
||
| pattern: "^([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\\s[2-9][0-9]{2}\\s[0-9]{4})$" |
Contributor
There was a problem hiding this comment.
P2: The phoneNumber regex anchors don't apply to both alternatives. ^ only anchors the hyphen-separated branch and $ only anchors the space-separated branch, so abc 519 888 6898 or 519-883-6898 xyz would incorrectly match. Consider wrapping both alternatives in a group anchored at both ends: ^(([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\s[2-9][0-9]{2}\s[0-9]{4}))$.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/test/resources/3_0/issue_20079_go_regex_wrongly_translated.yaml, line 155:
<comment>The phoneNumber regex anchors don't apply to both alternatives. `^` only anchors the hyphen-separated branch and `$` only anchors the space-separated branch, so `abc 519 888 6898` or `519-883-6898 xyz` would incorrectly match. Consider wrapping both alternatives in a group anchored at both ends: `^(([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\s[2-9][0-9]{2}\s[0-9]{4}))$`.</comment>
<file context>
@@ -0,0 +1,191 @@
+ non-matches: 888 6898, 5198886898, 519 883-6898"
+ type: string
+
+ pattern: "^([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\\s[2-9][0-9]{2}\\s[0-9]{4})$"
+ # Original was: ([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\s[2-9][0-9]{2}\s[0-9]{4})
+
</file context>
Suggested change
| pattern: "^([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\\s[2-9][0-9]{2}\\s[0-9]{4})$" | |
| pattern: "^(([2-9][0-9]{2}-[2-9][0-9]{2}-[0-9]{4})|([2-9][0-9]{2}\\s[2-9][0-9]{2}\\s[0-9]{4}))$" |
| @@ -0,0 +1,17 @@ | |||
| .gitignore | |||
Contributor
There was a problem hiding this comment.
P2: Generated test file test/api_default_test.go is not tracked in .openapi-generator/FILES. The file has the "Code generated by OpenAPI Generator" header and should be tracked to ensure proper cleanup during regeneration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/go/issue_20079_go_regex_wrongly_translated/.openapi-generator/FILES, line 17:
<comment>Generated test file `test/api_default_test.go` is not tracked in `.openapi-generator/FILES`. The file has the "Code generated by OpenAPI Generator" header and should be tracked to ensure proper cleanup during regeneration.</comment>
<file context>
@@ -0,0 +1,17 @@
+model__foo_get_200_response.go
+model_import_code.go
+response.go
+utils.go
</file context>
| return nil // exactly one match | ||
| } else { // no match | ||
| return fmt.Errorf("data failed to match schemas in oneOf({{classname}})") | ||
| {{#oneOf}} |
Contributor
There was a problem hiding this comment.
P3: {{#oneOf}} iteration in the "no match" else block generates duplicate Go code for types with multiple oneOf schemas. The template content only references {{classname}} (same every iteration), not {{{.}}} (current item), so each iteration produces an identical if err != nil { return ... } else { return ... } block. For types with 2+ oneOf items, all blocks after the first are unreachable dead code. This is a pre-existing pattern (original {{#oneOf}}return fmt.Errorf(...){{/oneOf}} had the same issue), but since this PR restructures the same section, consider removing the {{#oneOf}} wrapper entirely and keeping only the single non-iterated block with the {{^oneOf}} fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/go/model_oneof.mustache, line 83:
<comment>`{{#oneOf}}` iteration in the "no match" else block generates duplicate Go code for types with multiple oneOf schemas. The template content only references `{{classname}}` (same every iteration), not `{{{.}}}` (current item), so each iteration produces an identical `if err != nil { return ... } else { return ... }` block. For types with 2+ oneOf items, all blocks after the first are unreachable dead code. This is a pre-existing pattern (original `{{#oneOf}}return fmt.Errorf(...){{/oneOf}}` had the same issue), but since this PR restructures the same section, consider removing the `{{#oneOf}}` wrapper entirely and keeping only the single non-iterated block with the `{{^oneOf}}` fallback.</comment>
<file context>
@@ -80,7 +80,16 @@ func (dst *{{classname}}) UnmarshalJSON(data []byte) error {
return nil // exactly one match
} else { // no match
- return fmt.Errorf("data failed to match schemas in oneOf({{classname}})")
+ {{#oneOf}}
+ if err != nil {
+ return fmt.Errorf("data failed to match schemas in oneOf({{classname}}): %v", err)
</file context>
| * `PtrInt` | ||
| * `PtrInt32` | ||
| * `PtrInt64` | ||
| * `PtrFloat` |
Contributor
There was a problem hiding this comment.
P3: README documents a PtrFloat utility function that doesn't exist in the generated code. Users relying on the docs would get a compilation error. The actual helpers are PtrFloat32 and PtrFloat64 — consider removing PtrFloat from the list.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/go/issue_20079_go_regex_wrongly_translated/README.md, line 98:
<comment>README documents a `PtrFloat` utility function that doesn't exist in the generated code. Users relying on the docs would get a compilation error. The actual helpers are `PtrFloat32` and `PtrFloat64` — consider removing `PtrFloat` from the list.</comment>
<file context>
@@ -0,0 +1,107 @@
+* `PtrInt`
+* `PtrInt32`
+* `PtrInt64`
+* `PtrFloat`
+* `PtrFloat32`
+* `PtrFloat64`
</file context>
Suggested change
| * `PtrFloat` | |
| * `PtrFloat32` | |
| * `PtrFloat64` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
based on #20369
to close #20079
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Fix Go regex pattern generation to emit correct
validate:"regexp=..."tags, preserving delimiters/flags and proper escaping so patterns with backslashes, slashes, or commas don’t break validation. Also improves oneOf error messages and adds sample/CI coverage for the regression (issue 20079).regexptag generation: preserve/.../flags, escape backslashes correctly, and handle commas inside patterns.specVersionGreaterThanOrEqualTo310to avoid NPE whenopenAPIis null.go-regex-test.yamland a new sample undersamples/client/others/go/issue_20079_go_regex_wrongly_translated, update workflow to build it, and regenerate affected Go samples.Written for commit 616736a. Summary will update on new commits.