Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Discovery accepts array types but codegen panics on them
- Extended names-type codegen handling to support string arrays and 2D string arrays by converting them to slice forms before parser/test generation and by updating type/index logic.
- ✅ Fixed: UnmarshalJSON references parser not generated when NoParser set
- Changed JSON generation to use a local ParseFactory parser and direct names lookup so JSON methods no longer depend on generated Parse or String methods.
- ✅ Fixed: Test file generated with wrong package name suffix
- Kept same-package tests but avoided circular self-import by conditionally referencing local TestSuite when generating tests for the enumify package itself.
Or push these changes by commenting:
@cursor push e3435e43e9
Preview (e3435e43e9)
diff --git a/enumify.go b/enumify.go
--- a/enumify.go
+++ b/enumify.go
@@ -149,6 +149,7 @@
for _, etype := range etypes {
typeID := etype.Id()
namesVar := etype.NamesVarId()
+ namesVarValue := etype.namesVarValue()
f.Comment("//============================================================================")
f.Comment("// " + etype.Name + " Enum Type: Generated Functions and Methods")
@@ -163,7 +164,7 @@
// ideas on how to improve this.
parserVar := g.Id(LowerFirst(etype.Name) + "Parser")
parserType := g.Func().Params(g.Any()).Params(typeID, g.Error())
- f.Var().Add(parserVar, parserType).Op("=").Qual("go.rtnl.ai/enumify", "ParseFactory").Types(typeID).Call(namesVar)
+ f.Var().Add(parserVar, parserType).Op("=").Qual("go.rtnl.ai/enumify", "ParseFactory").Types(typeID).Call(namesVarValue)
f.Line()
f.Commentf("Parse%s parses the given value into a %s.", etype.Name, etype.Name)
@@ -198,7 +199,10 @@
f.Commentf("Ensure %s implements json.Marshaler.", etype.Name)
method := methodSig.Clone().Id("MarshalJSON").Call().Params(g.Id("[]byte"), g.Error())
method.Block(
- g.Return(g.Qual("encoding/json", "Marshal").Call(s.Clone().Dot("String").Call())),
+ g.If(s.Clone().Op(">=").Add(typeID).Call(g.Len(namesVar))).Block(
+ g.Return(g.Qual("encoding/json", "Marshal").Call(etype.IndexNames(etype.ZeroConstId()))),
+ ),
+ g.Return(g.Qual("encoding/json", "Marshal").Call(etype.IndexNames(s))),
)
f.Add(method)
f.Line()
@@ -207,6 +211,7 @@
method = methodPtrSig.Clone().Id("UnmarshalJSON").Call(g.Id("data").Id("[]byte")).Params(g.Id("err").Error())
method.Block(
g.Var().Id("v").Any(),
+ g.Id("parse").Op(":=").Qual("go.rtnl.ai/enumify", "ParseFactory").Types(typeID).Call(namesVarValue),
g.If(
g.Id("err").Op("=").Qual("encoding/json", "Unmarshal").Call(g.Id("data"), g.Op("&").Id("v")).Op(";").Id("err").Op("!=").Nil()).
Block(
@@ -214,7 +219,7 @@
),
g.Line(),
g.If(
- g.Op("*").Add(s).Op(",").Id("err").Op("=").Id("Parse"+etype.Name).Call(g.Id("v")).Op(";").Id("err").Op("!=").Nil().
+ g.Op("*").Add(s).Op(",").Id("err").Op("=").Id("parse").Call(g.Id("v")).Op(";").Id("err").Op("!=").Nil().
Block(
g.Return(g.Id("err")),
),
@@ -240,7 +245,6 @@
// Manage import names
f.ImportName("testing", "testing")
- f.ImportName("go.rtnl.ai/enumify", "enumify")
// Discover the enum types in the package.
var etypes EnumTypes
@@ -248,13 +252,20 @@
return err
}
+ testSuiteID := g.Qual("go.rtnl.ai/enumify", "TestSuite")
+ if opts.Pkg != "enumify" {
+ f.ImportName("go.rtnl.ai/enumify", "enumify")
+ } else {
+ testSuiteID = g.Id("TestSuite")
+ }
+
// For each enum type, write a test function that creates and executes an enumify
// test suite for the enum type.
for _, etype := range etypes {
f.Func().Id("Test"+UpperFirst(etype.Name)).Add(TestingT).Block(
- g.Id("suite").Op(":=").Qual("go.rtnl.ai/enumify", "TestSuite").Types(etype.Id(), etype.NamesVarTypeId()).Block(
+ g.Id("suite").Op(":=").Add(testSuiteID.Clone().Types(etype.Id(), etype.NamesVarTypeId())).Block(
g.Id("Values").Op(":").Add(etype.ConstLiteral()).Op(","),
- g.Id("Names").Op(":").Add(etype.NamesVarId()).Op(","),
+ g.Id("Names").Op(":").Add(etype.namesVarValue()).Op(","),
g.Id("ICase").Op(":").Lit(!opts.CaseSensitive).Op(","),
g.Id("ISpace").Op(":").Lit(!opts.SpaceSensitive).Op(","),
),
diff --git a/generation_regression_test.go b/generation_regression_test.go
new file mode 100644
--- /dev/null
+++ b/generation_regression_test.go
@@ -1,0 +1,148 @@
+package enumify_test
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ "go.rtnl.ai/enumify"
+)
+
+func TestGenerateSupportsArrayNameVars(t *testing.T) {
+ const src = `package sample
+
+type Kind uint8
+
+const (
+ KindUnknown Kind = iota
+ KindOn
+ KindOff
+)
+
+var kindNames = [3]string{"unknown", "on", "off"}
+
+type Mode uint8
+
+const (
+ ModeUnknown Mode = iota
+ ModeRead
+ ModeWrite
+)
+
+var modeNames = [2][3]string{
+ {"unknown", "read", "write"},
+ {"UNKNOWN", "READ", "WRITE"},
+}
+`
+
+ dir := t.TempDir()
+ require.NoError(t, os.WriteFile(filepath.Join(dir, "schema.go"), []byte(src), 0o644))
+
+ withWorkingDirectory(t, dir, func() {
+ opts := enumify.Options{
+ File: "schema.go",
+ Pkg: "sample",
+ }
+
+ require.NoError(t, enumify.Generate(opts))
+ require.NoError(t, enumify.GenerateTests(opts))
+
+ genBytes, err := os.ReadFile(filepath.Join(dir, "schema_gen.go"))
+ require.NoError(t, err)
+ gen := string(genBytes)
+ require.Contains(t, gen, "ParseFactory[Kind](kindNames[:])")
+ require.Contains(t, gen, "ParseFactory[Mode]([][]string{modeNames[0][:], modeNames[1][:]})")
+
+ testBytes, err := os.ReadFile(filepath.Join(dir, "schema_gen_test.go"))
+ require.NoError(t, err)
+ tests := string(testBytes)
+ require.Contains(t, tests, "Names: kindNames[:]")
+ require.Contains(t, tests, "Names: [][]string{modeNames[0][:], modeNames[1][:]}")
+ })
+}
+
+func TestGenerateJSONWithoutParserOrStringer(t *testing.T) {
+ const src = `package sample
+
+type State uint8
+
+const (
+ StateUnknown State = iota
+ StateReady
+)
+
+var stateNames = []string{"unknown", "ready"}
+`
+
+ dir := t.TempDir()
+ require.NoError(t, os.WriteFile(filepath.Join(dir, "schema.go"), []byte(src), 0o644))
+
+ withWorkingDirectory(t, dir, func() {
+ opts := enumify.Options{
+ File: "schema.go",
+ Pkg: "sample",
+ NoParser: true,
+ NoStringer: true,
+ NoJSON: false,
+ }
+
+ require.NoError(t, enumify.Generate(opts))
+
+ genBytes, err := os.ReadFile(filepath.Join(dir, "schema_gen.go"))
+ require.NoError(t, err)
+ gen := string(genBytes)
+ require.Contains(t, gen, "func (s State) MarshalJSON() ([]byte, error)")
+ require.Contains(t, gen, "func (s *State) UnmarshalJSON(data []byte) (err error)")
+ require.Contains(t, gen, "parse := enumify.ParseFactory[State](stateNames)")
+ require.NotContains(t, gen, "ParseState(")
+ require.NotContains(t, gen, "json.Marshal(s.String())")
+ })
+}
+
+func TestGenerateTestsAvoidsSelfImportForEnumifyPackage(t *testing.T) {
+ const src = `package enumify
+
+type Color uint8
+
+const (
+ ColorUnknown Color = iota
+ ColorBlue
+)
+
+var colorNames = []string{"unknown", "blue"}
+`
+
+ dir := t.TempDir()
+ require.NoError(t, os.WriteFile(filepath.Join(dir, "schema.go"), []byte(src), 0o644))
+
+ withWorkingDirectory(t, dir, func() {
+ opts := enumify.Options{
+ File: "schema.go",
+ Pkg: "enumify",
+ }
+
+ require.NoError(t, enumify.GenerateTests(opts))
+
+ genBytes, err := os.ReadFile(filepath.Join(dir, "schema_gen_test.go"))
+ require.NoError(t, err)
+ gen := string(genBytes)
+ require.Contains(t, gen, "package enumify")
+ require.Contains(t, gen, "suite := TestSuite[Color, []string]{")
+ require.NotContains(t, gen, "\"go.rtnl.ai/enumify\"")
+ })
+}
+
+func withWorkingDirectory(t *testing.T, dir string, fn func()) {
+ t.Helper()
+
+ cwd, err := os.Getwd()
+ require.NoError(t, err)
+
+ require.NoError(t, os.Chdir(dir))
+ t.Cleanup(func() {
+ require.NoError(t, os.Chdir(cwd))
+ })
+
+ fn()
+}
diff --git a/typedef.go b/typedef.go
--- a/typedef.go
+++ b/typedef.go
@@ -34,20 +34,38 @@
func (e *EnumType) NamesVarTypeId() *g.Statement {
switch {
- case isStringTable(e.NamesVar.Type()):
+ case isStringTable(e.NamesVar.Type()) || isStrings2DArray(e.NamesVar.Type()):
return g.Id("[][]string")
- case isStringSlice(e.NamesVar.Type()):
+ case isStringSlice(e.NamesVar.Type()) || isStringArray(e.NamesVar.Type()):
return g.Id("[]string")
default:
panic(fmt.Errorf("unsupported names variable type: %T", e.NamesVar.Type()))
}
}
+func (e *EnumType) namesVarValue() *g.Statement {
+ switch {
+ case isStringSlice(e.NamesVar.Type()) || isStringTable(e.NamesVar.Type()):
+ return e.NamesVarId()
+ case isStringArray(e.NamesVar.Type()):
+ return e.NamesVarId().Op("[:]")
+ case isStrings2DArray(e.NamesVar.Type()):
+ outer := e.NamesVar.Type().Underlying().(*types.Array)
+ rows := make([]g.Code, 0, int(outer.Len()))
+ for i := 0; i < int(outer.Len()); i++ {
+ rows = append(rows, e.NamesVarId().Index(g.Lit(i)).Op("[:]"))
+ }
+ return g.Id("[][]string").Values(rows...)
+ default:
+ panic(fmt.Errorf("unsupported names variable type: %T", e.NamesVar.Type()))
+ }
+}
+
func (e *EnumType) IndexNames(i g.Code) *g.Statement {
switch {
- case isStringTable(e.NamesVar.Type()):
+ case isStringTable(e.NamesVar.Type()) || isStrings2DArray(e.NamesVar.Type()):
return e.NamesVarId().Index(g.Lit(0)).Index(i)
- case isStringSlice(e.NamesVar.Type()):
+ case isStringSlice(e.NamesVar.Type()) || isStringArray(e.NamesVar.Type()):
return e.NamesVarId().Index(i)
default:
panic(fmt.Errorf("unsupported names variable type: %T", e.NamesVar.Type()))This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 862b2c1. Configure here.
| default: | ||
| panic(fmt.Errorf("unsupported names variable type: %T", e.NamesVar.Type())) | ||
| } | ||
| } |
There was a problem hiding this comment.
Discovery accepts array types but codegen panics on them
High Severity
The NamesVarTypeId and IndexNames functions panic when processing names variables declared as string arrays or 2D string arrays. While isNamesType correctly identifies these as valid, the code generation logic only handles string slices and string tables. This leads to crashes for users with array-based names variables, potentially breaking existing setups.
Reviewed by Cursor Bugbot for commit 862b2c1. Configure here.
| Block( | ||
| g.Return(g.Id("err")), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
UnmarshalJSON references parser not generated when NoParser set
Medium Severity
The generated UnmarshalJSON and MarshalJSON methods implicitly depend on other generated code, specifically Parse<TypeName> and String() respectively. If NoJSON is false but NoParser or NoStringer are true, the generated code will reference undefined functions, causing compilation failures.
Reviewed by Cursor Bugbot for commit 862b2c1. Configure here.
|
|
||
| // Generate the test file for the enum types. | ||
| func GenerateTests(opts Options) (err error) { | ||
| f := g.NewFile(opts.Pkg) |
There was a problem hiding this comment.
Test file generated with wrong package name suffix
Medium Severity
GenerateTests uses g.NewFile(opts.Pkg) instead of g.NewFile(opts.Pkg + "_test"), generating white-box tests in the same package. While this works for accessing unexported names vars, it means the generated test file (saved as *_gen_test.go) shares the package namespace with the source, which can cause symbol collisions with other test files in the same package that use package <name>_test (black-box testing). The old code used opts.Pkg + "_test" — if the intent was to switch to white-box tests, the generated test code also needs to avoid referencing enumify.TestSuite with a qualified import since it's now in the same compilation context as the source.
Reviewed by Cursor Bugbot for commit 862b2c1. Configure here.
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
1 participant
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.
Scope of changes
Actually generates enum code with enumify deps.
Estimated PR Size:
Acceptance criteria
This PR will be merged without review.
Author checklist
Note
Medium Risk
Medium risk because it replaces the generator entry points and emitted code shape (parser/stringer/JSON/test scaffolding), which can break downstream generated enums and their tests if discovery or name/type handling is off.
Overview
Implements actual enum code generation by introducing a
Discover-driven pipeline and rewritingGenerate/GenerateTeststo emit parser functions,String()implementations, and JSON marshal/unmarshal methods for each discovered enum type.Adds new CLI/options for
case-sensitiveandspace-sensitivethat feed into the generated test suites, plus new helper utilities (LowerFirst/UpperFirst) andEnumTypecodegen helpers to standardize identifier and names access. Updates examples to use slice-based name vars and adds new test fixtures/exports to validate supported names-variable types.Reviewed by Cursor Bugbot for commit 862b2c1. Configure here.