fix: use raw enum values for cross-enum conflict detection by natalie-o-perret · Pull Request #2390 · oapi-codegen/oapi-codegen

natalie-o-perret · 2026-06-01T16:10:24Z

Cross-enum conflict detection uses e1.GetValues() to check for shared values, but GetValues() returns prefixed names for enums already marked in an earlier loop iteration. Those prefixed names don't match the raw values of later enums, so some conflicts are silently missed, and whether an enum gets prefixed depends on iteration order rather than actual value overlap.

Fix: compare e1.Schema.EnumValues keys instead. These are always the unprefixed identifiers, regardless of whether the enum has been marked yet.

Regression test added (TestEnumConflictDetectionOrderIndependent): three string enums where A↔B and A↔C both conflict, but the A↔C conflict was previously missed once A was already prefixed.

Discovered via a downstream build failure after an upstream schema change removed a schema that had been the only direct conflict anchor for one of the enums.

greptile-apps · 2026-06-01T16:28:12Z

Greptile Summary

This PR fixes enum conflict detection so shared enum values are handled consistently. It changes:

Compares cross-enum conflicts using unprefixed enum identifiers.
Adds iterative detection for generated constant-name collisions after prefixing.
Adds a compatibility option to restore the older conflict detection behavior.
Updates regression coverage for order-independent enum prefixing.
Regenerates an affected enum fixture.

Confidence Score: 4/5

This is close, but the new config option should be wired into the schema before merging.

The enum conflict logic matches the intended fix.
The new compatibility option is incomplete for schema-validated configuration files.
No security concerns were found in the changed code.
configuration-schema.json should include compatibility.old-enum-conflict-detection.

Important Files Changed

Filename	Overview
pkg/codegen/codegen.go	Adds raw-value enum conflict detection plus an old-behavior compatibility branch.
pkg/codegen/configuration.go	Adds the compatibility flag but leaves the JSON configuration schema out of sync.

_{Reviews (2): Last reviewed commit: "feat: add old-enum-conflict-detection co..." | Re-trigger Greptile}

natalie-o-perret · 2026-06-01T16:30:06Z

Summoning @jamietanna for review

GetValues() returns prefixed identifier names for enums already marked as conflicting. Using it in the cross-enum comparison loop makes the result order-dependent: an enum that was prefixed in an earlier iteration produces keys like FooStateReady, which no longer match the unprefixed key Ready in a later enum, so the later enum misses the conflict and stays unprefixed. Fix by comparing Schema.EnumValues keys directly, which are always the raw (unprefixed) Go-safe identifiers regardless of PrefixTypeName state.

mromaszewicz · 2026-06-01T18:11:01Z

CI failed because your change affects existing generated code. I pulled your PR locally and I'm investigating, but we might need to feature-flag this to avoid breaking existing users.

mromaszewicz · 2026-06-01T18:16:31Z

I had Claude dig into this locally:

Why the files change

This PR changes conflict detection from comparing generated constant names (GetValues(), which reflects prefixing) to comparing raw schema values (Schema.EnumValues). That fixes the order-dependence in #2391, but it also drops a second class of conflict the old code happened to catch: an enum whose raw value equals another enum's prefixed constant name.

`internal/test/issues/issue-1189` — correct improvement ✅

Three enums all share raw values [bar, foo]: TestFieldA1, TestFieldB, TestFieldC1. All three should be prefixed. The old order-dependent code marked A1 and B, then — because GetValues() returned their already-prefixed names — failed to see that C1 still collided, leaving it unprefixed (Bar/Foo). The new code compares raw values, so C1 is now correctly prefixed to TestFieldC1Bar/TestFieldC1Foo. This is exactly the bug #2391 describes. Compiles fine, output is now consistent.

