RepoFileAdoptionTests' adopter census cannot see qualified reader calls, so the exact-set equality passes with an undeclared LF-reading file in the tree

[erikdarlingdata]

RepoFileAdoptionTests' adopter census cannot see a qualified call to either repo-file reader, so TheLfReadingPins_AreExactlyTheOnesDeclaredHere — an exact-set equality — passes while an undeclared LF-reading file exists in the tree today.

The two regexes, and what they miss

Darling/Darling.Tests/RepoFileAdoptionTests.cs:

private static readonly Regex RawCall = new(
    @"(?<![\w.])ReadRepoFile\s*\(", RegexOptions.Compiled);

private static readonly Regex LfCall = new(
    @"(?<![\w.])ReadRepoFileLf\s*\(", RegexOptions.Compiled);

The lookbehind excludes a preceding ., which is what stops ReadRepoFileLf( being double-counted as a raw call. It also makes the qualified spelling invisible to both:

spelling	RawCall	LfCall
ReadRepoFile(	true	false
RepoFile.ReadRepoFile(	false	false
ReadRepoFileLf(	false	true
RepoFile.ReadRepoFileLf(	false	false

And the raw-adopter decision compounds it — a file is raw only when RawCall.Matches(code).Count > LfCall.Matches(code).Count. With qualified calls both counts are 0, so 0 > 0 is false and the file is neither kind of adopter. It is not miscategorised; it is absent.

The declaration census is unaffected and worth stating so the fix is scoped: that pattern uses \bReadRepoFile(?:Lf)?\s*\( with a word boundary rather than the lookbehind, so ExactlyOneFile_DeclaresTheSharedRepoFileReader still works. Only call sites are blind.

There are TWO live instances, and they break different assertions

Measured on origin/dev (01fdeae92), both surviving comment and string stripping:

file	form	lines	breaks
RepoFileResolutionEquivalenceTests.cs	RepoFile.ReadRepoFileLf(	164, 184	the LF exact-set equality
PostgresCancelOriginTests.cs (added by #3127)	RepoFile.ReadRepoFile(	304, 342	the adopter floor

They are not the same defect twice. The LF caller makes TheLfReadingPins_AreExactlyTheOnesDeclaredHere pass with an undeclared LF adopter in the tree. The raw caller does not enter any declared set, but it does not count toward adopters.Length >= 30 either — so the floor is measured against an undercount that is now growing, since #3127 added to it yesterday.

The LF instance passes for the wrong reason

Darling/Darling.Tests/RepoFileResolutionEquivalenceTests.cs makes two qualified LF calls that survive comment and string stripping:

164:  var text = lf ? RepoFile.ReadRepoFileLf(segments) : RepoFile.ReadRepoFile(segments);
184:  Assert.Throws<FileNotFoundException>(() => RepoFile.ReadRepoFileLf(missing));

It is not in s_lfReaders (eight entries, and this is not one), and the set equality passes anyway.

Its exclusion is substantively correct — it is the equivalence test for the reader, parameterised over both forms, and it normalises line endings itself at line 171 (onDisk.Replace("\r\n", "\n", ...)) rather than relying on LF anchors. So the right file is excluded for the wrong reason: the census cannot see it, not that it isn't LF-anchored.

Why that matters rather than being a curiosity

A future pin using the qualified LF form with multi-line anchors would be invisible to the set equality, so the assertion would pass while that pin sat undeclared — which is precisely the drift this class exists to catch. The guard would report the tree adopted correctly while it hadn't.

That is the class's own subject turned on itself. Its summary enumerates its known blind spots and does not mention this one.

Secondary, no live risk: the adopters.Length >= 30 floor is measured against the same undercount. It reads 38 today, so the floor is satisfied with margin, but the figure it is satisfied by is smaller than the truth.

Suggested direction

1. Count qualified and unqualified separately rather than excluding . from the match. The lookbehind exists to stop the Lf spelling being double-counted as raw — a real problem, since the name is a prefix — but that is a disambiguation need, and it is being solved by making a whole legal form unmatchable. Match (?:RepoFile\.)?ReadRepoFileLf\s*\( and (?:RepoFile\.)?ReadRepoFile\s*\( and keep the existing count comparison to separate them.
2. Then re-check the declared set, because RepoFileResolutionEquivalenceTests.cs will become visible and must be either declared or excluded on its actual grounds — that it normalises line endings itself. If it is excluded, the exclusion needs to be expressible in the assertion rather than accidental.
3. Add the qualified form to the summary's blind-spot list if any part of it survives.

Acceptance criteria, both arms:

• a file making only RepoFile.ReadRepoFileLf( calls is seen by the census, and the set equality reds if it is undeclared;
• the existing eight declared readers still satisfy the equality, and no currently-passing adopter becomes miscategorised.

The second arm matters: a fix that simply drops the lookbehind reintroduces the double-count the lookbehind was added to prevent, which would classify every LF reader as also a raw one.

Related

• Darling PostgreSQL tests skips shared-library changes its two sibling jobs catch, reports green, and blames documentation #3116 / Share one path gate between the two Darling CI jobs and report its decision #3122 — surfaced this while pre-flighting the exact-set equality across a merge. That PR is unaffected: DarlingPathFilterGateTests uses the unqualified form via using static, so it is counted and declared, and every pre-flight that came back True did so legitimately.
• Read a PostgreSQL cancel's origin from the message text rather than SQLSTATE 57014 #3127 — its new test uses the qualified raw form, so it is correctly outside the LF set; the census simply cannot see it at all, rather than seeing it and classifying it raw.

[erikdarlingdata]

Fixed in #3135. Two of the mechanisms in the write-up above did not survive measurement, and the fix is shaped around what did.

RawCall never matched the Lf spelling. The truth table in the issue has ReadRepoFileLf( scoring false for RawCall, which is right, but the stated reason — that the lookbehind is what stops the double-count, "since the name is a prefix" — is not. The mandatory \s*\( is: ReadRepoFile is followed by L, not a paren. So RawCall and LfCall were disjoint, and RawCall.Count > LfCall.Count was comparing two populations that never overlapped. It reads as a subtraction and behaves as a comparison, and the sharpest way to say what that costs is that the comparison contradicts its own comment. That comment says the count form exists so "a class that legitimately uses both readers lands in both lists instead of one arbitrarily" — and in the mixed case it lands in exactly one, arbitrarily. Measured: an unqualified 1-raw + 2-LF file scores 1 > 2 and so is not a raw adopter, while the LF check is an independent if (LfCall.IsMatch(code)) and does add it — rawAdopters 29 unchanged, lfAdopters 9→10. It lands in the LF list only, and since adopters is the union it still clears the floor. Zero files in the tree did that, so it was latent.

Consequently, the second acceptance arm's predicted failure does not occur. I ran the exact naive fix — both patterns with the lookbehind deleted — over the real tree: it produces the same census as dev, not "every LF reader classified as also raw". The lookbehind is still load-bearing, but for the other half of its job: it is what refuses Something.ReadRepoFile( and MyReadRepoFile(, neither of which exists in the tree today, so the difference shows in a truth table and not in a census.

The fix therefore keeps the count comparison and makes it honest, by giving the raw pattern (?:Lf)? so the overlap the comparison assumes is real. All nine declared LF readers now report any == lf (ServerPageTabsTests 13/13), and the mixed case classifies correctly.

One number to re-derive: s_lfReaders has NINE entries on dev, not eight. #3122 added DarlingPathFilterGateTests.cs between 01fdeae92 and 641c4db7f — which is the same file the Related section names as legitimately counted through using static. Everything else measured here reproduces exactly: lines 164/184 and 304/342 are unchanged, 38 adopters before, 40 after.

RepoFileResolutionEquivalenceTests.cs stays out of s_lfReaders on the grounds you name, and is declared in a new s_lfSubjects whose every entry is asserted to be visible to the census — so the exclusion reds if the patterns regress, instead of being satisfied by a file the scan cannot find.

[erikdarlingdata]

Claude posting for Erik Darling

Fixed by #3135, merged to dev. Closed by hand — closing keywords do not fire here.

Census before → after: 1 declarer / 9 LF / 29 raw / 38 union → 1 / 10 / 31 / 40, against a floor of 30. Nothing lost on either axis; the only movement is the two files this issue named.

RepoFileResolutionEquivalenceTests.cs stays out of s_lfReaders on its actual grounds — it is the equivalence test for the reader, parameterised over both spellings, and normalises CRLF itself rather than relying on LF anchors — and is now declared in a new s_lfSubjects whose every entry is asserted to be visible to the census before the equality runs, plus Assert.Empty(s_lfReaders.Intersect(s_lfSubjects)) so no file can be both. An exemption naming a file the scan cannot find does no work; it now reds instead of reading as a decision. That is more than this issue asked for, which was only that the exclusion be expressible rather than accidental.

PostgresCancelOriginTests.cs is a raw adopter and now counts toward the floor.

The class summary's blind-spot list goes from three items to four, adding that a call is matched by name and receiver, and that an aliased receiver (using RF = RepoFile;) or a call reached through a delegate captured from the method remain outside the scan. Those are documented, not closed.

See the comment above for the two mechanisms in the original write-up that did not survive measurement — the short version is that RawCall never matched the Lf spelling, so the two patterns were disjoint, and the count comparison between them was comparing populations that never overlapped.