Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This closes #88, simpleType causing issues in the generated code #89

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

GiovanePS
Copy link

PR Details

Description

The line in the outer position of 'if' causes the simpleType element to be set to CurrentEle every time it is parsed. However, simpleType is not always the parent of all nested elements. If I have the following relationship as an example:

<element name="ElementWithSimpleType">
    <simpleType>
        <restriction base="string" />
    </simpleType>
</element>

then CurrentEle should be the element, not "simpleType". The attribute name defines the correct parent.

With this, the problem occurs when EndSimpleType is called, because CurrentEle is set to "simpleType", which it shouldn't be, incorrectly adding this simpleType element to ProtoTree.

Related Issue

#88

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
The line in the outer position of 'if' causes the simpleType element to
be set to CurrentEle every time it is parsed. However, simpleType is not
always the parent of all nested elements. If I have the following
relationship: <element> -> <simpleType> -> [some element], then
CurrentEle should be the element, not simpleType.
@GiovanePS
Copy link
Author

GiovanePS commented Mar 7, 2025

Good to say that, with this change, the output of the minimal XSD will be as follows:

// Code generated by xgen. DO NOT EDIT.

package schema

// SomeComplexType ...
type SomeComplexType struct {
	FirstElement               int                         `xml:"FirstElement"`
	ElementWithSimpleType      *ElementWithSimpleType      `xml:"ElementWithSimpleType"`
	OtherElementWithSimpleType *OtherElementWithSimpleType `xml:"OtherElementWithSimpleType"`
	ElementWithoutSimpleType   int                         `xml:"ElementWithoutSimpleType"`
}

Which is not the same as my expected output in the issue. (#88)

This is because the issue related to the types parsing in the code shown is related to an another issue in the restriction element.

I'll send a PR to address this.

@GiovanePS
Copy link
Author

GiovanePS commented Mar 9, 2025

This is related with restriction because this behaviour:

<xs:schema
	xmlns:xs="http://www.w3.org/2001/XMLSchema"
	targetNamespace="http://example.com/schemas"
	xmlns:xi="http://example.com/schemas"
	elementFormDefault="qualified">

	<xs:complexType name="SomeComplexType">
		<xs:sequence>
			<xs:element name="SomeElement">
				<xs:simpleType>
					<xs:restriction base="xs:string">
						<xs:minLength value = "4" fixed = "true" />
					</xs:restriction>
				</xs:simpleType>
			</xs:element>
		</xs:sequence>
	</xs:complexType>
</xs:schema>

With the above XSD, putting on xgen, we get this output:

// Code generated by xgen. DO NOT EDIT.

package schema

// SomeComplexType ...
type SomeComplexType struct {
	SomeElement string `xml:"SomeElement"`
}

Which is such correct. The problem comes when the minLength is removed (which is acceptable, happens a lot that restriction doesn't have a child). After commenting the minLenght line, this is the output that we've received:

// Code generated by xgen. DO NOT EDIT.

package schema

// SomeComplexType ...
type SomeComplexType struct {
	SomeElement *SomeElement `xml:"SomeElement"`
}

The SomeElement should be of the type string not a pointer to an itself type.

To solve this, after our initial change, we can simply add this chunk of code in xmlRestriction.go (like I did in 3f0bfd5):

if opt.Element.Len() > 0 {
	opt.Element.Peek().(*Element).Type, err = opt.GetValueType(valueType, protoTree)
	return
}

This occurs because the implementation of the restriction methods commonly waits a child element, like minLenght, maxLenght, minExclusive, maxExclusive, etc. And, syntactically, a restriction element must be a child of a simpleType element. With this, the types are manipulated by the OnRestriction/EndRestriction and the Lenght/Inclusive/Exclusive/etc. methods. But, with these changes, an element can properly be the actual parent of this relationship, which results in this output if that line on minLength is uncommented:

// Code generated by xgen. DO NOT EDIT.

package schema

// SomeComplexType ...
type SomeComplexType struct {
	SomeElement interface{} `xml:"SomeElement"`
}

This is the actual unexpected behaviour. It occurs because the code was made around this, not solving the problem, just putting it off.

The correct way to solve this is just remove all the following files:

  • xmlLength.go
  • xmlMinLength.go
  • xmlMaxLength.go
  • xmlMinExclusive.go
  • xmlMaxExclusive.go
  • xmlMinInclusive.go
  • xmlMaxInclusive.go
  • xmlPattern.go
  • xmlWhiteSpace.go
  • xmlFractionDigits.go
  • xmlTotalDigits.go

With these changes, there is no reason to their existence. They all makes the same thing: solve the problem that was created before.

After all, there is no issues with this elements. At least with all XSDs that I've tested. I didn't send any commit about all these deletions, because it is just it: delete them all.

This might be related with #80 and #63 issues.

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.

None yet

1 participant