Skip to content

Create a schema for the LinkAttribute XML files #2500

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.

Sign up for GitHub

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

Merged
jtschuster merged 19 commits into from
Jan 22, 2022
Merged

Create a schema for the LinkAttribute XML files #2500

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8cc539b
Initial Schema work
jtschuster Jan 12, 2022
abac411
Finalize schema so all test xml is valid
jtschuster Jan 12, 2022
5f3bdad
Add restrictions on some attributes and change uses of "nothing" to
jtschuster Jan 12, 2022
79ae2c2
Initial Schema work
jtschuster Jan 12, 2022
ed7b9e9
Finalize schema so all test xml is valid
jtschuster Jan 12, 2022
f837645
Add restrictions on some attributes and change uses of "nothing" to
jtschuster Jan 12, 2022
437f5cd
Remove schema reference in generated file expectation
jtschuster Jan 12, 2022
3ebd1fb
merge with main
jtschuster Jan 13, 2022
b142870
Adds a more restrictive schema for LinkAttributes and updates paths for
jtschuster Jan 13, 2022
8f200b1
Update docs to use 'none' instead of 'nothing'
jtschuster Jan 13, 2022
6838288
remove schema from xml expectation file
jtschuster Jan 13, 2022
6f5eb8f
remove namespace from xml schema location
jtschuster Jan 13, 2022
a1c1033
rename 'typechild' and allow multiple nested types
jtschuster Jan 13, 2022
c8e9f4b
remove <type> option from <linker> in LinkAttributes
jtschuster Jan 13, 2022
4c0cb95
Remove ILLink.xsd
jtschuster Jan 19, 2022
f70da3d
Add more restraints on xml schema
jtschuster Jan 19, 2022
fe36b38
Rename "none" to "nothing" in CreateRuntimeRootILLinkDescriptorFile
jtschuster Jan 19, 2022
9637260
change docs back to using preserve='nothing'
jtschuster Jan 19, 2022
504e4a2
Merge branch 'main' into XmlSchema
jtschuster Jan 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/data-formats.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ The `required` attribute specifies that if the type is not marked, during the ma
<type fullname="Assembly.B" />

<!-- Preserve the type declaration only -->
<type fullname="Assembly.C" preserve="nothing" />
<type fullname="Assembly.C" preserve="none" />

<!-- Preserve a nested type -->
<type fullname="Assembly.D/Nested" preserve="all" />
Expand Down
161 changes: 161 additions & 0 deletions src/ILLink.Shared/ILLink.LinkAttributes.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
<?xml version="1.0" encoding="utf-8"?>
<xsd:schema attributeFormDefault="unqualified"
elementFormDefault="unqualified"
xmlns:xsd="http://www.w3.org/2001/XMLSchema">

<xsd:complexType name="typechild">
Comment thread
jtschuster marked this conversation as resolved.
Outdated
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute"></xsd:element>
</xsd:choice>
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
<xsd:attribute name="name" />
<xsd:attribute name="signature" />
</xsd:complexType>

<xsd:element name="method">
<xsd:complexType>
<xsd:complexContent>
<xsd:extension base="typechild">
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element name="parameter">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute"></xsd:element>
</xsd:choice>
<xsd:attribute name="name" />
</xsd:complexType>
</xsd:element>
</xsd:choice>
<xsd:choice maxOccurs="1" minOccurs="0">
<xsd:element name="return">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute"></xsd:element>
</xsd:choice>
</xsd:complexType>
</xsd:element>
</xsd:choice>
</xsd:choice>
</xsd:extension>
</xsd:complexContent>
</xsd:complexType>
</xsd:element>

<xsd:element name="event">
<xsd:complexType>
<xsd:complexContent>
<xsd:extension base="typechild" />
</xsd:complexContent>
</xsd:complexType>
</xsd:element>

<xsd:element name="field">
<xsd:complexType>
<xsd:complexContent>
<xsd:extension base="typechild" />
</xsd:complexContent>
</xsd:complexType>
</xsd:element>

