GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix: ignore kin-openapi source locations when comparing type definitions by eyoshioka · Pull Request #2601 · oapi-codegen/oapi-codegen

Conversation

@eyoshioka

@eyoshioka eyoshioka commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #2599.

Since #2465, the loader sets IncludeOrigin, so every schema records where it was declared. TypeDefinitionsEquivalent used reflect.DeepEqual, so identical schemas declared on different lines no longer matched.

This implements option 2 from my comment on the issue. It compares the schemas the same way as reflect.DeepEqual, but treats any two *openapi3.Origin values as equal. The code follows the standard library's deepValueEqual, including cycle detection for recursive schemas, and its doc comment lists where the two differ. Fields are skipped by type, so this also covers Origin on SchemaRef, Discriminator and other kin-openapi types, and on any fields added later. It adds no new dependency. If you'd rather use go-cmp, I'm happy to switch, and the tests stay the same.

Tests:

  • TestTypeDefinitionsEquivalentIgnoresOrigin covers inline, nested and recursive schemas, which should be equal. It also covers two cases that must stay different: the same relative $ref resolving to different files, and a 3.1 $ref with a sibling keyword. Comparing JSON would fail both of these.
  • TestTypeDefinitionsEquivalentCycles compares two separately built self-referencing schemas. A loaded spec shares one *Schema per $ref target, so the loaded cases never reach the cycle detection. Without the detection, this test overflows the stack.
  • TestIdenticalXGoTypeNameSchemasWithOrigin runs the issue's spec through Generate.

make generate produces no diff with Go 1.27.1, which matches CI's stable. With Go 1.25 or 1.26, only the embedded spec blobs change, with or without this change. make test and make lint pass.

I used Claude to help write this change. I've reviewed and tested it myself.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

greptile-apps[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@eyoshioka @claude
fix: ignore kin-openapi source locations when comparing type definitions
24d0166
Since oapi-codegen#2465 the loader sets IncludeOrigin, so every schema records the
line it was declared on. TypeDefinitionsEquivalent compared schemas with
reflect.DeepEqual, so two identical schemas with the same
x-go-type-name on different lines no longer matched, and generation
failed with "duplicate typename".

Compare the schemas like reflect.DeepEqual does, but treat any two
*openapi3.Origin values as equal. Comparing the JSON encodings instead
would drop resolved $ref targets and OpenAPI 3.1 $ref siblings, so
different schemas could be merged silently.

Fixes oapi-codegen#2599

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@eyoshioka
eyoshioka marked this pull request as ready for review October 8, 2026 10:40
@eyoshioka
eyoshioka requested a review from a team as a code owner October 8, 2026 10:40
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Updates TypeDefinitionsEquivalent to ignore kin-openapi source locations without changing the schemas.

  • Matching named schemas can share one generated Go type despite different source locations.

Reviews (3) · Last reviewed commit: "Revert "fix: blank origins and reuse ref..." · Reviewed by Greptile

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mromaszewicz mromaszewicz added the bug Something isn't working label Oct 29, 2026
mromaszewicz and others added 2 commits October 31, 2026 16:38
@mromaszewicz @claude
fix: blank origins and reuse reflect.DeepEqual in TypeDefinitionsEqui…
a4d1d60
…valent

Instead of carrying a copy of reflect's deepValueEqual that skips
*openapi3.Origin, walk both schemas once, nil every reachable Origin
field, run the real reflect.DeepEqual, and restore the origins afterwards.
Origins are still read later (for example to order paths by source line),
so a test checks that they come back.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@mromaszewicz
Merge remote-tracking branch 'upstream/main' into fix/2599-ignore-ori…
00bd056
…gin-in-type-equivalence
@mromaszewicz

mromaszewicz commented Oct 31, 2026

Copy link
Copy Markdown
Member

Thanks for doing this. I'm going to push a change to it. Rather than reimplementing so much of DeepEquals, we're going to back up and zero out the Origin on the schemas, then restore before exiting, so that we can reuse the existing DeepEquals representation. It's not as clean, but it's a lot simpler and works for us.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

greptile-apps[bot]
greptile-apps Bot reviewed Oct 31, 2026
View reviewed changes
Comment thread pkg/codegen/utils.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mromaszewicz

mromaszewicz commented Oct 31, 2026

Copy link
Copy Markdown
Member

NVM, what I did is worse. Apologies. I'll put it back as it was, and I merged main into your branch so this applies cleanly.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mromaszewicz @claude
Revert "fix: blank origins and reuse reflect.DeepEqual in TypeDefinit…
ebdf1f9
…ionsEquivalent"

This reverts commit a4d1d606c1cc5bd5e1dbe42f2a2d7af1fa3c9bd4.

Clearing the origins before calling reflect.DeepEqual was a mistake.
Origins sit on every nested Schema, SchemaRef and Discriminator, so the
"clear and restore" walker ends up being its own recursive traversal with
cycle detection, while also mutating shared schema objects during the
comparison. The original read-only comparer that skips *openapi3.Origin
is the simpler and safer approach, so this restores it as submitted.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
mromaszewicz
mromaszewicz approved these changes Oct 31, 2026
View reviewed changes
@mromaszewicz
mromaszewicz enabled auto-merge (squash) October 31, 2026 21:48
@mromaszewicz
mromaszewicz merged commit b4fdfe1 into oapi-codegen:main Oct 31, 2026
13 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@mromaszewicz mromaszewicz mromaszewicz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

v2.8.0: identical schemas with the same x-go-type-name now fail with "duplicate typename"

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@eyoshioka @mromaszewicz
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.