From 2a20945d62c86e8f937623ab3f6b31c5fff5b885 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 10:09:14 +1000 Subject: [PATCH] Let a staging clear be scoped to the framework that settled InlineQueue.Settle is origin scoped, and the reason is on it: a framework that starts passing removes only its own label, so the other framework's still failing content stays reviewable. InlineStaging.Clear had no way to say the same thing, and staging is the same situation - one call site stages one trio per framework. So in a net8.0;net10.0 run with no queue owner, where net10 starts passing and net8 does not, net10's settle deleted net8's trio beside its own. That snapshot was then pending nowhere at all: not staged, and not in a queue either, because the reason it was staged is that nothing owned one. Additive rather than a change of behaviour. Clear takes an optional origin; passing none clears every framework's trio exactly as before, which is what a retire wants - the call site is not an inline snapshot in any framework any more - and is why ClearRemovesEveryFrameworksTrio still passes unchanged. A trio staged with no framework label is cleared either way, since there is nothing to scope it by and leaving it would strand it. Callers that want the queue's semantics can now ask for them; the decision about which of the two a producer should pass stays with the producer. --- src/DiffEngine.Tests/InlineStagingTests.cs | 27 ++++++++++++++++++ src/DiffEngine/Inline/InlineStaging.cs | 33 ++++++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/DiffEngine.Tests/InlineStagingTests.cs b/src/DiffEngine.Tests/InlineStagingTests.cs index 844182fd..f9313658 100644 --- a/src/DiffEngine.Tests/InlineStagingTests.cs +++ b/src/DiffEngine.Tests/InlineStagingTests.cs @@ -159,6 +159,33 @@ public async Task ClearRemovesEveryFrameworksTrio() await Assert.That(project.StagedFiles()).IsEmpty(); } + /// + /// A settle that names the framework it came from takes only that framework's trio. The queue + /// has always scoped a settle this way; staging is the same situation and had no way to say + /// it, so in a net8;net10 run where net10 started passing and net8 did not, net10's settle + /// deleted net8's still-failing snapshot and it was then pending nowhere. + /// + [Test] + public async Task ClearScopedToAnOriginLeavesTheOtherFrameworkStaged() + { + using var project = new TempProject(); + var source = project.Source("SampleTests.cs"); + InlineStaging.Persist( + [ + new( + [ + new(Patch(source, "from net8", framework: "net8.0"), ["net8.0"]), + new(Patch(source, "from net10", framework: "net10.0"), ["net10.0"]), + ]) + ]); + + var cleared = InlineStaging.Clear(source, 42, null, origin: "net10.0"); + + await Assert.That(cleared).IsEqualTo(1); + // net8's trio is still there, and still reviewable + await Assert.That(project.StagedFiles().Count).IsEqualTo(3); + } + [Test] public async Task ClearFindsACallSiteWhoseLineHasMovedByMember() { diff --git a/src/DiffEngine/Inline/InlineStaging.cs b/src/DiffEngine/Inline/InlineStaging.cs index 6f3098e7..c78c4dec 100644 --- a/src/DiffEngine/Inline/InlineStaging.cs +++ b/src/DiffEngine/Inline/InlineStaging.cs @@ -74,18 +74,34 @@ public static int Persist(IEnumerable pending) /// of its own. Verify's own fallback writes under the project's intermediate directory, which /// is normally inside that obj and found anyway, but does not have to be. /// - public static int Clear(string sourceFile, int line, string? memberName, string? extraDirectory = null) + /// + /// The framework moniker of the run that settled, which scopes the clear to that run's own + /// trio. This is what already does with a + /// queue, and staging is the same situation with the same answer: one call site stages one + /// trio per framework, so a multi-framework run where one framework starts passing and another + /// does not would otherwise have the passing one delete the failing one's staged snapshot. + /// + /// Null clears every framework's trio for the call site, which is right for a retire - the + /// call site is not an inline snapshot in any framework any more - and is the behaviour of + /// every caller that does not pass this. + /// + /// + /// A trio staged without a framework label is cleared either way, since there is nothing to + /// scope it by and leaving it would strand it forever. + /// + /// + public static int Clear(string sourceFile, int line, string? memberName, string? extraDirectory = null, string? origin = null) { var cleared = 0; foreach (var directory in StagingDirectories(sourceFile, extraDirectory)) { - cleared += ClearIn(directory, sourceFile, line, memberName); + cleared += ClearIn(directory, sourceFile, line, memberName, origin); } return cleared; } - static int ClearIn(string directory, string sourceFile, int line, string? memberName) + static int ClearIn(string directory, string sourceFile, int line, string? memberName, string? origin) { var staged = ReadStaged(directory) .Where(_ => SamePath(_.Patch.SourceFile, sourceFile)) @@ -111,6 +127,17 @@ static int ClearIn(string directory, string sourceFile, int line, string? member } } + if (origin != null) + { + // One call site stages one trio per framework, so a settle that names the framework it + // came from must take only that one. Without it a net9 run that started passing + // deleted net8's still-failing trio beside its own, and that snapshot was then pending + // nowhere: not staged, and not in a queue either, since nothing had one + matching = matching + .Where(_ => _.Patch.Framework == null || _.Patch.Framework == origin) + .ToList(); + } + var cleared = 0; foreach (var entry in matching) {