<xsd:element name="property">
<xsd:complexType>
<xsd:complexContent>
<xsd:extension base="typechild" />
</xsd:complexContent>
</xsd:complexType>
</xsd:element>

<xsd:element name="argument">
<xsd:complexType mixed="true">
<xsd:choice minOccurs="0" maxOccurs="1">
<xsd:element name="argument" type="xsd:string"/>
</xsd:choice>
<xsd:attribute name="type" />
</xsd:complexType>
</xsd:element>

<xsd:element name="attribute">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="argument" />
<xsd:element minOccurs="0" maxOccurs="unbounded" name="property">
<xsd:complexType mixed="true">
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute"></xsd:element>
</xsd:choice>
<xsd:attribute name="name" />
</xsd:complexType>
</xsd:element>
</xsd:choice>
<xsd:attribute name="internal" type="xsd:string" />
<xsd:attribute name="fullname" type="xsd:string" />
<xsd:attribute name="assembly" type="xsd:string" />
</xsd:complexType>
</xsd:element>

<xsd:element name="namespace">
<xsd:complexType>
<xsd:attribute name="fullname"/>
</xsd:complexType>
</xsd:element>

<xsd:element name="assembly">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute" />
<xsd:element ref="type" />
<xsd:element ref="namespace" />
</xsd:choice>
<xsd:attribute name="fullname" type="xsd:string" use="required" />
<xsd:attribute name="name" />
<xsd:attribute name="signature" />
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
</xsd:complexType>
</xsd:element>

<xsd:element name="type">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute"></xsd:element>
<xsd:element ref="event" />
<xsd:element ref="method" />
<xsd:element ref="field" />
<xsd:element ref="property" />
<xsd:element name="type">
<xsd:complexType>
<xsd:choice minOccurs="0" maxOccurs="unbounded">
<xsd:element ref="event" />
<xsd:element ref="method" />
<xsd:element ref="field" />
<xsd:element ref="property" />
<xsd:element ref="attribute"></xsd:element>
</xsd:choice>
<xsd:attribute name="name" type="xsd:string" />
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
</xsd:complexType>
Comment thread
jtschuster marked this conversation as resolved.
Outdated
</xsd:element>
</xsd:choice>
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
<xsd:attribute name="fullname" type="xsd:string" use="required"/>
</xsd:complexType>
</xsd:element>

<xsd:element name="linker">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="assembly" />
<xsd:element ref="type" />
</xsd:choice>
</xsd:complexType>
</xsd:element>

</xsd:schema>
6 changes: 6 additions & 0 deletions src/ILLink.Shared/ILLink.Shared.projitems
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,10 @@
<SubType>Designer</SubType>
</EmbeddedResource>
</ItemGroup>
<ItemGroup>
<None Include="$(MSBuildThisFileDirectory)ILLink.LinkAttributes.xsd">
<SubType>Designer</SubType>
</None>
<None Include="$(MSBuildThisFileDirectory)ILLink.xsd" />
</ItemGroup>
</Project>
193 changes: 193 additions & 0 deletions src/ILLink.Shared/ILLink.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
<?xml version="1.0" encoding="utf-8"?>

@vitek-karas vitek-karas Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename the file to ILLink.Descriptor.xsd

@jtschuster jtschuster Jan 13, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this schema broad enough to cover ILLink.Substitutions.xml as well as ILLink.Descriptor.xml and ILLink.LinkAttributes.xml, so I don't want to name it just for descriptors, but if we don't like ILLink.xsd, maybe something like ILLink.General.xsd?

@vitek-karas vitek-karas Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - I didn't see it there - does the ILLink.Attributes.xsd "inherit/include" the ILLink.xsd then?
I would probably call it ILLink.Common.xsd, but ILLink.General.xsd works as well.

