Skip to content

Omit unused numeric enum parse maps with noparse - #328

Open
jihtsan wants to merge 1 commit into
abice:masterfrom
jihtsan:codex/313-omit-unused-parse-map
Open

jihtsan wants to merge 1 commit into
abice:masterfrom
jihtsan:codex/313-omit-unused-parse-map

Conversation

@jihtsan

@jihtsan jihtsan commented Oct 19, 2026 •

Copy link
Copy Markdown

--noparse currently leaves a numeric enum's reverse lookup map in the output even when no parse function uses it. Emit that map alongside the parse function so it is omitted when parsing is disabled.

Maps needed by private parsers for marshal, SQL, flags, or MustParse remain generated. String enums also retain their lookup map because IsValid still uses it. Extend the existing regression cases and regenerate the noparse example.

Fixes #313.

Validation on macOS arm64, Go 1.25.5:

  • Numeric regression fails against the original template; string and marshal controls pass.
  • go test -race -shuffle=on ./...: passed.
  • go test -race -shuffle=on --tags=example ./example: passed.
  • go vet ./..., formatting and git diff --check: passed.
  • After make deps, complete make generate passes and leaves the committed checkout clean. The earlier attempt without mockgen failed; no generated failure was treated as a passing check.

Other platforms and the Go 1.26 CI lane were not run locally. No binary-size or runtime-performance claim is made. This contribution was prepared with AI assistance.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected generated enum lookup maps when parsing is disabled, omitting unnecessary maps while retaining those needed for validation or marshaling.
  • Tests
    • Added coverage for lookup-map behavior across integer and string enums, including enums with marshaling support.

@jihtsan
jihtsan requested a review from abice as a code owner October 19, 2026 10:30
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5c0c8677-bd7f-488a-a962-9763300d9116
📥 Commits

Reviewing files that changed from the base of the PR and between ad6b63d and e38adfa.

📒 Files selected for processing: 3
  • example/noparse_enum.go
  • generator/enum.tmpl
  • generator/generator_test.go
💤 Files with no reviewable changes: 1
  • example/noparse_enum.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The enum template now omits lookup maps when parsing is disabled. Tests cover map generation for integer and string enums, including marshal support. The example removes an unused lookup map.

Changes: Enum lookup map generation

Layer / File(s) Summary
Conditional lookup map generation
generator/enum.tmpl, generator/generator_test.go, example/noparse_enum.go
The template emits lookup maps only when parsing is enabled. Tests check map output for integer enums, string enums, and marshal support. The example removes an unused map.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e38ad

No concrete merge-blocking risk remains in the reviewed changes; the map is retained for generated features that need it.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: omitting unused numeric enum parse maps when noparse is enabled.
Linked Issues check Passed The change satisfies issue [#313]. generator/enum.tmpl now emits _{{.enum.Name}}Value inside the .generateParse block. The generated example/noparse_enum.go omits _UnparsedValuesValue, while…
Out of Scope Changes check Passed The changed files stay within issue [#313]. The template change implements conditional map generation. The generator test updates verify the required numeric, string, and marshal cases. The regenerate…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 …
Full details: Linked Issues check

Explanation

The change satisfies issue [#313]. generator/enum.tmpl now emits _{{.enum.Name}}Value inside the .generateParse block. The generated example/noparse_enum.go omits _UnparsedValuesValue, while UnparsedString retains its lookup map for IsValid. The reported regression checks also cover numeric no-parse output and numeric marshal output.

Full details: Out of Scope Changes check

Explanation

The changed files stay within issue [#313]. The template change implements conditional map generation. The generator test updates verify the required numeric, string, and marshal cases. The regenerated noparse example records the expected output. No unrelated change is shown.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

--noparse doesn't remove the no-longer needed _{{enum.Name}}Value map

1 participant