`internal/test/components` — regression ❌ (won't compile)

This spec was deliberately built to test the other collision class. From components.yaml:

Enum1: [One, Two, Three] — conflicts with Enum2, so it's prefixed → constant Enum1One = "One"
Enum3: [Enum1One, Foo, Bar] — its raw value is literally Enum1One (the comment even says "Enum values conflict with Enums above, need to be prefixed")

The old GetValues() code compared post-prefix names, so it saw Enum1's prefixed constant Enum1One collide with Enum3's value Enum1One and prefixed Enum3 → Enum3Enum1One. The new raw-value code only sees Enum3's raw values [Enum1One, Foo, Bar] vs Enum1's raw [One, Two, Three] — no overlap — so Enum3 is left unprefixed. Result:

Enum1One Enum1 = "One"        // line 16
Enum1One Enum3 = "Enum1One"   // line 59  ← duplicate

components/components.gen.go:59:2: Enum1One redeclared in this block
components/components.gen.go:68:7: invalid case Enum1One in switch ... (mismatched types Enum1 and Enum3)

A correct fix needs to detect both classes — raw-value overlap and the case where one enum's value matches another enum's would-be prefixed constant name —
while remaining order-independent. The current PR can't land as-is because internal/test/components no longer compiles.

mromaszewicz · 2026-06-01T18:28:58Z

@natalie-o-perret - This code is all extremely sensitive.

We can make new behavior default-on, but there definitely needs to be a flag to revert to current behavior, since I don't see a way of fixing this properly without breaking some old boilerplate.

natalie-o-perret · 2026-06-01T18:36:57Z

@mromaszewicz

CI failed because your change affects existing generated code. I pulled your PR locally and I'm investigating, but we might need to feature-flag this to avoid breaking existing users.

Thanks for the thorough write-up. I think you may have pulled before the second force push.

The current commit uses a 2-stage algorithm that handles both conflict classes.

internal/test/issues/issue-1189 -- correct improvement ✅

Agreed, that's exactly the bug this PR is fixing.

internal/test/components -- regression ❌ (won't compile)

This is handled by Stage 2. After Stage 1 marks Enum1 as prefixed (it shares raw values with Enum2), Stage 2 runs an iterative GetValues() comparison. At that point Enum1.GetValues() returns {"Enum1One": "One", ...} while Enum3.GetValues() (still unprefixed) returns {"Enum1One": "Enum1One", ...}. The key Enum1One appears in both, so Enum3 gets marked for prefixing too.

go test ./... passes and regenerating internal/test/components produces Enum3Bar, Enum3Enum1One, Enum3Foo with no compile errors. Could you try pulling again? Happy to debug further if you're still seeing the issue.

@natalie-o-perret - This code is all extremely sensitive.

We can make new behavior default-on, but there definitely needs to be a flag to revert to current behavior, since I don't see a way of fixing this properly without breaking some old boilerplate.

Fair point, I've added a compatibility.old-enum-conflict-detection: true flag (latest commit e13b895) that restores the pre-v2.7.1 GetValues()-based behavior. New behavior stays default-on; users who need to preserve existing generated output can opt out.

mromaszewicz · 2026-06-01T18:41:05Z

@greptileai re-review.

mromaszewicz · 2026-06-01T18:41:39Z

+		// already-applied prefixes, so enums processed later may miss conflicts
+		// with enums that were prefixed in an earlier iteration. Preserved here
+		// as an escape hatch for users who need to keep existing generated output.
+		for i := range enums {


If I understand this right, the next loop completely supersedes this one, so this one doesn't even need to run.

Not quite, Stage 2 alone misses CState in the AState/BState/CState case.

Once Stage 2's first pass prefixes AState because it shares "running" with BState, AState.GetValues() returns {AStateRunning, AStateMigrating}.

When it then checks AState vs CState, "migrating" in CState no longer matches any key in AState's now-prefixed values, so CState is left unprefixed.

I verified this by removing Stage 1 and running the tests, both TestEnumConflictDetectionOrderIndependent and TestEnumConflictDetectionBothOrders fail with Migrating CState = "migrating" in the output.

mromaszewicz · 2026-06-01T18:45:34Z

+	// prefixed could vary based on map iteration order (non-deterministic in Go).
+	// Set this only if you need to preserve existing generated output while you
+	// migrate. Please see https://github.com/oapi-codegen/oapi-codegen/issues/2391
+	OldEnumConflictDetection bool `yaml:"old-enum-conflict-detection,omitempty"`


This is getting confusing :) There's a old-enum-conflicts just above.

Sorry 🤦‍♀️

Folded into old-enum-conflicts in 204fe57, the legacy GetValues()-based path now runs when that flag is set.

Since old-enum-conflicts is already in configuration-schema.json, no schema update needed either.

natalie-o-perret force-pushed the fix/enum-conflict-detection-order-dependent branch 2 times, most recently from ddaed3d to 00d47d2 Compare June 1, 2026 16:22

natalie-o-perret marked this pull request as ready for review June 1, 2026 16:26

natalie-o-perret requested a review from a team as a code owner June 1, 2026 16:26

natalie-o-perret mentioned this pull request Jun 1, 2026

bug: cross-enum conflict detection is order-dependent due to GetValues() returning prefixed names #2391

Open

jamietanna added the auto:no-mentions label Jun 1, 2026

mromaszewicz added the bug Something isn't working label Jun 1, 2026

natalie-o-perret force-pushed the fix/enum-conflict-detection-order-dependent branch from 7fa6bd9 to 930146a Compare June 1, 2026 18:08

test: add order-independence test for both schema orderings

be163f0

natalie-o-perret force-pushed the fix/enum-conflict-detection-order-dependent branch from 1fb4e36 to be163f0 Compare June 1, 2026 18:31

feat: add old-enum-conflict-detection compatibility flag

e13b895

mromaszewicz reviewed Jun 1, 2026

View reviewed changes

greptile-apps Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread pkg/codegen/configuration.go Outdated

mromaszewicz reviewed Jun 1, 2026

View reviewed changes

natalie-o-perret added 2 commits June 1, 2026 20:57

fix: reuse old-enum-conflicts flag for legacy conflict detection path

204fe57

Merge branch 'main' into fix/enum-conflict-detection-order-dependent

f9a30af

natalie-o-perret requested a review from mromaszewicz June 8, 2026 12:05

Merge branch 'main' into fix/enum-conflict-detection-order-dependent

5c6b0af

Uh oh!

Conversation

natalie-o-perret commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

natalie-o-perret commented Jun 1, 2026

Uh oh!

mromaszewicz commented Jun 1, 2026

Uh oh!

mromaszewicz commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why the files change

internal/test/issues/issue-1189 — correct improvement ✅

internal/test/components — regression ❌ (won't compile)

Uh oh!

mromaszewicz commented Jun 1, 2026

Uh oh!

natalie-o-perret commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

internal/test/issues/issue-1189 -- correct improvement ✅

internal/test/components -- regression ❌ (won't compile)

Uh oh!

mromaszewicz commented Jun 1, 2026

Uh oh!

mromaszewicz Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

natalie-o-perret Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mromaszewicz Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

natalie-o-perret Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

natalie-o-perret Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

natalie-o-perret commented Jun 1, 2026 •

edited

Loading

greptile-apps Bot commented Jun 1, 2026 •

edited

Loading

mromaszewicz commented Jun 1, 2026 •

edited

Loading

`internal/test/issues/issue-1189` — correct improvement ✅

`internal/test/components` — regression ❌ (won't compile)

natalie-o-perret commented Jun 1, 2026 •

edited

Loading

`internal/test/issues/issue-1189` -- correct improvement ✅

`internal/test/components` -- regression ❌ (won't compile)