@MichalStrehovsky MichalStrehovsky Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of the schema? If we would like to give this to developers so that they have IntelliSense, having the schema suggest invalid values has potential to create harm (if there's no IntelliSense, one is forced to read docs; with IntelliSense, one relies on that for things that look "obvious").

Example: ILLink.Substitutions.xml will not look at name, only accepts signature for method. ILLink.Descriptors on the other hand will accept both name and signature and one can use it to preserve multiple methods (<method name="Foo" signature="System.Int32 Bar()"> will preserve all methods named Foo and the one Bar overload).

@jtschuster jtschuster Jan 18, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one I started as a form of documentation before I realized how different each xml file is. I think that's a good point, this schema is a little too general to provide any value. I'll remove this file and only do the more specific schema.

<xsd:schema attributeFormDefault="unqualified"
elementFormDefault="unqualified"
xmlns:xsd="http://www.w3.org/2001/XMLSchema">

<xsd:element name="method">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute" />
<xsd:element name="parameter">
<xsd:complexType>
<xsd:choice minOccurs="0" maxOccurs="unbounded">
<xsd:element ref="attribute" />
</xsd:choice>
<xsd:attribute name="name" />
</xsd:complexType>
</xsd:element>
<xsd:element name="return">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute" />
</xsd:choice>
</xsd:complexType>
</xsd:element>
</xsd:choice>
<xsd:attribute name="name" type="xsd:string" />
<xsd:attribute name="signature" type="xsd:string" />
<xsd:attribute name="required" type="xsd:boolean" />
<xsd:attribute name="body" type="xsd:string" />

@MichalStrehovsky MichalStrehovsky Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We could restrict this to the two supported values

<xsd:attribute name="value" type="xsd:string" />
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
</xsd:complexType>
</xsd:element>

<xsd:element name="event">
<xsd:complexType>
<xsd:attribute name="name" type="xsd:string" />
<xsd:attribute name="signature" type="xsd:string" />
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
</xsd:complexType>
</xsd:element>

<xsd:element name="field">
<xsd:complexType>
<xsd:sequence>
<xsd:element minOccurs="0" maxOccurs="unbounded" ref="attribute" />
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" />
<xsd:attribute name="signature" type="xsd:string" />
<xsd:attribute name="value" type="xsd:string" />
<xsd:attribute name="initialize" type="xsd:string" />

@MichalStrehovsky MichalStrehovsky Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this bool?

In the parser, this has the same problem as preserve: value of true means "initialize", value of anything else means "don't initialize", but maybe we can cut that stuff off.

<xsd:attribute name="required" type="xsd:boolean" />
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
</xsd:complexType>
</xsd:element>

<xsd:element name="property">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="attribute" />
</xsd:choice>
<xsd:attribute name="name" type="xsd:string"/>
<xsd:attribute name="signature" type="xsd:string" />
<xsd:attribute name="required" type="xsd:boolean" />
<xsd:attribute name="accessors" type="accessorvalues" />
<xsd:attribute name="type" type="xsd:string" />
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
</xsd:complexType>
</xsd:element>

<xsd:element name="resource">
<xsd:complexType>
<xsd:attribute name="name" use="required" />
<xsd:attribute name="action" use="required" />

@MichalStrehovsky MichalStrehovsky Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a type that restricts this to remove?

<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
<xsd:attribute name="feature" />
</xsd:complexType>
</xsd:element>

<xsd:element name="argument">
<xsd:complexType mixed="true">
<xsd:choice minOccurs="0" maxOccurs="1">
<xsd:element name="argument">
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="type" />
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>
</xsd:choice>
<xsd:attribute name="type" />
</xsd:complexType>
</xsd:element>

<xsd:element name="attribute">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="argument" />
<xsd:element minOccurs="0" maxOccurs="unbounded" name="property">
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="name" />
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>
</xsd:choice>
<xsd:attribute name="internal" type="xsd:string" />
<xsd:attribute name="fullname" type="xsd:string" />
<xsd:attribute name="assembly" type="xsd:string" />
</xsd:complexType>
</xsd:element>

<xsd:element name="namespace">
<xsd:complexType>
<xsd:attribute name="fullname"/>
</xsd:complexType>
</xsd:element>

<xsd:element name="assembly">
<xsd:complexType>
<xsd:choice minOccurs="0" maxOccurs="unbounded">
<xsd:element minOccurs="0" maxOccurs="unbounded" ref="type" />
<xsd:element minOccurs="0" maxOccurs="unbounded" ref="attribute" />
<xsd:element minOccurs="0" maxOccurs="unbounded" ref="resource" />
<xsd:element minOccurs="0" maxOccurs="unbounded" ref="namespace" />
</xsd:choice>
<xsd:attribute name="fullname" type="xsd:string" use="required" />
<xsd:attribute name="feature" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />
<xsd:attribute name="featuredefault" type="xsd:boolean" />
<xsd:attribute name="preserve" type="preservevalues" />
</xsd:complexType>
</xsd:element>

<xsd:simpleType name="preservevalues">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="all" />
<xsd:enumeration value="none" />
<xsd:enumeration value="fields" />
<xsd:enumeration value="methods" />
</xsd:restriction>
</xsd:simpleType>

<xsd:simpleType name="accessorvalues">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="all" />
<xsd:enumeration value="get" />
<xsd:enumeration value="set" />
</xsd:restriction>
</xsd:simpleType>

<xsd:element name="type">
<xsd:complexType>
<xsd:choice minOccurs="0" maxOccurs="unbounded">
<xsd:element ref="event" />
<xsd:element ref="method" />
<xsd:element ref="attribute" />
<xsd:element ref="field" />
<xsd:element ref="property" />
<xsd:element ref="type" />
</xsd:choice>
<xsd:attribute name="fullname" type="xsd:string" />
<xsd:attribute name="name" type="xsd:string" />

@MichalStrehovsky MichalStrehovsky Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we support name on type?

@jtschuster jtschuster Jan 18, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation has name supported on nested types, but not on top level.

@vitek-karas vitek-karas Jan 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the code do?

@jtschuster jtschuster Jan 20, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it also allows name on nested types but not on top level.

@jtschuster jtschuster Jan 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected virtual void ProcessTypes (AssemblyDefinition assembly, XPathNavigator nav, bool warnOnUnresolvedTypes) in ProcessLinkerXmlBase.cs gets the fullname attribute, but in ProcessType in LinkAttributesParser.cs it will look for name, and doesn't look for fullname

<xsd:attribute name="required" type="xsd:boolean" />
<xsd:attribute name="preserve" type="preservevalues" />
<xsd:attribute name="feature" type="xsd:string" />
<xsd:attribute name="featurevalue" type="xsd:boolean" />

@MichalStrehovsky MichalStrehovsky Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add featuredefault?

</xsd:complexType>
</xsd:element>

<xsd:element name="linker">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="assembly" />
<xsd:element ref="type" />
</xsd:choice>
</xsd:complexType>
</xsd:element>

</xsd:schema>
2 changes: 1 addition & 1 deletion src/ILLink.Tasks/CreateRuntimeRootDescriptorFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ static void AddXmlTypeNode (XmlDocument doc, XmlNode assemblyNode, string typeNa
typeNode.Attributes.Append (preserve);
} else if ((members.fields == null) && (members.methods == null)) {
XmlAttribute preserve = doc.CreateAttribute ("preserve");
preserve.Value = "nothing";
preserve.Value = "none";

@tlakollo tlakollo Jan 13, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's correct to have only one between the two, but just be aware that seems like runtime is using the 'nothing' notation since is what it was documented. See file.
The way I see it we either agreed on 'nothing' (and change only some linker tests) or agreed on 'none' and have to change other files outside the linker repo who may have used 'nothing' already

@vitek-karas vitek-karas Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the guiding force should be what the linker actually recognizes - if it's both, then we should allow both in the schema as well.

@tlakollo tlakollo Jan 13, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the code is looking for Nothing and if an invalid value is passed then it defaults to Nothing
https://github.com/dotnet/linker/blob/main/src/linker/Linker.Steps/DescriptorMarker.cs#L137-L146

typeNode.Attributes.Append (preserve);
}

Expand Down
Loading