Fix combination callbacks (#1887)

* Await every registered combination callback

UseCallbacks combined the callbacks with +=. Invoking a multicast Task returning
delegate returns only the last target's Task, so when UseCallbacks was called
more than once, for example by an extension package plus user code, every
earlier callback ran unawaited: its exceptions unobserved and its async work
racing the combination method. Extensions.Then documents the same hazard for
Func<Task>.

They are now held in lists and awaited in order. The no callback path still
returns Task.CompletedTask without allocating.

* Docs changes

---------

Co-authored-by: GitHub Action <action@github.com>

Author: SimonCropp

docs/combinations.md

@@ -175,7 +175,7 @@ public Task BuildAddressExceptionsDisabledTest()
             city);
 }
 ```
-<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L214-L230' title='Snippet source file'>snippet source</a> | <a href='#snippet-CombinationSample_CaptureExceptionsFalse' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L251-L267' title='Snippet source file'>snippet source</a> | <a href='#snippet-CombinationSample_CaptureExceptionsFalse' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 
@@ -401,7 +401,7 @@ class CustomCombinationConverter :
         string.Join(", ", keys.Select(_ => _.Value));
 }
 ```
-<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L260-L269' title='Snippet source file'>snippet source</a> | <a href='#snippet-CombinationSample_CustomSerializationConverter' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L297-L306' title='Snippet source file'>snippet source</a> | <a href='#snippet-CombinationSample_CustomSerializationConverter' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 Full control of serialization can be achieved by inheriting from `WriteOnlyJsonConverter<CombinationResults>`.
@@ -420,7 +420,7 @@ static CustomCombinationConverter customConverter = new();
 public static void Init() =>
     VerifierSettings.AddExtraSettings(_ => _.Converters.Insert(0, customConverter));
 ```
-<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L232-L240' title='Snippet source file'>snippet source</a> | <a href='#snippet-CombinationSample_CustomSerializationModuleInitializer' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L269-L277' title='Snippet source file'>snippet source</a> | <a href='#snippet-CombinationSample_CustomSerializationModuleInitializer' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 
@@ -551,5 +551,5 @@ Headers can be enabled globally:
 public static void EnableIncludeHeaders() =>
     CombinationSettings.IncludeHeaders();
 ```
-<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L272-L278' title='Snippet source file'>snippet source</a> | <a href='#snippet-GlobalCombinationHeader' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/StaticSettingsTests/CombinationTests.cs#L309-L315' title='Snippet source file'>snippet source</a> | <a href='#snippet-GlobalCombinationHeader' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->

src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt

@@ -0,0 +1,4 @@
+{
+  list: Result,
+     1: Exception: from the first before callback
+}

src/StaticSettingsTests/CombinationTests.cs

@@ -79,6 +79,43 @@ await Verify(new
             .UseMethodName("CallbackResults");
     }
 
+    // Two registrations, for example an extension package plus user code. A multicast
+    // delegate would return only the last target's Task, leaving the first to run
+    // unawaited: its exceptions unobserved and its work racing the combination method.
+    [Fact]
+    public async Task EveryRegisteredCallbackIsAwaited()
+    {
+        var exceptionMessages = new List<string>();
+
+        // Fails only after an await, so the failure is carried by the returned Task rather
+        // than thrown synchronously. Registered first, so a multicast delegate would return
+        // the second registration's Task instead and this one would never be observed.
+        CombinationSettings.UseCallbacks(
+            async _ =>
+            {
+                await Task.Yield();
+                throw new("from the first before callback");
+            },
+            (_, _) => Task.CompletedTask,
+            (_, exception) =>
+            {
+                exceptionMessages.Add(exception.Message);
+                return Task.CompletedTask;
+            });
+
+        CombinationSettings.UseCallbacks(
+            _ => Task.CompletedTask,
+            (_, _) => Task.CompletedTask,
+            (_, _) => Task.CompletedTask);
+
+        int[] list = [1];
+        await Combination()
+            .Verify((int param1) => param1, list)
+            .UseMethodName("EveryRegisteredCallbackIsAwaited_run");
+
+        Assert.Equal(["from the first before callback"], exceptionMessages);
+    }
+
     [Fact]
     public async Task AfterCallbackRawValueWhenRecording()
     {

src/Verify/Combinations/CombinationSettings.cs

@@ -1,4 +1,4 @@
-namespace VerifyTests;
+namespace VerifyTests;
 
 public delegate Task BeforeCombination(IReadOnlyList<object?> keys);
 public delegate Task AfterCombination(IReadOnlyList<object?> keys, object? result);
@@ -16,7 +16,11 @@ public static void IncludeHeaders() =>
     public static void CaptureExceptions() =>
         CaptureExceptionsEnabled = true;
 
-    static BeforeCombination? before;
+    // Held as lists rather than combined with `+=`. Invoking a multicast Task returning
+    // delegate returns only the last target's Task, so every earlier callback would run
+    // unawaited: its exceptions unobserved, and its async work racing the combination
+    // method. Same hazard the Then extension documents for Func<Task>.
+    static List<BeforeCombination>? before;
 
     internal static Task RunBeforeCallbacks(IReadOnlyList<object?> keys)
     {
@@ -25,10 +29,18 @@ internal static Task RunBeforeCallbacks(IReadOnlyList<object?> keys)
             return Task.CompletedTask;
         }
 
-        return before(keys);
+        return RunAll(before, keys);
     }
 
-    static AfterCombination? after;
+    static async Task RunAll(List<BeforeCombination> callbacks, IReadOnlyList<object?> keys)
+    {
+        foreach (var callback in callbacks)
+        {
+            await callback(keys);
+        }
+    }
+
+    static List<AfterCombination>? after;
 
     internal static Task RunAfterCallbacks(IReadOnlyList<object?> keys, object? result)
     {
@@ -37,10 +49,18 @@ internal static Task RunAfterCallbacks(IReadOnlyList<object?> keys, object? resu
             return Task.CompletedTask;
         }
 
-        return after(keys, result);
+        return RunAll(after, keys, result);
     }
 
-    static CombinationException? combinationException;
+    static async Task RunAll(List<AfterCombination> callbacks, IReadOnlyList<object?> keys, object? result)
+    {
+        foreach (var callback in callbacks)
+        {
+            await callback(keys, result);
+        }
+    }
+
+    static List<CombinationException>? combinationException;
 
     internal static Task RunExceptionCallbacks(IReadOnlyList<object?> keys, Exception exception)
     {
@@ -49,14 +69,25 @@ internal static Task RunExceptionCallbacks(IReadOnlyList<object?> keys, Exceptio
             return Task.CompletedTask;
         }
 
-        return combinationException(keys, exception);
+        return RunAll(combinationException, keys, exception);
+    }
+
+    static async Task RunAll(List<CombinationException> callbacks, IReadOnlyList<object?> keys, Exception exception)
+    {
+        foreach (var callback in callbacks)
+        {
+            await callback(keys, exception);
+        }
     }
 
     public static void UseCallbacks(BeforeCombination before, AfterCombination after, CombinationException exception)
     {
-        CombinationSettings.before += before;
-        CombinationSettings.after += after;
-        combinationException += exception;
+        CombinationSettings.before ??= [];
+        CombinationSettings.before.Add(before);
+        CombinationSettings.after ??= [];
+        CombinationSettings.after.Add(after);
+        combinationException ??= [];
+        combinationException.Add(exception);
     }
 
     public static void Reset()
@@ -67,4 +98,4 @@ public static void Reset()
         after = null;
         before = null;
     }
-}
+}