Handle duplicate fields and repeated child elements

element.go

@@ -206,23 +206,49 @@ 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{}{}
 
+		// 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 shouldCompact && !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, "*")
 			}
 		}
 
 		currentChild := childElement
-		if options.compactTypes {
+		if shouldCompact {
 			currentChild = firstNotContainerElement(childElement)
 		}
 		if topLevelElement, ok := options.namedTypes[currentChild.name]; ok {
@@ -234,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)
@@ -257,6 +283,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
 }

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
+}

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(
-						`<a>`,
-						`  <b/>`,
-						`  <B/>`,
-						`</a>`,
-					),
-				},
-				expectedErr: "B: duplicate field name",
+		{
+			name:    "duplicate_field_name",
+			options: []xmlstruct.GeneratorOption{},
+			xmlStrs: []string{
+				joinLines(
+					`<a>`,
+					`  <b/>`,
+					`  <B/>`,
+					`</a>`,
+				),
 			},
-		*/
+			expectedErr: "B: duplicate field name",
+		},
 		{
 			name: "duplicate_type_name",
 			options: []xmlstruct.GeneratorOption{
@@ -649,8 +646,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 +699,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\"`",
 				"}",
 			),
 		},
@@ -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(
+				`<root>`,
+				`  <single>`,
+				`    <value>unique</value>`,
+				`  </single>`,
+				`  <multi>`,
+				`    <value>first</value>`,
+				`  </multi>`,
+				`  <multi>`,
+				`    <value>second</value>`,
+				`  </multi>`,
+				`</root>`,
+			),
+			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(
+				`<root>`,
+				`  <single>`,
+				`    <value>unique</value>`,
+				`  </single>`,
+				`  <multi>`,
+				`    <value>first</value>`,
+				`  </multi>`,
+				`  <multi>`,
+				`    <value>second</value>`,
+				`  </multi>`,
+				`</root>`,
+			),
+			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()

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