From fc6dd5c83c96b4ff1d13dd96536758cb3f737171 Mon Sep 17 00:00:00 2001
From: Meir Blumenfeld <33358938+meblum@users.noreply.github.com>
Date: Mon, 11 Aug 2025 22:19:31 -0400
Subject: [PATCH 1/2] fix: `WithCompactTypes` generate repeated child
elements as slice
Fixes #43
---
element.go | 48 +++++++++++++++++++++++++++++++++++++++++++++--
generator_test.go | 8 ++++----
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/element.go b/element.go
index 65524da..dcaabce 100644
--- a/element.go
+++ b/element.go
@@ -212,11 +212,23 @@ func (e *element) writeGoType(w io.Writer, options *generateOptions, indentPrefi
}
fieldNames[exportedChildName] = struct{}{}
+ // Check for repeated/optional, considering compact types path.
+ _, repeated := e.repeatedChildren[childElement.name]
+ _, optional := e.optionalChildren[childElement.name]
+
+ // For compact types, also check if any element along the compact path is repeated.
+ if options.compactTypes && !repeated {
+ targetChild := firstNotContainerElement(childElement)
+ if targetChild != childElement {
+ repeated = isRepeatedInCompactPath(childElement, targetChild)
+ }
+ }
+
fmt.Fprintf(w, "%s\t%s ", indentPrefix, exportedChildName)
- if _, repeated := e.repeatedChildren[childElement.name]; repeated {
+ if repeated {
fmt.Fprintf(w, "[]")
} else if options.usePointersForOptionalFields {
- if _, optional := e.optionalChildren[childElement.name]; optional {
+ if optional {
fmt.Fprintf(w, "*")
}
}
@@ -257,6 +269,38 @@ func firstNotContainerElement(el *element) *element {
return el
}
+// isRepeatedInCompactPath checks if any element along the compact path from start to target is repeated.
+func isRepeatedInCompactPath(start, target *element) bool {
+ current := start
+ for current != target && current.isContainer() {
+ for childName, childElement := range current.childElements {
+ if _, repeated := current.repeatedChildren[childName]; repeated {
+ return true
+ }
+ if childElement == target || isAncestorOf(childElement, target) {
+ current = childElement
+ break
+ }
+ }
+ }
+ return false
+}
+
+// isAncestorOf checks if ancestor is an ancestor of descendant in the element tree.
+func isAncestorOf(ancestor, descendant *element) bool {
+ if ancestor == descendant {
+ return true
+ }
+ if ancestor.isContainer() {
+ for _, child := range ancestor.childElements {
+ if isAncestorOf(child, descendant) {
+ return true
+ }
+ }
+ }
+ return false
+}
+
func exportedName(el *element, options *generateOptions) string {
return exportedNameWithoutSuffix(el, options) + options.elemNameSuffix
}
diff --git a/generator_test.go b/generator_test.go
index 0f7e898..712e974 100644
--- a/generator_test.go
+++ b/generator_test.go
@@ -649,8 +649,8 @@ func TestGenerator(t *testing.T) {
` C struct {`,
"\t\t\tCharData string `xml:\",chardata\"`",
` D struct {`,
- "\t\t\t\tE struct{} `xml:\"e\"`",
- "\t\t\t\tG struct{} `xml:\"f>g\"`",
+ "\t\t\t\tE struct{} `xml:\"e\"`",
+ "\t\t\t\tG []struct{} `xml:\"f>g\"`",
"\t\t\t} `xml:\"d\"`",
"\t\t} `xml:\"c\"`",
"\t} `xml:\"b\"`",
@@ -702,8 +702,8 @@ func TestGenerator(t *testing.T) {
"}",
"",
"type D struct {",
- "\tE struct{} `xml:\"e\"`",
- "\tG struct{} `xml:\"f>g\"`",
+ "\tE struct{} `xml:\"e\"`",
+ "\tG []struct{} `xml:\"f>g\"`",
"}",
),
},
From fd23891df2802d171c21f2677813fca9ee5ac946 Mon Sep 17 00:00:00 2001
From: Meir Blumenfeld <33358938+meblum@users.noreply.github.com>
Date: Tue, 12 Aug 2025 00:14:10 -0400
Subject: [PATCH 2/2] fix: handle duplicate fields
---
element.go | 24 +++++++++---
generator.go | 43 ++++++++++++++++++++-
generator_test.go | 96 +++++++++++++++++++++++++++++++++++++++--------
xmlstruct.go | 1 +
4 files changed, 143 insertions(+), 21 deletions(-)
diff --git a/element.go b/element.go
index dcaabce..1c7d2a2 100644
--- a/element.go
+++ b/element.go
@@ -206,9 +206,23 @@ func (e *element) writeGoType(w io.Writer, options *generateOptions, indentPrefi
}
for _, childElement := range childElements {
- exportedChildName := exportedName(childElement, options)
+ // Decide whether to use compact types for this specific element
+ shouldCompact := options.compactTypes &&
+ childElement.isContainer() &&
+ !options.nonCompactableElements[childElement.name]
+
+ // Create options for this element with appropriate compact setting
+ elementOptions := *options
+ if !shouldCompact {
+ elementOptions.compactTypes = false
+ }
+ exportedChildName := exportedName(childElement, &elementOptions)
+
if _, ok := fieldNames[exportedChildName]; ok {
- fieldNames[exportedChildName] = struct{}{}
+ // Only report field name conflicts if we're not using named types for this element
+ if _, hasNamedType := options.namedTypes[childElement.name]; !hasNamedType {
+ return fmt.Errorf("%s: duplicate field name", exportedChildName)
+ }
}
fieldNames[exportedChildName] = struct{}{}
@@ -217,7 +231,7 @@ func (e *element) writeGoType(w io.Writer, options *generateOptions, indentPrefi
_, optional := e.optionalChildren[childElement.name]
// For compact types, also check if any element along the compact path is repeated.
- if options.compactTypes && !repeated {
+ if shouldCompact && !repeated {
targetChild := firstNotContainerElement(childElement)
if targetChild != childElement {
repeated = isRepeatedInCompactPath(childElement, targetChild)
@@ -234,7 +248,7 @@ func (e *element) writeGoType(w io.Writer, options *generateOptions, indentPrefi
}
currentChild := childElement
- if options.compactTypes {
+ if shouldCompact {
currentChild = firstNotContainerElement(childElement)
}
if topLevelElement, ok := options.namedTypes[currentChild.name]; ok {
@@ -246,7 +260,7 @@ func (e *element) writeGoType(w io.Writer, options *generateOptions, indentPrefi
return err
}
}
- fmt.Fprintf(w, " `xml:\"%s\"`\n", attrName(childElement, options.compactTypes))
+ fmt.Fprintf(w, " `xml:\"%s\"`\n", attrName(childElement, shouldCompact))
}
fmt.Fprintf(w, "%s}", indentPrefix)
diff --git a/generator.go b/generator.go
index f8274d3..4125ef8 100644
--- a/generator.go
+++ b/generator.go
@@ -282,11 +282,19 @@ func (g *Generator) Generate() ([]byte, error) {
options.importPackageNames["encoding/xml"] = struct{}{}
}
+ // When compact types is enabled, detect elements that would cause conflicts if compacted
+ nonCompactableElements := make(map[xml.Name]bool)
+ if options.compactTypes {
+ nonCompactableElements = g.detectCompactConflicts()
+ }
+ options.nonCompactableElements = nonCompactableElements
+
var typeElements []*element
if g.namedTypes {
options.namedTypes = make(map[xml.Name]*element)
for k, v := range g.typeElements {
- if !options.compactTypes || !v.isContainer() || v.root {
+ shouldCompact := options.compactTypes && v.isContainer() && !nonCompactableElements[k] && !v.root
+ if !shouldCompact || v.root {
options.namedTypes[k] = v
}
}
@@ -500,3 +508,36 @@ FOR:
}
}
}
+
+// detectCompactConflicts detects elements that would cause field name conflicts if compacted.
+func (g *Generator) detectCompactConflicts() map[xml.Name]bool {
+ nonCompactable := make(map[xml.Name]bool)
+
+ // Check each parent element for potential conflicts among its children
+ for _, parentElement := range g.typeElements {
+ if len(parentElement.childElements) <= 1 {
+ continue
+ }
+
+ // Collect what the field names would be if each child were compacted
+ fieldNames := make(map[string][]xml.Name)
+ for childName, childElement := range parentElement.childElements {
+ if childElement.isContainer() {
+ // Get the name that would result from compacting
+ compactedName := g.exportNameFunc(firstNotContainerElement(childElement).name)
+ fieldNames[compactedName] = append(fieldNames[compactedName], childName)
+ }
+ }
+
+ // Mark elements as non-compactable if they would cause conflicts
+ for _, childNames := range fieldNames {
+ if len(childNames) > 1 {
+ for _, childName := range childNames {
+ nonCompactable[childName] = true
+ }
+ }
+ }
+ }
+
+ return nonCompactable
+}
diff --git a/generator_test.go b/generator_test.go
index 712e974..61303c9 100644
--- a/generator_test.go
+++ b/generator_test.go
@@ -352,22 +352,19 @@ func TestGenerator(t *testing.T) {
`}`,
),
},
- // FIXME make the following test pass
- /*
- {
- name: "duplicate_field_name",
- options: []xmlstruct.GeneratorOption{},
- xmlStrs: []string{
- joinLines(
- ``,
- ` `,
- ` `,
- ``,
- ),
- },
- expectedErr: "B: duplicate field name",
+ {
+ name: "duplicate_field_name",
+ options: []xmlstruct.GeneratorOption{},
+ xmlStrs: []string{
+ joinLines(
+ ``,
+ ` `,
+ ` `,
+ ``,
+ ),
},
- */
+ expectedErr: "B: duplicate field name",
+ },
{
name: "duplicate_type_name",
options: []xmlstruct.GeneratorOption{
@@ -834,6 +831,75 @@ func TestGenerator(t *testing.T) {
"}",
),
},
+ {
+ name: "compact_types_duplicated_field_names",
+ options: []xmlstruct.GeneratorOption{
+ xmlstruct.WithImports(false),
+ xmlstruct.WithPackageName(""),
+ xmlstruct.WithHeader(""),
+ xmlstruct.WithCompactTypes(true),
+ },
+ xmlStr: joinLines(
+ ``,
+ ` `,
+ ` unique`,
+ ` `,
+ ` `,
+ ` first`,
+ ` `,
+ ` `,
+ ` second`,
+ ` `,
+ ``,
+ ),
+ expectedStr: joinLines(
+ "type Root struct {",
+ "\tSingle struct {",
+ "\t\tValue string `xml:\"value\"`",
+ "\t} `xml:\"single\"`",
+ "\tMulti []struct {",
+ "\t\tValue string `xml:\"value\"`",
+ "\t} `xml:\"multi\"`",
+ "}",
+ ),
+ },
+ {
+ name: "compact_named_types_duplicated_field_names",
+ options: []xmlstruct.GeneratorOption{
+ xmlstruct.WithImports(false),
+ xmlstruct.WithPackageName(""),
+ xmlstruct.WithHeader(""),
+ xmlstruct.WithCompactTypes(true),
+ xmlstruct.WithNamedTypes(true),
+ },
+ xmlStr: joinLines(
+ ``,
+ ` `,
+ ` unique`,
+ ` `,
+ ` `,
+ ` first`,
+ ` `,
+ ` `,
+ ` second`,
+ ` `,
+ ``,
+ ),
+ expectedStr: joinLines(
+ "type Multi struct {",
+ "\tValue string `xml:\"value\"`",
+ "}",
+ "",
+ "type Root struct {",
+ "\tSingle Single `xml:\"single\"`", //nolint:dupword
+ "\tMulti []Multi `xml:\"multi\"`",
+ "}",
+ "",
+ "type Single struct {",
+ "\tValue string `xml:\"value\"`",
+ "}",
+ ),
+ },
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
diff --git a/xmlstruct.go b/xmlstruct.go
index 3719c5d..4654b39 100644
--- a/xmlstruct.go
+++ b/xmlstruct.go
@@ -127,6 +127,7 @@ type generateOptions struct {
namedRoot bool
namedTypes map[xml.Name]*element
compactTypes bool
+ nonCompactableElements map[xml.Name]bool
preserveOrder bool
simpleTypes map[xml.Name]struct{}
usePointersForOptionalFields bool