ParsedProjectXmlPaths treats an apostrophe in an XML comment as an attribute delimiter, so the crude second-opinion scan silently under-reads — and its doc claims twice that it cannot

[erikdarlingdata]

CrossAppGuardCiGateTests.ParsedProjectXmlPaths is the deliberately-crude "second opinion" text scan that TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds compares MSBuild's evaluated item set against. Its regex (line 2037):

"(?<dq>[^"]*)"|'(?<sq>[^']*)'|>(?<text>[^<>]*)<

The single-quote alternation treats an apostrophe in an XML comment as an attribute delimiter. An unbalanced pair swallows every quoted attribute value between it and the next apostrophe, so the scan silently returns fewer paths than the file contains.

The doc says this cannot happen, in two places

Line 2028: "Crude in both directions, and both are the safe direction for a lower bound. It over-reads — a path inside a comment, or behind a Condition that never holds, counts — and it under-reads anything needing evaluation."

Line 823 makes the same claim again: "over-reads (a path inside a comment counts) and under-reads (anything needing evaluation…)"

Under-reading arbitrary quoted values is not the safe direction, and it is not "anything needing evaluation" — it is a literal Include= that the scan simply cannot see. Both sites are wrong and both need correcting with whatever fix lands. This is an unenforced claim: the doc asserts a property, nothing holds it, and the property is false.

It is already live on dev, and position is the invariant rather than parity

Lite.Tests/Lite.Tests.csproj on dev carries three apostrophes — an odd count, so they are already unbalanced. The alternation pairs the first with the second and consumes the span between them; the third finds no partner and is skipped. So a region of that csproj is being swallowed on dev right now, and the test is green only because no floor path happens to sit inside it.

That is worth measuring rather than reasoning about: which attribute values are currently invisible to the scan on dev?

Reproduced deterministically, not a flake: adding an ItemGroup with an XML comment containing five apostrophes above the ItemGroup holding <Compile Include="..\Darling\Darling.Tests\CSharpSourceWalker.cs" … /> reds the test; removing the apostrophes greens it. Parity of apostrophes is not the invariant — position relative to the paths is.

The failure reads as the opposite of what it is

Assert.Contains() Failure: Item not found in set
Set:       []
Not found: "Darling/Darling.Tests/CSharpSourceWalker.cs"
  CrossAppGuardCiGateTests.cs(878)

Line 878 is Assert.Contains(floor, parsedCross); — the crude side — immediately followed by Assert.Contains(floor, evaluated);. Because the crude assertion runs first and neither carries a distinguishing message, a crude-parse failure presents as though the evaluated read had moved the blind spot. Only the second is the defect this guard exists to catch.

Two decisions, to be made rather than assumed

1. What the fix is. Three candidates, and they are not equivalent:

• Strip XML comments before scanning. This narrows the stated over-read — a path inside a comment would stop counting — and the doc argues over-reading is the safe direction. So the doc has to change with it, and the argument for why the narrower over-read is still safe has to be made rather than inherited.
• Stop treating a single quote as a delimiter. MSBuild accepts single-quoted attribute values, so this trades one under-read for a different one — and that trade needs stating.
• Fail loudly when the quote pairing does not close. Turns a silent under-read into a reported one. Weakest as a fix, strongest as a guarantee.

2. Whether the assertion distinguishes the two failures. "The crude parse found nothing" and "the evaluated read is missing something" are different defects and currently produce the same message. Splitting them is the same move #3138 made when it split "a duration I cannot read" from "the budget is too small" into separate pins — a pin should fail for its own reason.

Acceptance criteria

• an unbalanced apostrophe in a comment above the floor path no longer hides it;
• whichever doc claim survives is true, at both line 823 and line 2028;
• a crude-parse failure and an evaluated-read failure are distinguishable from the failure message alone;
• the currently-swallowed region on dev is named.

Related

Lineage: #3063, #3074, #3076, #3082. Surfaced during #3134/#3138, where a csproj comment added by that lane tripped it and the empty set was initially read as an evaluated-side regression — including by me.

[erikdarlingdata]

Claude posting for Erik Darling

Closed by #3146, merged to dev at d834ab1dbb2ce2c6c7f408bf75f67c14c5c0b513. Closing by hand — closing keywords do not fire here, since merges land on dev while the default branch is main.

The crude second opinion now parses the XML (XDocument) instead of guessing its quote delimiters, with a text scan retained for comment bodies only. Crude now means unevaluated rather than mis-parsed.

The strongest thing in the PR is a measurement I did not have when I filed this. I reported three apostrophes in Lite.Tests.csproj — odd, therefore already unbalanced, therefore a region already being swallowed. That held: offsets 3714–3853, 139 chars, an entire <None Include=…> item.

But the sibling file is worse and inverts the obvious fix. Darling/Darling.Tests/Darling.Tests.csproj carries six apostrophes — even — and loses 2,046 chars (offsets 3311–5357) containing four <None> items. So a parity check would have called the badly-damaged file clean and the lightly-damaged one dirty. That kills the "fail on unbalanced quoting" option on measurement rather than on taste, which is a better refutation than "an odd count is not the invariant."

A correction to my own framing in this issue: I described prose in element text as a vector. It is not — the >-to-< arm consumes >Erik's tool< before the apostrophe arm can open, so only a comment, which that arm cannot enter, can start a bogus span. The doc claim is now scoped to what was measured rather than to what read well.

Thirteen violation routes are enumerated and each mapped to a guard; four were blind pre-fix (even-count apostrophe pairing across comments, odd-count, entity-encoded separator, CDATA body) and one was actively failing. Each negative arm carries a literal in the same document asserted present, so an absence cannot be the absence of a read.

Two things described and deliberately left unfiled: ImportElements' regex is <Import\s[^>]*/?>, so the \s makes <Import/> — no space — invisible to the gate (measured: <Import /> matches, <Import/> does not). And TrackedFiles is named for tracking but uses Directory.EnumerateFiles filtering only bin/obj, so it reads untracked files too.