Repository navigation
Fix VB XML attribute-axis reads - #1293
Draft
CosmonautJones wants to merge 3 commits into
Draft
CosmonautJones wants to merge 3 commits into
CosmonautJones wants to merge 3 commits into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #950.
Problem
VB's
element.@namereturns a string, orNothingwhen the attribute is missing. The converter emitsAttributes(name), a collection that cannot be assigned to a string.Solution
For a single element, emit the
XAttributestring conversion overAttribute(name), producing(string)element.Attribute("name")after simplification. ForIEnumerable<XElement>, read the first element's attribute value. A missing attribute on the first element must stay null even when a later element has that attribute; an empty sequence also returns null. Explicit element/interface casts and static LINQ-to-Objects calls preserve VB's binding for derived, custom, queryable, and value-type receivers.Unprefixed attributes ignore the default XML namespace; explicit prefixes retain their namespace. Conditional attribute chains capture the receiver once so the string conversion, null checks, and subsequent member/index access preserve VB behavior. Statement forms, including explicit
Call, use anifwhen required.The conditional-access lowering is the part most worth maintainer scrutiny. Tests cover null initial receivers, null intermediate values, child/index chains,
Withinside an unrelated conditional lambda, and void/discarded-return calls. This patch covers attribute-axis reads, not setters or the separate XML-literal entity-decoding issue.Validation
All results below are for exact commit
ae1d57ca8b0ffa7eadedab2dc9acc83d6418ec3bon Windows x64, built with .NET SDK 10.0.402. Both Release target-framework builds completed with 0 warnings and 0 errors.net10.0)net8.0)The broader selection includes both directions' expression tests, VB-to-C# self-verifying tests, newline tests, and the existing
CallStatementAsync. All 23 new XML facts passed individually in each broader run. The three existing skips areDeeplyNestedBinaryExpressionShouldNotStackOverflowAsync(too slow),LinqAggregateSumAsync, andLinqGroupJoinAsync(the latter two tracked by #29).Build command for each target:
The net10.0 runs used
dotnet testwith--no-build --no-restoreand these filters:The net8.0 assembly was executed with the existing SDK 9.0.316 VSTest 17.14.1 launcher, using
/Framework:.NETCoreApp,Version=v8.0and the same filters. That launcher ran on .NET 9.0.18; the actual testhost ran on 8.0.32, verified from its loadedcoreclr.dll, host traces, and diagnostics. The compiler harness uses the repository's .NET 8 reference assemblies for both runs. This does not claim an SDK-8 build or full upstream CI, Linux, VSIX, web, or full-repository validation.A separate internal AI QA agent reviewed the final diff, added two challenge tests whose failures were fixed, independently built and ran the final net10.0 focused suite, and checked the final broader and net8.0 evidence. No actionable findings remained. The initial focused regression reproduced the collection-to-string conversion failure before the fix.
Process