From 42eae735cdb527587d9398bf559cb2e10b9d0961 Mon Sep 17 00:00:00 2001 From: Mikheil Maisuradze <28116494+hacks1ash@users.noreply.github.com> Date: Fri, 17 Apr 2026 14:47:50 +0400 Subject: [PATCH 1/2] Handle proto optional fields: add GetterReturnsPtr to matcher, adjust generator logic, and introduce integration test. --- integration_test.go | 143 ++++++++++++++++++++++++++++++++ internal/generator/generator.go | 22 ++++- internal/matcher/matcher.go | 8 ++ 3 files changed, 171 insertions(+), 2 deletions(-) diff --git a/integration_test.go b/integration_test.go index f378ff8..2af87a6 100644 --- a/integration_test.go +++ b/integration_test.go @@ -5,6 +5,7 @@ import ( "go/parser" "go/token" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -1140,6 +1141,148 @@ type LocalUser struct { }) } +// TestSuite4_EndToEnd_CrossPackage_ProtoOptionalFields verifies that proto +// optional fields (pointer field + value getter) generate correct assignments. +func TestSuite4_EndToEnd_CrossPackage_ProtoOptionalFields(t *testing.T) { + tmpDir := t.TempDir() + modName := "example.com/prototest" + + gomod := "module " + modName + "\n\ngo 1.21\n" + if err := os.WriteFile(filepath.Join(tmpDir, "go.mod"), []byte(gomod), 0644); err != nil { + t.Fatalf("writing go.mod: %v", err) + } + + // Proto-style package with optional (pointer) fields and value-returning getters. + extPkgDir := filepath.Join(tmpDir, "pb") + if err := os.MkdirAll(extPkgDir, 0755); err != nil { + t.Fatalf("creating pb dir: %v", err) + } + + pbSrc := `package pb + +type ProtoMessage struct { + Name *string ` + "`json:\"name\"`" + ` + Country *string ` + "`json:\"country\"`" + ` + Age *int ` + "`json:\"age\"`" + ` +} + +func (x *ProtoMessage) GetName() string { + if x != nil && x.Name != nil { + return *x.Name + } + return "" +} + +func (x *ProtoMessage) GetCountry() string { + if x != nil && x.Country != nil { + return *x.Country + } + return "" +} + +func (x *ProtoMessage) GetAge() int { + if x != nil && x.Age != nil { + return *x.Age + } + return 0 +} +` + if err := os.WriteFile(filepath.Join(extPkgDir, "message.go"), []byte(pbSrc), 0644); err != nil { + t.Fatalf("writing pb/message.go: %v", err) + } + + appPkgDir := filepath.Join(tmpDir, "app") + if err := os.MkdirAll(appPkgDir, 0755); err != nil { + t.Fatalf("creating app dir: %v", err) + } + + // Local struct with value fields (common case: proto *T -> local T). + appSrc := `package app + +type LocalModel struct { + Name string ` + "`json:\"name\" mapper:\"bind:Name\"`" + ` + Country *string ` + "`json:\"country\" mapper:\"bind:Country\"`" + ` + Age int ` + "`json:\"age\" mapper:\"bind:Age\"`" + ` +} +` + if err := os.WriteFile(filepath.Join(appPkgDir, "model.go"), []byte(appSrc), 0644); err != nil { + t.Fatalf("writing app/model.go: %v", err) + } + + appPctx, err := loader.LoadPackage(appPkgDir) + if err != nil { + t.Fatalf("LoadPackage(app): %v", err) + } + + extPctx, err := loader.LoadExternalPackage(tmpDir, modName+"/pb") + if err != nil { + t.Fatalf("LoadExternalPackage(pb): %v", err) + } + + localModel, err := loader.LoadStructFromPkg(appPctx, "LocalModel") + if err != nil { + t.Fatalf("load LocalModel: %v", err) + } + + protoMsg, err := loader.LoadStructFromPkg(extPctx, "ProtoMessage") + if err != nil { + t.Fatalf("load ProtoMessage: %v", err) + } + + getters := loader.DiscoverGetters(extPctx, "ProtoMessage") + + crossResult := matcher.MatchCross(localModel, protoMsg, getters) + + ccfg := generator.CrossConfig{ + PackageName: "app", + InternalType: "LocalModel", + ExternalType: "ProtoMessage", + ExternalPkgName: "pb", + ExternalPkgPath: modName + "/pb", + ToExternalFuncName: "MapLocalModelToProtoMessage", + FromExternalFuncName: "MapProtoMessageToLocalModel", + ToExternalPairs: crossResult.ToExternal.Pairs, + FromExternalPairs: crossResult.FromExternal.Pairs, + Bidirectional: true, + } + + code, err := generator.GenerateCross(ccfg) + if err != nil { + t.Fatalf("GenerateCross: %v", err) + } + + output := string(code) + + t.Run("parses_as_valid_go", func(t *testing.T) { + parseAndVerify(t, code) + }) + + // Proto *string -> local string: getter returns string, so direct assignment (NoneConversion). + t.Run("proto_ptr_to_local_value_uses_getter_no_deref", func(t *testing.T) { + assertContains(t, output, "dst.Name = src.GetName()") + assertContains(t, output, "dst.Age = src.GetAge()") + }) + + // Proto *string -> local *string: skip getter, use direct field access to preserve nil. + t.Run("proto_ptr_to_local_ptr_skips_getter_for_nil_safety", func(t *testing.T) { + assertContains(t, output, "dst.Country = src.Country") + }) + + // Write and verify the file compiles. + genPath := filepath.Join(appPkgDir, "mapper_gen.go") + if err := os.WriteFile(genPath, code, 0644); err != nil { + t.Fatalf("writing generated file: %v", err) + } + + t.Run("generated_code_compiles", func(t *testing.T) { + cmd := exec.Command("go", "build", "./...") + cmd.Dir = tmpDir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("generated code does not compile:\n%s\n%s", out, output) + } + }) +} + func TestSuite4_EndToEnd_MatcherFieldVerification(t *testing.T) { // End-to-end test that uses LoadPackage on testdata/integration // and verifies all match behavior using real loaded types. diff --git a/internal/generator/generator.go b/internal/generator/generator.go index 8c2734b..4f065f4 100644 --- a/internal/generator/generator.go +++ b/internal/generator/generator.go @@ -183,7 +183,11 @@ func renderCrossFunc(buf *bytes.Buffer, funcName, srcType, dstType string, pairs srcExpr := "src." + pair.Src.Name // Use getter if available and we are reading from external struct. - if useGetters && pair.UseGetter && pair.GetterName != "" { + // Skip getter when it returns a value type but the destination needs + // a pointer — direct field access preserves nil semantics. + useGetter := useGetters && pair.UseGetter && pair.GetterName != "" && + !(pair.Src.IsPtr && !pair.GetterReturnsPtr && pair.Dst.IsPtr) + if useGetter { srcExpr = "src." + pair.GetterName + "()" } @@ -211,6 +215,16 @@ func renderCrossFunc(buf *bytes.Buffer, funcName, srcType, dstType string, pairs } conv := pair.Conversion() + + // When using a getter that returns a value type for a pointer field + // (e.g., proto GetCountry() string for Country *string), adjust the + // conversion since the getter already strips the pointer. + if useGetter && pair.Src.IsPtr && !pair.GetterReturnsPtr { + if conv == matcher.DerefConversion { // *T → T, but getter gives T → T + conv = matcher.NoneConversion + } + } + switch conv { case matcher.NoneConversion: fmt.Fprintf(buf, "\tdst.%s = %s\n", pair.Dst.Name, srcExpr) @@ -219,8 +233,12 @@ func renderCrossFunc(buf *bytes.Buffer, funcName, srcType, dstType string, pairs fmt.Fprintf(buf, "\t\tdst.%s = *%s\n", pair.Dst.Name, srcExpr) fmt.Fprintf(buf, "\t}\n") case matcher.AddrConversion: + elemType := pair.Src.ElemType + if pair.Dst.ElemType != "" { + elemType = pair.Dst.ElemType + } fmt.Fprintf(buf, "\tdst.%s = func() *%s { v := %s; return &v }()\n", - pair.Dst.Name, pair.Src.ElemType, srcExpr) + pair.Dst.Name, elemType, srcExpr) } } diff --git a/internal/matcher/matcher.go b/internal/matcher/matcher.go index a0920b2..bbee3c1 100644 --- a/internal/matcher/matcher.go +++ b/internal/matcher/matcher.go @@ -8,6 +8,8 @@ package matcher import ( + "strings" + "github.com/hacks1ash/goxmap/internal/loader" ) @@ -43,6 +45,11 @@ type FieldPair struct { TypeCast bool // CastTypeName is the destination type name for named type casts. CastTypeName string + + // GetterReturnsPtr indicates whether the getter method returns a pointer type. + // For proto optional fields, the field is *T but the getter returns T. + // This is used to adjust pointer conversion when generating code with getters. + GetterReturnsPtr bool } // PointerConversion describes how to handle pointer differences between matched fields. @@ -359,6 +366,7 @@ func MatchCross(internal, external *loader.StructInfo, getters map[string]loader if gi, ok := getters[extField.Name]; ok { fromExtPair.UseGetter = true fromExtPair.GetterName = gi.MethodName + fromExtPair.GetterReturnsPtr = strings.HasPrefix(gi.ReturnType, "*") } } From 137b36f1c9687be881636f38b5137d06d5855483 Mon Sep 17 00:00:00 2001 From: Mikheil Maisuradze <28116494+hacks1ash@users.noreply.github.com> Date: Fri, 17 Apr 2026 14:52:24 +0400 Subject: [PATCH 2/2] Fix generator logic to correctly handle getter usage conditions --- internal/generator/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/generator/generator.go b/internal/generator/generator.go index 4f065f4..2542853 100644 --- a/internal/generator/generator.go +++ b/internal/generator/generator.go @@ -186,7 +186,7 @@ func renderCrossFunc(buf *bytes.Buffer, funcName, srcType, dstType string, pairs // Skip getter when it returns a value type but the destination needs // a pointer — direct field access preserves nil semantics. useGetter := useGetters && pair.UseGetter && pair.GetterName != "" && - !(pair.Src.IsPtr && !pair.GetterReturnsPtr && pair.Dst.IsPtr) + (!pair.Src.IsPtr || pair.GetterReturnsPtr || !pair.Dst.IsPtr) if useGetter { srcExpr = "src." + pair.GetterName + "()" }