BatmanAoD commented 4 years ago

Description

A suggested fix for #7. I am leaving it as a "draft" for now due to the following issues:

There are some unrelated changes
The branch name does not include the issue number
Only Go, TS, and C code-gen are modified; Rust and Java still need to be updated

Related Issue

7

Motivation and Context

This is needed for XSD files that use complexType with any character-data.

How Has This Been Tested

I updated the base64.xsd file to ensure that all types are included in a sampe XML generated with http://xsd2xml.com/. I also added a sixth type that is more similar to the example in #7. I then updated the "reference" code-generation files to match my expected output and ensured that the tests still pass.

Types of changes

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

It's kind of a toss-up whether this is a bug or a new feature.

Checklist

[x] My code follows the code style of this project.
[x] My change requires a change to the documentation.
- We should indicate that the Value field contains the character data for complex types.
[ ] I have updated the documentation accordingly.
- I can do this if the approach I've proposed is acceptable.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

alexandre-normand commented 2 years ago

I stumbled on this PR when trying to generate Go from a xsd that uses extensions. @BatmanAoD or @xuri : Any plans on getting that support added soon or how much work is left? Is there a way I can help?

For what it's worth, I naively tried running with this branch and got this error:

FATA[0000] open assets/BaseType.template: no such file or directory

xuri commented 2 years ago

Sorry I've been little busy recently, maybe need taken a while to respond.

alexandre-normand commented 2 years ago

First, thanks, @BatmanAoD for getting this PR up and getting this going!

I was able to try this partial PR after rebasing from master. At first, the xsd I was using didn't seem like a good fit for the way the generated code handles the base type when generating the extending type's struct. To make it a little easier to grasp, here's the base type definition:

         <xsd:complexType name="BaseStmtInfoType">
        <xsd:annotation>
            <xsd:documentation>the type that contains the basic statement information</xsd:documentation>
        </xsd:annotation>
        <xsd:sequence>
            <xsd:element name="StatementSetOptions" type="shp:SetOptionsType" minOccurs="0" maxOccurs="1" />
        </xsd:sequence>
        <xsd:attribute name="StatementCompId" type="xsd:int" use="optional" />
        <xsd:attribute name="StatementEstRows" type="xsd:double" use="optional" />
        <xsd:attribute name="StatementId" type="xsd:int" use="optional" />
        <xsd:attribute name="StatementOptmLevel" type="xsd:string" use="optional" />
        <xsd:attribute name="StatementOptmEarlyAbortReason" use="optional">
            <xsd:simpleType>
                <xsd:restriction base="xsd:string">
                    <xsd:enumeration value="TimeOut" />
                    <xsd:enumeration value="MemoryLimitExceeded" />
                    <xsd:enumeration value="GoodEnoughPlanFound" />
                </xsd:restriction>
            </xsd:simpleType>
        </xsd:attribute>
        <xsd:attribute name="CardinalityEstimationModelVersion" type="xsd:string" use="optional" />
        <xsd:attribute name="StatementSubTreeCost" type="xsd:double" use="optional" />
        <xsd:attribute name="StatementText" type="xsd:string" use="optional" />
        ...
    </xsd:complexType>

And here's one of the types that extends it:

    <xsd:complexType name="StmtSimpleType">
        <xsd:annotation>
            <xsd:documentation>The simple statement that may or may not contain query plan, UDF plan or Stored Procedure plan </xsd:documentation>
        </xsd:annotation>
        <xsd:complexContent>
            <xsd:extension base="shp:BaseStmtInfoType">
                <xsd:sequence>
                    <xsd:element name="QueryPlan" type="shp:QueryPlanType" minOccurs="0" maxOccurs="1" />
                    <xsd:element name="UDF" type="shp:FunctionType" minOccurs="0" maxOccurs="unbounded" />
                    <xsd:element name="StoredProc" type="shp:FunctionType" minOccurs="0" maxOccurs="1" />
                </xsd:sequence>
            </xsd:extension>
        </xsd:complexContent>
    </xsd:complexType>

In this example above, the natural way to generate the code would be to use struct embedding. So something like:

// StmtSimpleType is The simple statement that may or may not contain query plan, UDF plan or Stored Procedure plan
type StmtSimpleType struct {
    QueryPlan  *QueryPlanType  `xml:"QueryPlan"`
    UDF        []*FunctionType `xml:"UDF"`
    StoredProc *FunctionType   `xml:"StoredProc"`
    *BaseStmtInfoType
}

// BaseStmtInfoType is the type that contains the basic statement information
type BaseStmtInfoType struct {
    StatementText                     string          `xml:"StatementText,attr,omitempty"`
    StatementIDAttr                   int             `xml:"StatementId,attr,omitempty"`
    StatementCompIDAttr               int             `xml:"StatementCompId,attr,omitempty"`
    StatementType                     string          `xml:"StatementType,attr,omitempty"`
    RetrievedFromCache                string          `xml:"RetrievedFromCache,attr,omitempty"`
    StatementSubTreeCost              float64         `xml:"StatementSubTreeCost,attr,omitempty"`
    StatementEstRowsAttr              float64         `xml:"StatementEstRows,attr,omitempty"`
    SecurityPolicyApplied             bool            `xml:"SecurityPolicyApplied,attr"`
    StatementOptmLevelAttr            string          `xml:"StatementOptmLevel,attr,omitempty"`
    QueryHash                         string          `xml:"QueryHash,attr,omitempty"`
    QueryPlanHash                     string          `xml:"QueryPlanHash,attr,omitempty"`
    StatementOptmEarlyAbortReasonAttr string          `xml:"StatementOptmEarlyAbortReason,attr,omitempty"`
    StatementSetOptions               *SetOptionsType `xml:"StatementSetOptions"`
    CardinalityEstimationModelVersion string          `xml:"CardinalityEstimationModelVersion,attr,omitempty"`
        ...
}

In fact, I made that quick change here and got that working as expected and decoding xml correctly (except for a few other unrelated things that I need to look into separately).

When coming back to this PR though, I see that the base type can be a primitive type (or it looks like it?) and embedding that primitive base type (like what's be added in this PR in base64.xsd) would obviously be broken. It looks like both cases are valid and should be handled differently but I thought I'd get some feedback before jumping into it since I hadn't had to play with XML/XSDs in a few years so I don't have a lot of relevant recent experience 😅 .

November 11th update: updated the generation to conditionally generate a Value field when it's a built-in type or use embedding when it's not. You can see that commit here.

Thanks!

BatmanAoD commented 2 years ago

Superseded by #36

xuri / xgen

Generate `Value` field for types that use XSD's `extension` element #18