Version 3.4.12; updated SDK to .NET 10; added gitHub/agent instructions #79
Conversation
… is provided for the `gen -a path\` command.
…comparisons from older versions.
…lass TypeEnumValidator is no longer being generated!
…re error messages on test failure.
….System.DateTimeOnly and System.Memory nuget.
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>net6.0;net5.0;netcoreapp3.1;netframework472</TargetFrameworks> | ||
| <TargetFrameworks>net10.0;net9.0;net8.0;net7.0;net6.0;net5.0;netcoreapp3.1;netframework472</TargetFrameworks> |
There was a problem hiding this comment.
@mamift I don't think this is needed?
Normally you can run tools targeting earlier frameworks without issue on newer one?
Listing a framework explicitly should only be required if you intend on using a new framework-specific feature.
Having many targets needs more build passes (so longer) and bloats IDE memory, which tries to juggle available apis for all requested target framework.
There was a problem hiding this comment.
That is only for the LinqToXsd tool project; XObjectsCore and XObjectsCodeGen still target just 2 framework monikers.
There was a problem hiding this comment.
What is different about LinqToXsd tool? If it's built for net5.0 I can run it on my machine that has .net10.0, can't I?
Another drawback of this long list of target frameworks is that anyone who works on LinqToXsd source needs to have all these targeting packs installed.
There was a problem hiding this comment.
Another drawback of this long list of target frameworks is that anyone who works on LinqToXsd source needs to have all these targeting packs installed
No they don't. I've only got .NET 6, 8, and 10 SDK installed. Only .NET 8 and 10 runtimes and am able to build it all fine. Also the build server I use only uses the .NET 10 SDK and runtime and it's able to produce assets for all TFMs without issue.
There was a problem hiding this comment.
Not SDKs, targeting packs.
You can have just .NET 10 SDK and build for all targets listed here, but you need targeting packs.
At least for .net framework versions... now that I look into it, targeting packs don't seem to exists for .net core, makes me wonder how it works.
But I still don't get why so many targets are needed? If you target net5.0 (and build with .net 10 SDK), it should run just fine on a machine that has .net 10 runtime?
…e a name for a simple type union type.
mamift
force-pushed
the
github/agent-instructions
branch
from
d04efd2 to
c79f35d
Compare
…d so its includedin pack/publish.
…peReference class).
…, an error is thrown; when a config file doesn't exist, then a blank LinqToXsdSettings instance is created.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // TODO: Continue from here | ||
| if (attribute.Name == "date") { | ||
| Debugger.Break(); | ||
| } |
There was a problem hiding this comment.
🟡 Debugger.Break left in production code path for attribute named "date"
When generating properties for schema attributes, the code unconditionally triggers a Debugger.Break() for an attribute named date, which will halt execution when a debugger is attached.
Click to expand
In BuildPropertyForAttribute, the following code runs before any real logic:
// TODO: Continue from here
if (attribute.Name == "date") {
Debugger.Break();
}This means that running code generation under a debugger (including many IDE test/debug runs) will unexpectedly stop whenever a schema contains an attribute named date.
- Actual: generator breaks into debugger for certain schemas.
- Expected: generator should not contain ad-hoc debugging traps.
- Impact: disrupts developer workflows and can break automated debug runs.
Recommendation: Remove the Debugger.Break() block (and the adjacent TODO if not actionable), or guard it behind a local, explicitly opt-in diagnostic flag (never keyed on schema content).
Was this helpful? React with 👍 or 👎 to provide feedback.
| var thePossibleType = type.Members.Cast<CodeTypeMember>().SingleOrDefault(predicate); | ||
| if (thePossibleType is null) { | ||
| IEnumerable<CodeTypeDeclaration> nestedTypes = type.Members.OfType<CodeTypeDeclaration>(); | ||
| foreach (var nestedType in nestedTypes) { | ||
| thePossibleType = nestedType.SearchAllNestedTypesRecursively(predicate); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🔴 Recursive nested-type search can throw due to SingleOrDefault on potentially non-unique predicate
SearchAllNestedTypesRecursively uses SingleOrDefault(predicate) when searching members, which will throw InvalidOperationException if more than one member matches the predicate. Callers use broad predicates (e.g., Name.Contains(...)), making multiple matches realistic.
Click to expand
In CodeDomExtensions:
var thePossibleType = type.Members.Cast<CodeTypeMember>().SingleOrDefault(predicate);If a type has multiple nested members that satisfy predicate, this throws instead of returning one match or continuing the search.
A concrete caller added in this PR uses a substring match:
ec.Name.Contains(this.TypeReference.Name)which can match multiple validator/helper types.
- Actual: code generation can crash with
InvalidOperationException: Sequence contains more than one matching element. - Expected: recursive search should be tolerant of multiple matches (e.g., return the first/best match, or allow caller to decide).
- Impact: potential hard failure during code generation for schemas that create multiple similarly-named nested types.
Recommendation: Replace SingleOrDefault with FirstOrDefault (or FirstOrDefault with deterministic ordering), and short-circuit recursion once a match is found. If uniqueness is required, enforce it explicitly with a clearer error message and a stricter predicate.
Was this helpful? React with 👍 or 👎 to provide feedback.
… uses FirstOrDefault instead of SingleOrDefault when returning a possible match.
| private static void TraverseAllSimpleTypes(XmlSchemaObject schemaObject, ref Dictionary<XElementOrAttrOrQName, XmlSchemaSimpleType> simpleTypes, out bool breakOutOfLoop) | ||
| { | ||
| if (simpleTypes == null) throw new ArgumentNullException(nameof(simpleTypes)); | ||
|
|
||
| bool didAdd = true; | ||
| switch (schemaObject) { | ||
| case XmlSchemaAttribute attribute: { | ||
| didAdd = simpleTypes.AddIfNotAlreadyExists(attribute, attribute.AttributeSchemaType); | ||
| break; | ||
| } | ||
|
|
||
| case XmlSchemaElement element: { | ||
| if (element.ElementSchemaType is XmlSchemaSimpleType simpleType) { | ||
| didAdd = simpleTypes.AddIfNotAlreadyExists(element, simpleType); | ||
| } | ||
| else if (element.ElementSchemaType is XmlSchemaComplexType complexType) { | ||
| foreach (var attr in complexType.AttributeUses.Values.Cast<XmlSchemaAttribute>()) { | ||
| TraverseAllSimpleTypes(attr, ref simpleTypes, out breakOutOfLoop); | ||
| if (breakOutOfLoop) return; | ||
| } | ||
|
|
||
| var childElements = complexType.LocalXsdElements() | ||
| .Where(e => !ReferenceEquals(e, schemaObject)) | ||
| .ToList(); | ||
|
|
||
| foreach (var el in childElements) { | ||
| TraverseAllSimpleTypes(el, ref simpleTypes, out breakOutOfLoop); | ||
| if (breakOutOfLoop) return; | ||
| } | ||
| } | ||
| else { | ||
| if (element.ElementSchemaType is XmlSchemaComplexType { Particle: XmlSchemaGroupBase gb }) { | ||
| foreach (var particle in gb.Items) { | ||
| TraverseAllSimpleTypes(particle, ref simpleTypes, out breakOutOfLoop); | ||
| if (breakOutOfLoop) return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| break; | ||
| } | ||
|
|
||
| case XmlSchemaComplexType complexType: { | ||
| foreach (var attribute in complexType.AttributeUses.Values.OfType<XmlSchemaAttribute>()) { | ||
| if (attribute.AttributeSchemaType is { } simpleType) { | ||
| didAdd = simpleTypes.AddIfNotAlreadyExists(attribute, simpleType); | ||
| } | ||
| } | ||
|
|
||
| if (complexType.ContentTypeParticle != null) { | ||
| TraverseAllSimpleTypes(complexType.ContentTypeParticle, ref simpleTypes, out breakOutOfLoop); | ||
| if (breakOutOfLoop) return; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
|
|
||
| case XmlSchemaGroupBase groupBase: { | ||
| OneOf<XmlSchemaAll, XmlSchemaSequence, XmlSchemaChoice> matchModel = default; | ||
| if (groupBase is XmlSchemaAll all) { matchModel = all; } | ||
| else if (groupBase is XmlSchemaSequence seq) { matchModel = seq; } | ||
| else if (groupBase is XmlSchemaChoice choice) { matchModel = choice; } | ||
|
|
||
| foreach (var item in matchModel.Match(a => a.Items, s => s.Items, c => c.Items)) { | ||
| TraverseAllSimpleTypes(item, ref simpleTypes, out breakOutOfLoop); | ||
| if (breakOutOfLoop) return; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| } | ||
|
|
||
| breakOutOfLoop = !didAdd; | ||
| } |
There was a problem hiding this comment.
🔴 RetrieveAllSimpleTypes traversal stops early when encountering a duplicate key, missing types
TraverseAllSimpleTypes uses breakOutOfLoop = !didAdd to abort recursion whenever a (key,type) pair has already been added to the dictionary. This causes traversal to stop prematurely for schemas with shared/reused objects (common in compiled XSD object graphs), skipping unrelated branches and returning an incomplete set of discovered simple types.
- Actual: The first duplicate encountered terminates the current traversal branch (and often the caller’s loop), so many anonymous/simple types are never collected.
- Expected: Duplicates should be ignored while traversal continues, or cycle detection should prevent infinite recursion without preventing discovery of other types.
Click to expand
At the end of the traversal function:
breakOutOfLoop = !didAdd;didAdd becomes false whenever AddIfNotAlreadyExists(...) sees an existing key. In an XSD object graph, that is not a signal to abort discovery; it just means “already visited this node”.
Recommendation: Replace the breakOutOfLoop logic with a real visited-set (e.g., HashSet<XmlSchemaObject>), or simply do not abort on duplicates. If cycle prevention is needed, track visited nodes and skip revisiting them while continuing traversal.
Was this helpful? React with 👍 or 👎 to provide feedback.
Updated to version 3.4.12
gen -a path\command.This is a release that fixes exactly one CLI bug and adds more test for quality assurance for future releases.
Also addresses #67