Follow-up from the review of #677. Several behaviours added or changed there are not pinned by any test — reverting them leaves the whole suite green. Given that every round of that PR produced the next round's regressions, these are the surfaces most likely to drift silently.
All results below are from reverting the change in place and running GOWORK=off go test -count=1 ./internal/parser/languages/ ./internal/resolver/.
csharp_binding_scopes.go is entirely unpinned
Inserting if true { return } as the first statement of csharpCollectExtraBindingScopes — turning all 298 lines into a no-op — leaves both whole packages green.
The cause is that every new test asserts only the negative direction (the binder must not shadow at this call site). The file's actual purpose — that foreach variables, lambda parameters, out vars, patterns and using resources do shadow a same-named field inside their extent — has no coverage. A probe with a field _box and three methods (foreach (var _box in xs) { _box.Touch(); }, Array.ForEach(xs, _box => _box.Touch()), using (var _box = new PBag()) { _box.Touch(); }) goes from 0 to 3 spurious field reads under the mutation, and nothing notices.
A coverage run puts 43% of the file's lines at zero executions under the PR's own suite.
Two of the three per-record guard changes are inert or unpinned
The per-record typedRecordAt guard replaced the function-wide guard in three places. Only one is load-bearing:
| guard |
revert leaves suite green? |
pinned by |
tier 1 (b.typ != "", first var pass) |
no |
TestResolveCSharpTypedLocal_SiblingScopeRedeclarationKeepsItsRecord |
tier 2 (b.typ != "", await pass) |
yes |
— |
shape pass (b.shape != "") |
yes |
— |
Instrumenting both conditions to log every disagreement and running the entire C# suite: the tier-2 guard never diverges at all — the per-record and function-wide answers are identical on every fixture in the repo. The shape guard diverges exactly once (Sibling.cs::SbRunner.Run, name conv) and reverting it changes no assertion, because tier 1 already rescues that site.
Either add a fixture that distinguishes each, or revert the two that buy nothing — a speculative untested guard change is exactly the surface that bit in previous rounds.
ambiguousAt is unpinned
Reverting it leaves the suite green, and the test written to restore its pin passes for a different reason than the one it names.
Two new tests cannot fail
TestResolveCSharpInterfaceDispatch_RespelledBasesKeepTheFamily passes unchanged at the merge base f9e3f442 and survives removal of the fix it is named for.- The
widgetSurvives assertion in TestResolveCSharp_UnequalSpanMethodsSharingALine is vacuous — the fan-out it checks has no type-argument gate now that the gate is deferred.
- The write half of
TestResolveCSharpTypedLocal_AliveAcrossSwitchSections cannot fail: its fixture declares no field, so the "no field write may be minted" branch is unreachable.
Suggested approach
For each item above, the cheap check is the same one used to find it: revert the line, run the two packages, and confirm something goes red. Worth making that a habit for this extractor specifically — the mutation sweep took a few minutes and found five unpinned behaviours across a diff that was otherwise well tested.
Follow-up from the review of #677. Several behaviours added or changed there are not pinned by any test — reverting them leaves the whole suite green. Given that every round of that PR produced the next round's regressions, these are the surfaces most likely to drift silently.
All results below are from reverting the change in place and running
GOWORK=off go test -count=1 ./internal/parser/languages/ ./internal/resolver/.csharp_binding_scopes.gois entirely unpinnedInserting
if true { return }as the first statement ofcsharpCollectExtraBindingScopes— turning all 298 lines into a no-op — leaves both whole packages green.The cause is that every new test asserts only the negative direction (the binder must not shadow at this call site). The file's actual purpose — that
foreachvariables, lambda parameters, out vars, patterns andusingresources do shadow a same-named field inside their extent — has no coverage. A probe with a field_boxand three methods (foreach (var _box in xs) { _box.Touch(); },Array.ForEach(xs, _box => _box.Touch()),using (var _box = new PBag()) { _box.Touch(); }) goes from 0 to 3 spurious field reads under the mutation, and nothing notices.A coverage run puts 43% of the file's lines at zero executions under the PR's own suite.
Two of the three per-record guard changes are inert or unpinned
The per-record
typedRecordAtguard replaced the function-wide guard in three places. Only one is load-bearing:b.typ != "", firstvarpass)TestResolveCSharpTypedLocal_SiblingScopeRedeclarationKeepsItsRecordb.typ != "", await pass)b.shape != "")Instrumenting both conditions to log every disagreement and running the entire C# suite: the tier-2 guard never diverges at all — the per-record and function-wide answers are identical on every fixture in the repo. The shape guard diverges exactly once (
Sibling.cs::SbRunner.Run, nameconv) and reverting it changes no assertion, because tier 1 already rescues that site.Either add a fixture that distinguishes each, or revert the two that buy nothing — a speculative untested guard change is exactly the surface that bit in previous rounds.
ambiguousAtis unpinnedReverting it leaves the suite green, and the test written to restore its pin passes for a different reason than the one it names.
Two new tests cannot fail
TestResolveCSharpInterfaceDispatch_RespelledBasesKeepTheFamilypasses unchanged at the merge basef9e3f442and survives removal of the fix it is named for.widgetSurvivesassertion inTestResolveCSharp_UnequalSpanMethodsSharingALineis vacuous — the fan-out it checks has no type-argument gate now that the gate is deferred.TestResolveCSharpTypedLocal_AliveAcrossSwitchSectionscannot fail: its fixture declares no field, so the "no field write may be minted" branch is unreachable.Suggested approach
For each item above, the cheap check is the same one used to find it: revert the line, run the two packages, and confirm something goes red. Worth making that a habit for this extractor specifically — the mutation sweep took a few minutes and found five unpinned behaviours across a diff that was otherwise well tested.