From cc1980bed5bfb83b9fbb99ac5e28104161274fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Thu, 29 Oct 2026 16:23:23 +0100 Subject: [PATCH 1/3] fix: CMTK0003 reports an unawaited assertion through a pass-through or a branch An assertion of CodoMetis.TypeKit.Testing on a Task or ValueTask fails the test only through the task it returns. CMTK0003 reported one dropped as a statement, but not one that returns a plain Task followed by ConfigureAwait or WaitAsync (ShouldBeNone, command ShouldBeSuccess, the expected-value ShouldBeError, ShouldBeAbsent, ShouldBeNull), nor any assertion in a branch of a ?:, ?? or switch expression so wrapped. Tasks.PassesOn now hands on a plain Task's ConfigureAwait and WaitAsync too, and the pending-assertion check runs inside the Producers loop that already judged each branch of a dropped result, so both forms of the rule take one path. Awaited, held, returned and `_ =` uses stay silent. Decision 117 covers the wider reach. Input class: AIS, a call's task dropped by a statement. There was no catalogue for it; Variants/DroppedTaskShapes is new: 25 calls (every Testing assertion on a Task and a ValueTask, and a call returning a Task of a Result) by 9 pass-throughs (none, both ConfigureAwait overloads, every WaitAsync overload, one chained) by 6 forms (the call, the lowered static form, ?., both branches of ?:, ??'s right operand, both arms of a switch) by 6 uses (dropped, a lambda's body, awaited, discarded, held, returned): 6,831 statements, DroppedTaskVariantTests. Seeded three ways: without the plain Task in PassesOn the 12 plain-Task assertions fail; with the assertion check limited to the statement's own call all 24 assertions fail; without the await condition all 24 assertions fail on false reports of their awaited forms (58 for ShouldHold on a Task alone). No sibling beyond #152's two was found. Claim T02-a-wrapped-assertion-is-reported quotes the README's new sentence. Closes #152 Defect-Class: AIS Found-By: OR Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- design/analyzers.md | 2 +- .../IgnoredOutcomeAnalyzer.cs | 26 +- src/CodoMetis.TypeKit.Analyzers/README.md | 6 +- .../DroppedTaskVariantTests.cs | 55 ++++ .../Variants/DroppedTaskShapes.cs | 248 ++++++++++++++++++ test/oracles/claims.json | 16 ++ 7 files changed, 340 insertions(+), 15 deletions(-) create mode 100644 test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs create mode 100644 test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 659912a..f57e481 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ section is the GitHub release's notes. - `TraverseAsync` on `Option` and `Patch`: an asynchronous check of an optional input or a patch field, with a token form; nothing is called without a value ([Collect many results](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/docs/result.md#collect-many-results)). - `Choose()`, `Choose(selector)`, `SequenceAsync` and `TraverseAsync` read an `IAsyncEnumerable`, in order and without a list first; the token reaches the stream, nothing is read after the first error, and the enumerator is disposed however they stop ([Chain steps](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit/README.md#chain-steps)). - `Fault.Cause`: `Fault.Of(reason, detail, exception)` keeps the exception underneath for your log; equality and `ToString()` leave it out, and `ToHttpResult()` hands it to the host as `IFaultFeature.Cause` and `FaultProblem.Cause`, never into the problem ([What failed underneath](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.AspNetCore/README.md#what-failed-underneath)). -- Testing: every assertion works on a `Task` or `ValueTask` of what it checks, `await service.FindAsync(id).ShouldBeSome()`; a task that throws fails the test with its own exception, and an assertion left without `await` is reported (CMTK0003) ([CodoMetis.TypeKit.Testing](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Testing/README.md)). +- Testing: every assertion works on a `Task` or `ValueTask` of what it checks, `await service.FindAsync(id).ShouldBeSome()`; a task that throws fails the test with its own exception, and an assertion left without `await` is reported (CMTK0003), through `ConfigureAwait` or `WaitAsync` and in a branch of a `?:`, `??` or `switch` too ([CodoMetis.TypeKit.Testing](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Testing/README.md)). - OpenAPI: `AddTypeKit(typeKit => typeKit.InlineValueObjects = true)` describes every value object in place by its underlying type's schema, in every position and in OpenAPI 3.1, 3.0 and 2.0, with no component of its own; closed sets keep theirs, and without the setting the document is unchanged ([Describe value objects as plain types](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/docs/aspnet-core.md#describe-value-objects-as-plain-types)). - Analyzers: CMTK0009 warns at `FirstOrNone`, `SingleOrNone` or `FirstOrError` on a query given a `Func` variable or a method group, which binds to the `IEnumerable` overload and reads every row; a lambda at the call or an `Expression` stays silent ([A condition that reads every row](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Analyzers/README.md#a-condition-that-reads-every-row)). - Docs: the guides, the package READMEs, the changelog and the API of every run-time package as one site with a menu and search, built from the latest release ([caffeinatedcoder.github.io/CodoMetis.TypeKit](https://caffeinatedcoder.github.io/CodoMetis.TypeKit/)). diff --git a/design/analyzers.md b/design/analyzers.md index 7c93846..58501a1 100644 --- a/design/analyzers.md +++ b/design/analyzers.md @@ -33,7 +33,7 @@ exist. |---|---|---|---| | CMTK0001 | No `default`/`default(T)`/`new()`/`new T()` of a value object or a `[RequireCustomInitialization]` type. An operand of `==`/`!=`, or an argument of a call whose name has the word `Equal`, `Equals` or `Compare` or ends in `Be` (`ThrowIfEqual`, `Assert.NotEqual`, `ShouldNotBe`), is exempt: a guard check is the only defence against the defaults the rule cannot see. So is a `default` assigned to an `out` parameter of a method returning `bool`, the Try pattern. Where a sibling branch does not bind (same-project value objects), the target type is taken from the enclosing conditional, switch arm or collection | Error | shipped in 1.0.0 | | CMTK0002 | Type implements `IValue<>`/`IValidatedValue<,,>`, but the compilation does not reference `CodoMetis.TypeKit.Generators`, so it is never woven | Error | shipped in 1.0.0 | -| CMTK0003 | `Result`/`Option` a call returns, dropped by an expression statement, awaited or not, or a `Task` of one converted to a plain `Task` (`Task Cancel() => CancelAsync();`); `_ =` opts out. `Tap`, `TapAsync`, `TapError` and `TapErrorAsync` on a stored result or task are exempt, since they return their receiver, and so is a chain of them on one (`r.Tap(log).TapError(warn)`, `(await pending).Tap(log)`, `pending.WaitAsync(t).TapAsync(log)`), measured as an Error's false report before 2.0.0 was tagged; so is such a tap converted to a plain `Task` or waited for (`Task Log() => pending.TapAsync(log);`, `held.TapAsync(log).Wait()`), on a `Result` and on a `Task` of one, which an independent review found reported on 2026-10-14. Also a collection of results the statement made (an array, `ICollection` or `IReadOnlyCollection`, such as `await Task.WhenAll(ids.Select(orders.CancelAsync))`), and a method group of a `Task` method converted to a delegate returning `Task`; tasks held in variables are silent, and so is a lazy `IEnumerable`, where nothing has run, unless a call that was given no result returned it and the statement materialises it. Also `Wait` on a task the expression made, tasks of results the statement made converted to plain ones (`Task.WaitAll`, `WaitAny`), and a `foreach` or `await foreach` over results its header made whose body never reads the iteration variable; a variable named `_` opts out. From 2.3.0, an assertion of `CodoMetis.TypeKit.Testing` on a `Task` or `ValueTask` of what it checks, dropped by an expression statement without `await`: `service.FindAsync(id).ShouldBeNone();`, `service.LoadAsync().ShouldBeSuccess().ConfigureAwait(false);`, the body of a lambda that returns nothing; it fails the test only through its task. Known by the package's assembly, so a consumer's extension of the same name on a pending result is silent; awaited, held, returned or discarded with `_ =`, it is silent too. CA1806 can only enforce this per method name, not per return type | Error | shipped in 1.0.0 at Warning; Error in 2.0.0 ([decision 50](adr/0050-warnings-cmtk0003-alone-becomes-error-2.md)), its 1.x gaps closed first; the dropped pending assertion in 2.3.0, a form only 2.3.0's API writes ([decision 117](adr/0117-an-error-may-learn-a-form-only-new-api-can-write.md)) | +| CMTK0003 | `Result`/`Option` a call returns, dropped by an expression statement, awaited or not, or a `Task` of one converted to a plain `Task` (`Task Cancel() => CancelAsync();`); `_ =` opts out. `Tap`, `TapAsync`, `TapError` and `TapErrorAsync` on a stored result or task are exempt, since they return their receiver, and so is a chain of them on one (`r.Tap(log).TapError(warn)`, `(await pending).Tap(log)`, `pending.WaitAsync(t).TapAsync(log)`), measured as an Error's false report before 2.0.0 was tagged; so is such a tap converted to a plain `Task` or waited for (`Task Log() => pending.TapAsync(log);`, `held.TapAsync(log).Wait()`), on a `Result` and on a `Task` of one, which an independent review found reported on 2026-10-14. Also a collection of results the statement made (an array, `ICollection` or `IReadOnlyCollection`, such as `await Task.WhenAll(ids.Select(orders.CancelAsync))`), and a method group of a `Task` method converted to a delegate returning `Task`; tasks held in variables are silent, and so is a lazy `IEnumerable`, where nothing has run, unless a call that was given no result returned it and the statement materialises it. Also `Wait` on a task the expression made, tasks of results the statement made converted to plain ones (`Task.WaitAll`, `WaitAny`), and a `foreach` or `await foreach` over results its header made whose body never reads the iteration variable; a variable named `_` opts out. From 2.3.0, an assertion of `CodoMetis.TypeKit.Testing` on a `Task` or `ValueTask` of what it checks, dropped by an expression statement without `await`: `service.FindAsync(id).ShouldBeNone();`, the lowered static call, through `?.`, the body of a lambda that returns nothing, through `ConfigureAwait` or `WaitAsync` of the `Task` or `Task` it returns (every overload), and at each branch of a `?:`, `??` or `switch` expression so passed through, judged by the path every dropped call takes (#152, held by `DroppedTaskVariantTests` over `Variants/DroppedTaskShapes`); it fails the test only through its task. Known by the package's assembly, so a consumer's extension of the same name on a pending result is silent; awaited, held, returned or discarded with `_ =`, it is silent too. CA1806 can only enforce this per method name, not per return type | Error | shipped in 1.0.0 at Warning; Error in 2.0.0 ([decision 50](adr/0050-warnings-cmtk0003-alone-becomes-error-2.md)), its 1.x gaps closed first; the dropped pending assertion in 2.3.0, a form only 2.3.0's API writes ([decision 117](adr/0117-an-error-may-learn-a-form-only-new-api-can-write.md)) | | CMTK0004 | `IValueObjectMaterializer<,>.Materialize`, `ValueObjectConverter<,>.Materialize` or a hand-written value object's public `Materialize`, called or referenced (expression trees included). The satellite's own call is compiled in the satellite; EF's compiled model is generated code, whose diagnostics are not reported (measured: reported, it is six errors in the smoke test's AOT consumer). `StoredJsonConverterFactory` goes through `IStoredJsonConverterSource`, not `Materialize` | Error | shipped in 1.0.0 | | CMTK0005 | `new T[n]`, `stackalloc T[n]`, `GC.AllocateUninitializedArray/AllocateArray` or `Array.Resize` of a no-default struct: every new slot starts as `default`. A constant length of zero (for `Array.Resize` and the `GC` methods too) or initial elements are silent | Warning | shipped in 1.0.0 | | CMTK0006 | Field or auto-property of a no-default struct type in a class that no initializer, `required` or constructor sets; a non-public parameterless constructor (EF, serializers) and a record's copy constructor are exempt. Measured 2026-09-28 on the repository's EF entities and request types: 12 of 12 reports on settable properties of a type with only the implicit constructor, each a real way to a default. `required`, the fix the message names where it compiles, works with EF's compiled model, precompiled queries and Native AOT (the smoke test rerun with it). A `[SetsRequiredMembers]` constructor must assign the `required` members it promises | Warning | shipped in 1.0.0 | diff --git a/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs b/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs index 5d4b154..591988b 100644 --- a/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs +++ b/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs @@ -141,29 +141,30 @@ private static void Analyze(OperationAnalysisContext context, TypeKitSymbols sym // task still held, await held.WaitAsync(t), nothing is dropped. var passedOn = tasks.PassedOn(dropped); - var reported = false; foreach (var invocation in Producers(dropped, tasks)) { // The statement's own call keeps the statement's location, and its type unless a // pass-through stands between them; a branch's call keeps its own. var own = ReferenceEquals(invocation, passedOn); var type = own && ReferenceEquals(invocation, dropped) ? expression.Type : awaited is null ? invocation.Type : tasks.Awaited(invocation.Type); - if (DroppedOutcome(type, invocation, symbols, tasks) is not { } outcome) continue; + + // pending.ShouldBeSuccess(); asserts nothing: the assertion fails the test only through its task. + var outcome = DroppedOutcome(type, invocation, symbols, tasks) + ?? (awaited is null && IsPendingAssertion(invocation.TargetMethod, tasks) ? invocation.Type?.Name ?? "Task" : null); + if (outcome is null) continue; context.Report(Diagnostic.Create(Rule, (own ? expression : invocation).Syntax.GetLocation(), outcome, invocation.TargetMethod.Name)); - reported |= own; } - - // pending.ShouldBeSuccess(); asserts nothing: the assertion fails the test only through its task. - if (!reported && awaited is null && passedOn is IInvocationOperation assertion && IsPendingAssertion(assertion.TargetMethod, tasks)) - context.Report(Diagnostic.Create(Rule, expression.Syntax.GetLocation(), assertion.Type?.Name ?? "Task", assertion.TargetMethod.Name)); } /// /// An assertion of CodoMetis.TypeKit.Testing on a Task or ValueTask of the value it /// asserts on, await service.FindAsync(id).ShouldBeSome(): it awaits the value and fails the /// test through the task it returns, so that task dropped unawaited passes whatever the value is. - /// Every such assertion is new in 2.3, so reporting its drop breaks no build that worked before. + /// It is judged as every dropped call is, through : passed through + /// ConfigureAwait or WaitAsync, or reached through a branch of a conditional, a + /// ?? or a switch expression. Every such assertion is new in 2.3, so reporting its drop + /// breaks no build that worked before (design/adr/0117). /// private static bool IsPendingAssertion(IMethodSymbol method, Tasks tasks) => TypeKitSymbols.IsTestingAssembly(method.ContainingAssembly) @@ -446,13 +447,16 @@ public IOperation PassedOn(IOperation operation) } /// - /// ConfigureAwait and WaitAsync of Task<T> or ValueTask<T>, - /// every overload: each completes with the task's own result and makes none. + /// ConfigureAwait and WaitAsync of Task, Task<T> or + /// ValueTask<T>, every overload: each completes as the task does, with its own result, + /// and makes none. A plain Task carries no result, but it can be the one an assertion of + /// CodoMetis.TypeKit.Testing returns (). /// public bool PassesOn(IInvocationOperation call) => call.TargetMethod is { IsStatic: false, Name: "ConfigureAwait" or "WaitAsync" } method && (SymbolEqualityComparer.Default.Equals(method.ContainingType.OriginalDefinition, generic) - || SymbolEqualityComparer.Default.Equals(method.ContainingType.OriginalDefinition, genericValueTask)); + || SymbolEqualityComparer.Default.Equals(method.ContainingType.OriginalDefinition, genericValueTask) + || SymbolEqualityComparer.Default.Equals(method.ContainingType, Plain)); /// A Task or ValueTask of a result, not awaited: the result is dropped with it. public string? PendingOutcome(ITypeSymbol? type, TypeKitSymbols symbols) => symbols.OutcomeName(Awaited(type)); diff --git a/src/CodoMetis.TypeKit.Analyzers/README.md b/src/CodoMetis.TypeKit.Analyzers/README.md index 28ebcba..7c666e4 100644 --- a/src/CodoMetis.TypeKit.Analyzers/README.md +++ b/src/CodoMetis.TypeKit.Analyzers/README.md @@ -189,8 +189,10 @@ conversion to `ValueTask`. **Assertions on a task.** An assertion of `CodoMetis.TypeKit.Testing` on a `Task` or `ValueTask`, such as `service.FindAsync(id).ShouldBeSome()`, fails the test only through the task it returns. Left without `await`, the test passes whatever the task holds, and C# warns only in an `async` -method. CMTK0003 reports the line, naming the assertion. The rule knows these assertions by the -package's own assembly, so an extension of yours on a pending result is left alone. +method. CMTK0003 reports the line, naming the assertion. It also reports an assertion passed through +`ConfigureAwait` or `WaitAsync`, and one in a branch of a `?:`, `??` or `switch` expression, as it does +for a dropped result. The rule knows these assertions by the package's own assembly, so an extension of +yours on a pending result is left alone. **Collections.** A collection of results is dropped as silently as one result: diff --git a/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs b/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs new file mode 100644 index 0000000..e6312d9 --- /dev/null +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs @@ -0,0 +1,55 @@ +using CodoMetis.TypeKit.Analyzers.Tests.Variants; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; + +namespace CodoMetis.TypeKit.Analyzers.Tests; + +/// +/// CMTK0003 over : a call's task dropped in every form and through every +/// pass-through is reported, once per call that made it, and kept in any other use is silent. +/// +/// +/// The oracle is the rule's own statement, which each shape holds to: the statement's own call is reported +/// whole, a branch's call at the branch, and an awaited task only where what it completes with is a result. +/// An assertion of CodoMetis.TypeKit.Testing and a call that returns a task of a Result run through +/// the same shapes, so the two forms of the rule agree on each (#152). +/// +public sealed class DroppedTaskVariantTests +{ + public static TheoryData Calls() => [.. DroppedTaskShapes.Calls.Select(call => call.Name)]; + + [Theory] + [MemberData(nameof(Calls))] + public Task A_dropped_task_is_reported_in_every_form_and_a_kept_one_never(string name) + { + var call = DroppedTaskShapes.Calls.Single(candidate => candidate.Name == name); + var (source, expected) = DroppedTaskShapes.Use(call); + + var test = new AnalyzerTest(source); + test.TestState.Sources.Add(("Declarations.cs", DroppedTaskShapes.Declarations)); + test.TestState.AdditionalReferences.Add(RealTesting.Reference); + test.ExpectedDiagnostics.AddRange( + Enumerable.Range(0, expected).Select(location => + new DiagnosticResult("CMTK0003", DiagnosticSeverity.Error).WithLocation(location).WithArguments(call.Outcome, call.Method))); + + return test.RunAsync(TestContext.Current.CancellationToken); + } + + /// Every axis of the catalogue reaches the sweep: a shape that compiles to no case would pass as silent. + [Fact] + public void Every_shape_makes_a_case() + { + foreach (var call in DroppedTaskShapes.Calls) + { + var (source, expected) = DroppedTaskShapes.Use(call); + + foreach (var passThrough in DroppedTaskShapes.PassThroughs.Where(passThrough => passThrough.Suffix.Length > 0)) + source.ShouldContain(passThrough.Suffix, customMessage: $"{call}: no case passes through {passThrough}."); + + foreach (var form in DroppedTaskShapes.Forms.Where(form => DroppedTaskShapes.Applies(form, call))) + source.ShouldContain($"// {form.Name}, ", customMessage: $"{call}: no case of {form}."); + + expected.ShouldBeGreaterThan(0, $"{call}: nothing is expected to be reported."); + } + } +} diff --git a/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs b/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs new file mode 100644 index 0000000..5db07e1 --- /dev/null +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs @@ -0,0 +1,248 @@ +namespace CodoMetis.TypeKit.Analyzers.Tests.Variants; + +/// A call that returns a task whose drop CMTK0003 judges. +/// What the call is, as a test case names it. +/// The call as written on the run method's parameters. +/// The same call in the lowered static form, or where it has none. +/// The same call through ?., or where its receiver is a struct. +/// The type the call returns, which a ??'s held operand is declared as. +/// The method the message names. +/// What the message says is dropped. +/// +/// Whether awaiting it drops something too: a task of a Result completes with the result, which the +/// statement then drops; an assertion completes once it has passed, with nothing left to see. +/// +public sealed record PendingCall(string Name, string Call, string? Static, string? NullConditional, string Returns, string Method, string Outcome, bool AwaitDrops) +{ + public override string ToString() => Name; +} + +/// A call on a task that hands on the task it is called on, or none. +/// What the call is, as a test case names it. +/// The call as written after the task. +public sealed record TaskPassThrough(string Name, string Suffix) +{ + public override string ToString() => Name; +} + +/// Where the dropped call stands in the statement's expression. +/// What the form is, as a test case names it. +/// +/// The expression: {call} is the call, {w} the pass-through, and [[ ]] enclose a +/// branch's call, which a diagnostic reports on its own. A form without them is the statement's own call, +/// reported whole. +/// +/// Whether the form is a statement only with a pass-through: a bare ?:, ?? or switch is CS0201. +public sealed record DropForm(string Name, string Expression, bool NeedsPassThrough = false) +{ + public bool IsBranch => Expression.Contains("[[", StringComparison.Ordinal); + + public override string ToString() => Name; +} + +/// What the statement does with the expression. +/// What the use is, as a test case names it. +/// {e} is the expression, and << >> enclose what a diagnostic reports for the statement's own call. +/// Whether the use drops the task: where it drops what the task completes with, which only holds anything of. +/// Whether the use needs an async method. +public sealed record TaskUse(string Name, string Statement, bool? Drops, bool Async = false) +{ + public override string ToString() => Name; +} + +/// +/// The variant catalogue for CMTK0003's dropped task (design/adr/0095): every way a statement can leave a +/// call's task unobserved, and the uses that keep it. +/// +/// +/// +/// Four axes. The call: every assertion of CodoMetis.TypeKit.Testing on a Task and a ValueTask, +/// those that return a plain Task and those that return a Task<T>, beside a call that returns a +/// task of a Result, whose drop the rule judged first. The pass-through: none, and every overload of +/// ConfigureAwait and WaitAsync, one chained onto another. The form: the call alone, in the lowered +/// static form, through ?., and as a branch of a ?:, a ?? and a switch expression. The +/// use: dropped as a statement or as a lambda's body, against awaited, discarded, held and returned. +/// +/// +/// Both calls go through the one path, so they agree on every form: #152 found the assertions alone silent +/// through the pass-throughs of a plain Task and through the branches. +/// +/// +public static class DroppedTaskShapes +{ + /// The run method's parameters, which the calls and the forms name. + public const string Parameters = + "Service service, Service? maybe, bool choose, CancellationToken token, " + + "Task> option, ValueTask> optionValue, " + + "Task> result, ValueTask> resultValue, " + + "Task> command, ValueTask> commandValue, " + + "Task> patch, ValueTask> patchValue, " + + "Task> composed, ValueTask> composedValue"; + + /// The declarations the parameters name, a file of their own. + public const string Declarations = + """ + using System.Threading.Tasks; + using CodoMetis.TypeKit; + + public enum Fault { Refused } + + public enum Other { Late } + + public sealed class Service + { + public Task> LoadAsync() => Task.FromResult>(1); + } + + """; + + public static IReadOnlyList Calls { get; } = + [ + .. Assertion("ShouldBeNone", "OptionAssertions", "option", "", "Task"), + .. Assertion("ShouldBeSuccess", "ResultAssertions", "command", "", "Task", "command"), + .. Assertion("ShouldBeError", "ResultAssertions", "result", "Fault.Refused", "Task", "expected-value"), + .. Assertion("ShouldBeError", "ResultAssertions", "command", "Fault.Refused", "Task", "command expected-value"), + .. Assertion("ShouldBeAbsent", "PatchAssertions", "patch", "", "Task"), + .. Assertion("ShouldBeNull", "PatchAssertions", "patch", "", "Task"), + .. Assertion("ShouldBeSome", "OptionAssertions", "option", "", "Task"), + .. Assertion("ShouldBeSuccess", "ResultAssertions", "result", "", "Task"), + .. Assertion("ShouldBeError", "ResultAssertions", "result", "", "Task"), + .. Assertion("ShouldBeError", "ResultAssertions", "command", "", "Task", "command"), + .. Assertion("ShouldBeValue", "PatchAssertions", "patch", "", "Task"), + .. Assertion("ShouldHold", "FaultAssertions", "composed", "Fault.Refused", "Task>"), + + new("a call that returns a Task of a Result", "service.LoadAsync()", null, "maybe?.LoadAsync()", "Task>", "LoadAsync", "Result", AwaitDrops: true), + ]; + + public static IReadOnlyList PassThroughs { get; } = + [ + new("no pass-through", ""), + new("ConfigureAwait(bool)", ".ConfigureAwait(false)"), + new("ConfigureAwait(ConfigureAwaitOptions)", ".ConfigureAwait(ConfigureAwaitOptions.None)"), + new("WaitAsync(TimeSpan)", ".WaitAsync(TimeSpan.FromSeconds(1))"), + new("WaitAsync(CancellationToken)", ".WaitAsync(token)"), + new("WaitAsync(TimeSpan, CancellationToken)", ".WaitAsync(TimeSpan.FromSeconds(1), token)"), + new("WaitAsync(TimeSpan, TimeProvider)", ".WaitAsync(TimeSpan.FromSeconds(1), TimeProvider.System)"), + new("WaitAsync(TimeSpan, TimeProvider, CancellationToken)", ".WaitAsync(TimeSpan.FromSeconds(1), TimeProvider.System, token)"), + new("WaitAsync then ConfigureAwait", ".WaitAsync(token).ConfigureAwait(false)"), + ]; + + public static IReadOnlyList Forms { get; } = + [ + new("the call", "{call}{w}"), + new("the lowered static form", "{static}{w}"), + new("a null-conditional call", "{nc}{w}"), + new("both branches of a conditional", "(choose ? [[{call}]] : [[{call}]]){w}", NeedsPassThrough: true), + new("the right operand of ??", "(held ?? [[{call}]]){w}", NeedsPassThrough: true), + new("both arms of a switch expression", "(choose switch { true => [[{call}]], false => [[{call}]] }){w}", NeedsPassThrough: true), + ]; + + public static IReadOnlyList Uses { get; } = + [ + new("dropped as a statement", "<<{e}>>;", Drops: true), + new("dropped as a lambda's body", "Action dropped = () => <<{e}>>;", Drops: true), + new("awaited", "<>;", Drops: null, Async: true), + new("discarded with _ =", "_ = {e};", Drops: false), + new("held in a local", "var kept = {e};", Drops: false), + new("returned from a lambda", "Func returned = () => {e};", Drops: false), + ]; + + /// + /// A file whose two run methods use in every form, through every pass-through, in + /// every use, and the spans a diagnostic is expected at, by markup location. + /// + public static (string Source, int Expected) Use(PendingCall call) + { + var expected = 0; + var body = new System.Text.StringBuilder(); + var awaited = new System.Text.StringBuilder(); + + foreach (var form in Forms) + foreach (var passThrough in PassThroughs) + foreach (var use in Uses) + { + // A statement that does not compile is no case. + if (form.NeedsPassThrough && passThrough.Suffix.Length == 0) continue; + if (!Applies(form, call)) continue; + + // await x?.M().ConfigureAwait(false) awaits a nullable awaiter, which has no GetAwaiter, and await x?.M() + // a task that may be null (CS8602). + if (use.Async && form.Expression.Contains("{nc}", StringComparison.Ordinal)) continue; + + var reported = use.Drops ?? call.AwaitDrops; + var expression = form.Expression + .Replace("{call}", call.Call) + .Replace("{static}", call.Static) + .Replace("{nc}", call.NullConditional) + .Replace("{w}", passThrough.Suffix); + var statement = use.Statement.Replace("{e}", expression); + + statement = Mark(statement, "<<", ">>", reported && !form.IsBranch, ref expected); + statement = Mark(statement, "[[", "]]", reported, ref expected); + + // ??'s left operand is a task the caller holds, declared where the form names it. + var held = expression.Contains("held ??", StringComparison.Ordinal) ? $"{call.Returns}? held = choose ? null : {call.Call};\n " : ""; + + (use.Async ? awaited : body).Append($" {{ // {form.Name}, {passThrough.Name}, {use.Name}\n ") + .Append(held) + .Append(statement) + .Append("\n }\n"); + } + + var source = + $$""" + #nullable enable + using System; + using System.Threading; + using System.Threading.Tasks; + using CodoMetis.TypeKit; + using CodoMetis.TypeKit.Testing; + + public static class Use + { + public static void Run({{Parameters}}) + { + {{body}} + } + + public static async Task RunAsync({{Parameters}}) + { + {{awaited}} + } + } + """; + + return (source, expected); + } + + /// Whether can be written in : a call has no static form, a ValueTask no ?.. + public static bool Applies(DropForm form, PendingCall call) => + !(form.Expression.Contains("{static}", StringComparison.Ordinal) && call.Static is null) + && !(form.Expression.Contains("{nc}", StringComparison.Ordinal) && call.NullConditional is null); + + /// Each span between and becomes the next markup location, or plain text where nothing is reported. + private static string Mark(string text, string open, string close, bool reported, ref int next) + { + var marked = new System.Text.StringBuilder(); + var at = 0; + + for (var start = text.IndexOf(open, StringComparison.Ordinal); start >= 0; start = text.IndexOf(open, at, StringComparison.Ordinal)) + { + var end = text.IndexOf(close, start, StringComparison.Ordinal); + marked.Append(text, at, start - at).Append(reported ? $"{{|#{next++}:" : "").Append(text, start + open.Length, end - start - open.Length).Append(reported ? "|}" : ""); + at = end + close.Length; + } + + return marked.Append(text, at, text.Length - at).ToString(); + } + + /// An assertion, on a Task of what it checks and on a ValueTask. + private static IEnumerable Assertion(string method, string @class, string receiver, string arguments, string returns, string? kind = null) + { + var name = kind is null ? method : $"{kind} {method}"; + var tail = arguments.Length == 0 ? "" : ", " + arguments; + + yield return new($"{name} on a Task", $"{receiver}.{method}({arguments})", $"{@class}.{method}({receiver}{tail})", $"{receiver}?.{method}({arguments})", returns, method, "Task", AwaitDrops: false); + yield return new($"{name} on a ValueTask", $"{receiver}Value.{method}({arguments})", $"{@class}.{method}({receiver}Value{tail})", null, returns, method, "Task", AwaitDrops: false); + } +} diff --git a/test/oracles/claims.json b/test/oracles/claims.json index 344b875..dc47531 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -4758,6 +4758,22 @@ "IgnoredOutcomeAnalyzerTests" ] }, + { + "id": "T02-a-wrapped-assertion-is-reported", + "source": "src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0003", + "quote": "It also reports an assertion passed through `ConfigureAwait` or `WaitAsync`, and one in a branch of a `?:`, `??` or `switch` expression, as it does for a dropped result.", + "claim": "It also reports an assertion passed through `ConfigureAwait` or `WaitAsync`, and one in a branch of a `?:`, `??` or `switch` expression, as it does for a dropped result.", + "oracle": "claim", + "surfaces": [ + "OptionAssertions.*", + "ResultAssertions.*", + "PatchAssertions.*", + "FaultAssertions.*" + ], + "heldBy": [ + "DroppedTaskVariantTests" + ] + }, { "id": "W01-answers-are-real", "source": "docs/walkthrough.md#walkthrough-one-orders-service", From 9e45d271cb1b7d10838ee4b5be5567724a767446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Thu, 29 Oct 2026 19:10:48 +0100 Subject: [PATCH 2/3] fix: CMTK0003 reports assertions and results lost through Task's combinators An assertion of CodoMetis.TypeKit.Testing fails a test only through its task. CMTK0003 now reports one handed to Task.WhenAll, Task.Run or Task.WhenEach whose task is dropped, and one given to Task.WhenAny or Task.WaitAny, which finish with the first task and never show a failure, awaited or not. It counts arguments, arrays, collection expressions, a Select, a query, a ToList, a method group and an async lambda that awaits one (no catch between). Tasks the caller holds, an assertion discarded where it is made and one given to a call that returns no task stay silent; Task.WaitAll throws, so it is silent. A WhenAny of assertions converted to a plain Task or waited for, and a loop that never reads the assertions its header made, are reported too (#157). For tasks of results, the same sweep found four forms 2.2.0 missed, which ship in 2.3.0 under decision 0119: a dropped Task.WhenAny (Tasks.PendingOutcome now sees a task of a task, #159), Wait() on a ?:, ?? or switch (AnalyzeWait goes through Producers, #160), an array written in Task.WaitAll or WaitAny (each element's call, #161) and a dropped Task.WhenEach (#162). A WhenAny of held tasks returns one of them and is now silent, where 2.2.0 reported it awaited. Input class: AIS, a call's task dropped or lost by a statement. Variants/DroppedTaskShapes now has 25 calls by 9 pass-throughs by 39 forms by 11 uses, run one compilation per call and form (830 cases, DroppedTaskVariantTests): a single compilation per call overflowed the verifier's recursive matching. Seeded 18 ways, each caught: without the combinators 378 cases fail, without HandsOn's checks 48, a task of a task one level deep 3 plus the WhenAny fact, WhenAny of held tasks not stored 1 plus the fact, Wait on the own call only 3, arrays 2, method groups 18, the await jump 48, the catch check 24, Task.Run 48, WhenEach 25, the assertion loop 48, the conversion's assertion path 72, WaitAny 48, query syntax 24, calls that return no task 24, Wait's assertion path 72, WhenEach of results 1. Claims T02-combined-assertions-are-reported, T02-wait-all-observes-assertions and T02-when-any-of-results-is-reported quote the README. Defect-Class: AIS Found-By: VA Co-Authored-By: Claude Opus 5.5 --- .../IgnoredOutcomeAnalyzer.cs | 213 ++++++++++++++++-- src/CodoMetis.TypeKit.Analyzers/README.md | 19 +- .../DroppedTaskVariantTests.cs | 39 +++- .../IgnoredOutcomeAnalyzerTests.cs | 33 +++ .../Variants/DroppedTaskShapes.cs | 188 +++++++++++++--- test/oracles/claims.json | 46 ++++ 6 files changed, 472 insertions(+), 66 deletions(-) diff --git a/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs b/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs index 561d782..d0369b0 100644 --- a/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs +++ b/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs @@ -148,15 +148,106 @@ private static void Analyze(OperationAnalysisContext context, TypeKitSymbols sym var own = ReferenceEquals(invocation, passedOn); var type = own && ReferenceEquals(invocation, dropped) ? expression.Type : awaited is null ? invocation.Type : tasks.Awaited(invocation.Type); - // pending.ShouldBeSuccess(); asserts nothing: the assertion fails the test only through its task. - var outcome = DroppedOutcome(type, invocation, symbols, tasks) - ?? (awaited is null && IsPendingAssertion(invocation.TargetMethod, tasks) ? invocation.Type?.Name ?? "Task" : null); + var outcome = DroppedOutcome(type, invocation, symbols, tasks) ?? DroppedAssertion(invocation, awaited is not null, tasks); if (outcome is null) continue; context.Report(Diagnostic.Create(Rule, (own ? expression : invocation).Syntax.GetLocation(), outcome, invocation.TargetMethod.Name)); } } + /// + /// The task that fails a test, dropped, or awaited where says so (the caller + /// of a conversion to a plain Task and Wait wait for it too): pending.ShouldBeSuccess(); + /// asserts nothing, since the assertion fails the test only through its task (). + /// Nor does Task.WhenAll(first.ShouldBeNone(), second.ShouldBeNone()); or Task.Run(() => + /// pending.ShouldBeNone());, whose task faults when an assertion fails, or Task.WhenAny of them, + /// awaited or not, whose task completes with the first to finish and leaves every failure unobserved, when + /// the statement made an assertion it hands on (). + /// + private static string? DroppedAssertion(IInvocationOperation invocation, bool awaited, Tasks tasks) + { + var method = invocation.TargetMethod; + + var dropped = IsPendingAssertion(method, tasks) ? !awaited + : tasks.PassesOnFailure(method) is { } faults && (!awaited || !faults) + && invocation.Arguments.Any(argument => MadeAssertions(argument.Value, tasks).Any()); + + if (!dropped) return null; + + // Task.WhenEach(first.ShouldBeNone(), second.ShouldBeNone()) yields them unobserved. + return tasks.ElementOfSequence(invocation.Type) is { } element ? $"sequence of {element.Name}s" : invocation.Type?.Name ?? "Task"; + } + + /// + /// The assertions makes and hands on the task of, ' way, as + /// an element of an array or a collection, or from a lambda or a method group a call that returns tasks was + /// given: new[] { a.ShouldBeNone(), b.ShouldBeNone() }, ids.Select(id => Find(id).ShouldBeNone()), + /// pending.Select(OptionAssertions.ShouldBeNone), ids.Select(async id => await Find(id).ShouldBeNone()). + /// An assertion assigned or discarded in it keeps its own task, and one awaited fails only the + /// async lambda it is in, which is handed on in its place. + /// + private static IEnumerable<(IOperation Assertion, string Name)> MadeAssertions(IOperation value, Tasks tasks) + { + foreach (var operation in value.DescendantsAndSelf()) + { + var method = operation switch + { + IInvocationOperation call => call.TargetMethod, + IMethodReferenceOperation reference => reference.Method, + _ => null, + }; + + if (method is not null && IsPendingAssertion(method, tasks) && HandsOn(operation, value, tasks)) yield return (operation, method.Name); + } + } + + private static bool HandsOn(IOperation operation, IOperation root, Tasks tasks) + { + for (var current = operation; !ReferenceEquals(current, root); current = current.Parent!) + { + // await pending.ShouldBeNone() in an async lambda fails the lambda's task: follow that instead, + // unless a catch on the way can swallow the failure. + if (current.Parent is IAwaitOperation) + { + if (EnclosingAsyncFunction(current, root) is not { } function) return false; + + current = function; + if (ReferenceEquals(current, root)) return true; + } + + var handsOn = current.Parent switch + { + IConversionOperation or IConditionalOperation or ICoalesceOperation or ISwitchExpressionArmOperation or ISwitchExpressionOperation => true, + IArrayInitializerOperation or IArrayCreationOperation or ICollectionExpressionOperation or IArgumentOperation => true, + IReturnOperation or IBlockOperation or IAnonymousFunctionOperation or IDelegateCreationOperation or ITranslatedQueryOperation => true, + IInvocationOperation call when ReferenceEquals(call.Instance, current) => tasks.PassesOn(call), + IInvocationOperation call => tasks.HoldsTasks(call.Type), + _ => false, + }; + + if (!handsOn) return false; + } + + return true; + } + + /// The async lambda or anonymous method inside that fails, with no catch between them. + private static IOperation? EnclosingAsyncFunction(IOperation operation, IOperation root) + { + for (var current = operation.Parent; current is not null && !ReferenceEquals(current, root); current = current.Parent) + { + switch (current) + { + case ITryOperation { Catches.Length: > 0 }: + return null; + case IAnonymousFunctionOperation function: + return function.Symbol.IsAsync ? function : null; + } + } + + return null; + } + /// /// An assertion of CodoMetis.TypeKit.Testing on a Task or ValueTask of the value it /// asserts on, await service.FindAsync(id).ShouldBeSome(): it awaits the value and fails the @@ -213,6 +304,10 @@ private static IEnumerable Producers(IOperation operation, if ((symbols.OutcomeName(type) ?? tasks.PendingOutcome(type, symbols)) is { } outcome) return ReturnsItsStoredReceiver(invocation, symbols, tasks) ? null : outcome; + // Task.WhenEach(first.CancelAsync(), second.CancelAsync()) yields the tasks it was given, results and all. + if (tasks.PassesOnFailure(invocation.TargetMethod) == false && tasks.SequenceOutcome(type, symbols) is { } element) + return DroppedHolder($"sequence of {element}s", invocation, symbols, tasks); + return DroppedHolder(tasks.HolderOutcome(type, symbols) ?? tasks.HolderOutcome(tasks.Awaited(type), symbols), invocation, symbols, tasks); } @@ -269,6 +364,21 @@ private static void AnalyzeConversion(OperationAnalysisContext context, TypeKitS var toPlainTask = SymbolEqualityComparer.Default.Equals(conversion.Type, tasks.Plain); if (!toPlainTask && !tasks.HoldsPlainTasks(conversion.Type)) return; + // Task.WaitAll(new[] { first.CancelAsync(), second.CancelAsync() }) converts the array of tasks whole: + // each element's call is converted as if alone. + if (!toPlainTask && Unwrap(conversion.Operand) is IArrayCreationOperation { Initializer.ElementValues: var elements }) + { + foreach (var element in elements) + foreach (var invocation in Producers(element, tasks)) + { + if (ReturnsItsStoredReceiver(invocation, symbols, tasks) || tasks.PendingOutcome(invocation.Type, symbols) is not { } pending) continue; + + context.Report(Diagnostic.Create(Rule, invocation.Syntax.GetLocation(), pending, invocation.TargetMethod.Name)); + } + + return; + } + // The call, or each branch's call where a conditional or a switch expression was converted whole. // A pass-through between the conversion and the call, LoadAsync().WaitAsync(t), is reported whole. var passedOn = tasks.PassedOn(conversion.Operand); @@ -276,9 +386,11 @@ private static void AnalyzeConversion(OperationAnalysisContext context, TypeKitS foreach (var invocation in Producers(conversion.Operand, tasks)) { // held.TapAsync(log) returns the task of a result the caller still holds. + // Task Check() => Task.WhenAny(a.ShouldBeNone(), b.ShouldBeNone()); is awaited as a task that never faults. var outcome = toPlainTask ? (ReturnsItsStoredReceiver(invocation, symbols, tasks) ? null : tasks.PendingOutcome(invocation.Type, symbols)) ?? DroppedHolder(tasks.HolderOutcome(tasks.Awaited(invocation.Type), symbols), invocation, symbols, tasks) + ?? DroppedAssertion(invocation, awaited: true, tasks) : DroppedCollection(tasks.PendingOutcome(tasks.ElementOfSequence(invocation.Type), symbols), invocation, symbols, tasks); if (outcome is null) continue; @@ -292,40 +404,70 @@ private static void AnalyzeConversion(OperationAnalysisContext context, TypeKitS /// Wait on a task of a result, or of a collection of results, that the expression made: /// orders.CancelAsync(id).Wait() blocks until the result exists and hands it to nobody, /// whether the statement drops what Wait returns or an if reads it. A task still held, - /// or a tap on one, keeps its result. + /// or a tap on one, keeps its result. Each branch of a conditional, a ?? or a switch + /// expression is its own call, as in a dropped statement. Wait on a Task.WhenAny of + /// assertions waits for the first and never sees a failure, and so does Task.WaitAny. /// private static void AnalyzeWait(OperationAnalysisContext context, TypeKitSymbols symbols, Tasks tasks) { var wait = (IInvocationOperation)context.Operation; - if (wait.TargetMethod is not { Name: "Wait", IsStatic: false } method || !SymbolEqualityComparer.Default.Equals(method.ContainingType, tasks.Plain)) return; - if (wait.Instance is null || tasks.PassedOn(wait.Instance) is not IInvocationOperation invocation) return; + if (wait.TargetMethod is not { Name: "Wait" or "WaitAny" } method || !SymbolEqualityComparer.Default.Equals(method.ContainingType, tasks.Plain)) return; - if (ReturnsItsStoredReceiver(invocation, symbols, tasks)) return; + // Task.WaitAny(a.ShouldBeNone(), b.ShouldBeNone()) returns an index and leaves every failure + // unobserved; Task.WaitAll throws them. Tasks of results are its conversions' (AnalyzeConversion). + if (method.IsStatic) + { + if (method.Name != "WaitAny") return; - var outcome = tasks.PendingOutcome(invocation.Type, symbols) - ?? DroppedHolder(tasks.HolderOutcome(tasks.Awaited(invocation.Type), symbols), invocation, symbols, tasks); - if (outcome is null) return; + foreach (var argument in wait.Arguments.Where(argument => argument.Parameter?.Ordinal == 0)) + foreach (var (assertion, name) in MadeAssertions(argument.Value, tasks)) + context.Report(Diagnostic.Create(Rule, assertion.Syntax.GetLocation(), "Task", name)); + + return; + } + + if (method.Name != "Wait" || wait.Instance is null) return; + + // The task's own call is reported at the Wait, each branch's call at the branch. + var passedOn = tasks.PassedOn(wait.Instance); + + foreach (var invocation in Producers(wait.Instance, tasks)) + { + if (ReturnsItsStoredReceiver(invocation, symbols, tasks)) continue; + + var outcome = tasks.PendingOutcome(invocation.Type, symbols) + ?? DroppedHolder(tasks.HolderOutcome(tasks.Awaited(invocation.Type), symbols), invocation, symbols, tasks) + ?? DroppedAssertion(invocation, awaited: true, tasks); + if (outcome is null) continue; - context.Report(Diagnostic.Create(Rule, wait.Syntax.GetLocation(), outcome, invocation.TargetMethod.Name)); + context.Report(Diagnostic.Create(Rule, (ReferenceEquals(invocation, passedOn) ? wait : invocation).Syntax.GetLocation(), outcome, invocation.TargetMethod.Name)); + } } /// /// A foreach or await foreach over results, or tasks of them, that its header made, /// whose body never reads the iteration variable: await foreach (var outcome in - /// orders.CancelEachAsync(ids)) { } runs every cancellation and drops every result. A variable + /// orders.CancelEachAsync(ids)) { } runs every cancellation and drops every result. So does one + /// over assertions the header made, Task.WhenEach of them or a Select, whose failures nobody + /// sees. A variable /// named _ says the drop is intended; a collection held in a variable, a parameter or a field /// keeps its results. /// private static void AnalyzeLoop(OperationAnalysisContext context, TypeKitSymbols symbols, Tasks tasks) { if (context.Operation is not IForEachLoopOperation { LoopControlVariable: IVariableDeclaratorOperation { Symbol: { Name: not "_" } variable } } loop) return; - if ((symbols.OutcomeName(variable.Type) ?? tasks.PendingOutcome(variable.Type, symbols)) is not { } outcome) return; - if (Unwrap(loop.Collection) is not IInvocationOperation invocation || !MadeResults(invocation, symbols, tasks)) return; + if (Unwrap(loop.Collection) is not IInvocationOperation invocation) return; + + // foreach (var check in pending.Select(p => p.ShouldBeNone())) { } makes every assertion and observes none. + var outcome = (symbols.OutcomeName(variable.Type) ?? tasks.PendingOutcome(variable.Type, symbols)) is { } element + ? MadeResults(invocation, symbols, tasks) ? $"sequence of {element}s" : null + : tasks.IsTask(variable.Type) && MadeAssertions(invocation, tasks).Any() ? $"sequence of {variable.Type.Name}s" : null; + if (outcome is null) return; if (loop.Body.Descendants().OfType().Any(reference => SymbolEqualityComparer.Default.Equals(reference.Local, variable))) return; - context.Report(Diagnostic.Create(Rule, invocation.Syntax.GetLocation(), $"sequence of {outcome}s", invocation.TargetMethod.Name)); + context.Report(Diagnostic.Create(Rule, invocation.Syntax.GetLocation(), outcome, invocation.TargetMethod.Name)); } /// @@ -386,7 +528,8 @@ private static IOperation Unwrap(IOperation operation) => /// /// A call that returns its receiver unchanged, on a receiver the caller still holds: the /// outcome's own Tap, TapAsync, TapError and TapErrorAsync, and the - /// Task continuations TapAsync and TapErrorAsync, which return the task's result as it was. + /// Task continuations TapAsync and TapErrorAsync, which return the task's result as it was; + /// and Task.WhenAny of tasks the caller still holds, which completes with one of them. /// private static bool ReturnsItsStoredReceiver(IInvocationOperation invocation, TypeKitSymbols symbols, Tasks tasks) { @@ -400,9 +543,22 @@ private static bool ReturnsItsStoredReceiver(IInvocationOperation invocation, Ty if (method.Name is "TapAsync" or "TapErrorAsync" && symbols.IsDeclaredByResultClass(method)) return IsStored(invocation.Instance ?? invocation.Arguments.FirstOrDefault(argument => argument.Parameter?.Ordinal == 0)?.Value, symbols, tasks); + // Task.WhenAny(first, second) completes with one of the tasks it was given, results and all. + if (tasks.PassesOnFailure(method) == false) + return invocation.Arguments.All(argument => Elements(argument.Value).All(element => IsStored(element, symbols, tasks))); + return false; } + /// The elements of an array or a collection written out, such as a params argument's, or the value itself. + private static IEnumerable Elements(IOperation value) => + Unwrap(value) switch + { + IArrayCreationOperation { Initializer.ElementValues: var elements } => elements, + ICollectionExpressionOperation { Elements: var elements } => elements, + var other => [other], + }; + /// /// Whether the caller still holds : a local, a parameter or a field; a /// tap that returned such a receiver, so that r.Tap(log).TapError(warn) holds at every link; @@ -458,8 +614,29 @@ public bool PassesOn(IInvocationOperation call) => || SymbolEqualityComparer.Default.Equals(method.ContainingType.OriginalDefinition, genericValueTask) || SymbolEqualityComparer.Default.Equals(method.ContainingType, Plain)); - /// A Task or ValueTask of a result, not awaited: the result is dropped with it. - public string? PendingOutcome(ITypeSymbol? type, TypeKitSymbols symbols) => symbols.OutcomeName(Awaited(type)); + /// + /// For a method that hands on the tasks it is given: for Task.WhenAll and + /// Task.Run, whose task faults when one of them has; for + /// Task.WhenAny, whose task completes with the first and never faults, and Task.WhenEach, + /// which yields them as they complete, faulted or not; for any other method. + /// + public bool? PassesOnFailure(IMethodSymbol method) => + method is { IsStatic: true, Name: "WhenAll" or "WhenAny" or "WhenEach" or "Run" } && SymbolEqualityComparer.Default.Equals(method.ContainingType, Plain) + ? method.Name is "WhenAll" or "Run" + : null; + + /// Whether a value of is a task, or a collection or a sequence of them. + public bool HoldsTasks(ITypeSymbol? type) => + IsTask(type) || IsTask(ElementOfSequence(type)); + + public bool IsTask(ITypeSymbol? type) => SymbolEqualityComparer.Default.Equals(type, Plain) || Awaited(type) is not null; + + /// + /// A Task or ValueTask of a result, not awaited: the result is dropped with it. So is a task + /// of one, such as the Task<Task<Result<…>>> of Task.WhenAny, which completes + /// with a task of a result. + /// + public string? PendingOutcome(ITypeSymbol? type, TypeKitSymbols symbols) => symbols.OutcomeName(Awaited(type)) ?? symbols.OutcomeName(Awaited(Awaited(type))); /// What a Task<T> or ValueTask<T> completes with, or for any other type. public ITypeSymbol? Awaited(ITypeSymbol? type) => diff --git a/src/CodoMetis.TypeKit.Analyzers/README.md b/src/CodoMetis.TypeKit.Analyzers/README.md index 7c666e4..0b409f0 100644 --- a/src/CodoMetis.TypeKit.Analyzers/README.md +++ b/src/CodoMetis.TypeKit.Analyzers/README.md @@ -194,6 +194,13 @@ method. CMTK0003 reports the line, naming the assertion. It also reports an asse for a dropped result. The rule knows these assertions by the package's own assembly, so an extension of yours on a pending result is left alone. +Handing assertions on doesn't hide them either. You get the report when you hand assertions to +`Task.WhenAll`, `Task.Run` or `Task.WhenEach` and drop what it returns. You get it for `Task.WhenAny` and +`Task.WaitAny` of them even when you wait, because those finish with the first task and never show a +failure. It covers an array of them, a `Select`, a query, an `async` lambda that awaits one, and a method +group such as `pending.Select(OptionAssertions.ShouldBeNone)`. `Task.WaitAll` throws the failures, so it is +silent. So is a loop over assertions that reads each one. + **Collections.** A collection of results is dropped as silently as one result: ```csharp @@ -238,10 +245,16 @@ await foreach (var _ in orders.CancelEachAsync(ids)) { } // a variable n ``` - **`Wait`** is reported on a task the expression made, whether the statement drops the `bool` a - timeout overload returns or an `if` reads it. + timeout overload returns or an `if` reads it. On a `?:`, `??` or `switch` expression, each branch's + call is reported, as when you drop it. - **`Task.WaitAll` and `Task.WaitAny`** take plain tasks. So tasks of results that the statement made - are converted and their results dropped, as one task's conversion is. An explicit cast to the plain - tasks' type says you mean that drop. + are converted and their results dropped, as one task's conversion is. That includes an array you + write in the call, `Task.WaitAll(new[] { orders.CancelAsync(first), orders.CancelAsync(second) })`. An + explicit cast to the plain tasks' type says you mean that drop. +- **`Task.WhenAny`** finishes with one of the tasks you give it, so every other result is lost. A + `WhenAny` of tasks the statement made is reported whether you drop it, await it, wait for it or return + it as a plain `Task`. Of tasks you still hold, it hands back one of yours, and is silent. +- **`Task.WhenEach`** of tasks the statement made, dropped, is reported as a sequence of results. - **A `foreach` or `await foreach`** is reported when its header made the results, by the rule above, and its body never reads the iteration variable. diff --git a/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs b/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs index e6312d9..4f3544e 100644 --- a/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs @@ -10,46 +10,61 @@ namespace CodoMetis.TypeKit.Analyzers.Tests; /// /// /// The oracle is the rule's own statement, which each shape holds to: the statement's own call is reported -/// whole, a branch's call at the branch, and an awaited task only where what it completes with is a result. +/// whole, a branch's call at the branch, a combination of assertions at the combinator, and an awaited task +/// only where what it completes with is a result, or a WhenAny that leaves every failure unobserved. /// An assertion of CodoMetis.TypeKit.Testing and a call that returns a task of a Result run through /// the same shapes, so the two forms of the rule agree on each (#152). /// public sealed class DroppedTaskVariantTests { - public static TheoryData Calls() => [.. DroppedTaskShapes.Calls.Select(call => call.Name)]; + public static TheoryData Cases() + { + var cases = new TheoryData(); + foreach (var call in DroppedTaskShapes.Calls) + foreach (var form in DroppedTaskShapes.Forms.Where(form => DroppedTaskShapes.Applies(form, call))) + cases.Add(call.Name, form.Name); + + return cases; + } [Theory] - [MemberData(nameof(Calls))] - public Task A_dropped_task_is_reported_in_every_form_and_a_kept_one_never(string name) + [MemberData(nameof(Cases))] + public Task A_dropped_task_is_reported_in_every_form_and_a_kept_one_never(string name, string shape) { var call = DroppedTaskShapes.Calls.Single(candidate => candidate.Name == name); - var (source, expected) = DroppedTaskShapes.Use(call); + var form = DroppedTaskShapes.Forms.Single(candidate => candidate.Name == shape); + var (source, expected) = DroppedTaskShapes.Use(call, form); var test = new AnalyzerTest(source); test.TestState.Sources.Add(("Declarations.cs", DroppedTaskShapes.Declarations)); test.TestState.AdditionalReferences.Add(RealTesting.Reference); test.ExpectedDiagnostics.AddRange( - Enumerable.Range(0, expected).Select(location => - new DiagnosticResult("CMTK0003", DiagnosticSeverity.Error).WithLocation(location).WithArguments(call.Outcome, call.Method))); + expected.Select((message, location) => + new DiagnosticResult("CMTK0003", DiagnosticSeverity.Error).WithLocation(location).WithArguments(message.Outcome, message.Method))); return test.RunAsync(TestContext.Current.CancellationToken); } - /// Every axis of the catalogue reaches the sweep: a shape that compiles to no case would pass as silent. + /// + /// Every axis of the catalogue reaches the sweep, and each call has a form that reports and one that keeps: + /// a shape that compiles to no case would pass as silent. + /// [Fact] public void Every_shape_makes_a_case() { foreach (var call in DroppedTaskShapes.Calls) { - var (source, expected) = DroppedTaskShapes.Use(call); + var uses = DroppedTaskShapes.Forms.Where(form => DroppedTaskShapes.Applies(form, call)).Select(form => DroppedTaskShapes.Use(call, form)).ToList(); + var source = string.Concat(uses.Select(use => use.Source)); foreach (var passThrough in DroppedTaskShapes.PassThroughs.Where(passThrough => passThrough.Suffix.Length > 0)) source.ShouldContain(passThrough.Suffix, customMessage: $"{call}: no case passes through {passThrough}."); - foreach (var form in DroppedTaskShapes.Forms.Where(form => DroppedTaskShapes.Applies(form, call))) - source.ShouldContain($"// {form.Name}, ", customMessage: $"{call}: no case of {form}."); + foreach (var use in DroppedTaskShapes.Uses) + source.ShouldContain($", {use.Name}\n", customMessage: $"{call}: no case is {use}."); - expected.ShouldBeGreaterThan(0, $"{call}: nothing is expected to be reported."); + uses.ShouldContain(use => use.Expected.Count > 0, $"{call}: nothing is expected to be reported."); + uses.Where(use => use.Source.Contains("{|#", StringComparison.Ordinal)).Count().ShouldBeLessThan(uses.Count, $"{call}: every form reports, so no form keeps the task."); } } } diff --git a/test/CodoMetis.TypeKit.Analyzers.Tests/IgnoredOutcomeAnalyzerTests.cs b/test/CodoMetis.TypeKit.Analyzers.Tests/IgnoredOutcomeAnalyzerTests.cs index 0dbbd27..b9364d0 100644 --- a/test/CodoMetis.TypeKit.Analyzers.Tests/IgnoredOutcomeAnalyzerTests.cs +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/IgnoredOutcomeAnalyzerTests.cs @@ -267,6 +267,39 @@ public static void Run(Patch quantity, Patch nickname, List se """) .RunAsync(TestContext.Current.CancellationToken); + /// + /// Task.WhenAny completes with a task of a result: dropped, awaited or not, converted to a plain + /// Task or waited for, the result of every task the statement made is gone (#159). Of tasks the + /// caller holds it completes with one of them, and drops nothing. + /// + [Fact] + public Task A_WhenAny_of_results_the_statement_made_reports_and_of_held_ones_is_silent() => + Test( + """ + public static class Caller + { + public static async Task Run(Service service, Task> held, Task> other, Task>[] all) + { + {|#0:Task.WhenAny(service.LoadAsync(), service.LoadAsync())|}; + {|#1:await Task.WhenAny(held, service.LoadAsync())|}; + {|#2:Task.WhenAny(service.LoadAsync(), held).Wait()|}; + await Task.WhenAny(held, other); + Task.WhenAny(held, other); + await Task.WhenAny(all).ConfigureAwait(false); + _ = Task.WhenAny(service.LoadAsync(), service.LoadAsync()); + } + + public static Task Converted(Service service) => {|#3:Task.WhenAny(service.LoadAsync(), service.LoadAsync())|}; + + public static Task Held(Task> held, Task> other) => Task.WhenAny(held, other); + } + """, + Cmtk0003(0, "Result", "WhenAny"), + Cmtk0003(1, "Result", "WhenAny"), + Cmtk0003(2, "Result", "WhenAny"), + Cmtk0003(3, "Result", "WhenAny")) + .RunAsync(TestContext.Current.CancellationToken); + /// /// ConfigureAwait and WaitAsync hand on the task they are called on: a task the caller /// still holds keeps its result through them, as it does through await held;. They were diff --git a/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs b/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs index 5db07e1..6a9eb36 100644 --- a/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs @@ -12,7 +12,9 @@ namespace CodoMetis.TypeKit.Analyzers.Tests.Variants; /// Whether awaiting it drops something too: a task of a Result completes with the result, which the /// statement then drops; an assertion completes once it has passed, with nothing left to see. /// -public sealed record PendingCall(string Name, string Call, string? Static, string? NullConditional, string Returns, string Method, string Outcome, bool AwaitDrops) +/// Whether it is an assertion of CodoMetis.TypeKit.Testing, whose combination the message names as a Task. +/// A sequence of the calls made through a method group, or where the call takes more than its receiver. +public sealed record PendingCall(string Name, string Call, string? Static, string? NullConditional, string Returns, string Method, string Outcome, bool AwaitDrops, bool IsAssertion = true, string? Group = null) { public override string ToString() => Name; } @@ -20,7 +22,8 @@ public sealed record PendingCall(string Name, string Call, string? Static, strin /// A call on a task that hands on the task it is called on, or none. /// What the call is, as a test case names it. /// The call as written after the task. -public sealed record TaskPassThrough(string Name, string Suffix) +/// Whether what it returns is still a task, as WaitAsync's is and ConfigureAwait's awaitable is not. +public sealed record TaskPassThrough(string Name, string Suffix, bool KeepsTask = true) { public override string ToString() => Name; } @@ -33,19 +36,63 @@ public sealed record TaskPassThrough(string Name, string Suffix) /// reported whole. /// /// Whether the form is a statement only with a pass-through: a bare ?:, ?? or switch is CS0201. -public sealed record DropForm(string Name, string Expression, bool NeedsPassThrough = false) +/// +/// The Task method that combines the calls, which the message names in place of the call's, or +/// . A WhenAll of results drops a collection of them, and a WhenAny one +/// result (#159). +/// +/// Whether awaiting the combination of assertions still drops a failure, which WhenAny's does; where the call decides. +/// Whether nothing is dropped in any use: the statement combines no task it made. +/// What the expression is: a task, or what Task.WaitAny and Task.WaitAll return, which loses the tasks in every use. +/// Whether an assertion's failure is seen all the same: Task.WaitAll throws it. +/// Whether only an assertion is written in the form, where a Result's would be a statement of its own. +/// Whether the sequence runs nothing until a loop reads it, so that dropping it drops nothing. +public sealed record DropForm( + string Name, + string Expression, + bool NeedsPassThrough = false, + string? Combinator = null, + bool? AwaitDrops = null, + bool Silent = false, + FormValue Value = FormValue.Task, + bool AssertionsObserved = false, + bool AssertionsOnly = false, + bool Lazy = false) { public bool IsBranch => Expression.Contains("[[", StringComparison.Ordinal); public override string ToString() => Name; } +/// What a form's expression is. +public enum FormValue +{ + /// A task, or what a pass-through makes of one. + Task, + + /// The index Task.WaitAny returns. + Index, + + /// Nothing: Task.WaitAll returns void. + Nothing, + + /// A sequence of the tasks, read with foreach: a Select, which runs nothing until it is read. + Sequence, + + /// A sequence of the tasks, read with await foreach: Task.WhenEach's. + AsyncSequence, +} + /// What the statement does with the expression. /// What the use is, as a test case names it. /// {e} is the expression, and << >> enclose what a diagnostic reports for the statement's own call. /// Whether the use drops the task: where it drops what the task completes with, which only holds anything of. /// Whether the use needs an async method. -public sealed record TaskUse(string Name, string Statement, bool? Drops, bool Async = false) +/// Whether the use needs a task: it converts it to a plain Task or calls Wait on it. +/// Whether the use writes a call after the expression, which binds inside a ?.. +/// Whether the expression is a statement, so that one of no value can be written there. +/// The sequence a loop reads the expression as, or for a use that is no loop. +public sealed record TaskUse(string Name, string Statement, bool? Drops, bool Async = false, bool NeedsTask = false, bool Chains = false, bool IsStatement = false, FormValue? Loops = null) { public override string ToString() => Name; } @@ -60,12 +107,18 @@ public sealed record TaskUse(string Name, string Statement, bool? Drops, bool As /// those that return a plain Task and those that return a Task<T>, beside a call that returns a /// task of a Result, whose drop the rule judged first. The pass-through: none, and every overload of /// ConfigureAwait and WaitAsync, one chained onto another. The form: the call alone, in the lowered -/// static form, through ?., and as a branch of a ?:, a ?? and a switch expression. The -/// use: dropped as a statement or as a lambda's body, against awaited, discarded, held and returned. +/// static form, through ?., as a branch of a ?:, a ?? and a switch expression, and +/// handed on by Task.WhenAll, Task.WhenAny or Task.Run as arguments, an array, a collection +/// expression, a Select of a lambda, an async lambda or a method group, beside a held task and as a +/// branch, and given to Task.WaitAny or Task.WaitAll, against held tasks, an assertion discarded +/// where it was made and one whose failure a catch swallows. The use: dropped as a statement or as a +/// lambda's body, awaited, converted to a plain Task and waited for with Wait, against discarded, +/// held and returned. /// /// /// Both calls go through the one path, so they agree on every form: #152 found the assertions alone silent -/// through the pass-throughs of a plain Task and through the branches. +/// through the pass-throughs of a plain Task and through the branches, #157 through the combinators, +/// and #159 a WhenAny of results dropped unawaited. /// /// public static class DroppedTaskShapes @@ -111,20 +164,20 @@ .. Assertion("ShouldBeError", "ResultAssertions", "command", "", "Task", .. Assertion("ShouldBeValue", "PatchAssertions", "patch", "", "Task"), .. Assertion("ShouldHold", "FaultAssertions", "composed", "Fault.Refused", "Task>"), - new("a call that returns a Task of a Result", "service.LoadAsync()", null, "maybe?.LoadAsync()", "Task>", "LoadAsync", "Result", AwaitDrops: true), + new("a call that returns a Task of a Result", "service.LoadAsync()", null, "maybe?.LoadAsync()", "Task>", "LoadAsync", "Result", AwaitDrops: true, IsAssertion: false), ]; public static IReadOnlyList PassThroughs { get; } = [ new("no pass-through", ""), - new("ConfigureAwait(bool)", ".ConfigureAwait(false)"), - new("ConfigureAwait(ConfigureAwaitOptions)", ".ConfigureAwait(ConfigureAwaitOptions.None)"), + new("ConfigureAwait(bool)", ".ConfigureAwait(false)", KeepsTask: false), + new("ConfigureAwait(ConfigureAwaitOptions)", ".ConfigureAwait(ConfigureAwaitOptions.None)", KeepsTask: false), new("WaitAsync(TimeSpan)", ".WaitAsync(TimeSpan.FromSeconds(1))"), new("WaitAsync(CancellationToken)", ".WaitAsync(token)"), new("WaitAsync(TimeSpan, CancellationToken)", ".WaitAsync(TimeSpan.FromSeconds(1), token)"), new("WaitAsync(TimeSpan, TimeProvider)", ".WaitAsync(TimeSpan.FromSeconds(1), TimeProvider.System)"), new("WaitAsync(TimeSpan, TimeProvider, CancellationToken)", ".WaitAsync(TimeSpan.FromSeconds(1), TimeProvider.System, token)"), - new("WaitAsync then ConfigureAwait", ".WaitAsync(token).ConfigureAwait(false)"), + new("WaitAsync then ConfigureAwait", ".WaitAsync(token).ConfigureAwait(false)", KeepsTask: false), ]; public static IReadOnlyList Forms { get; } = @@ -135,53 +188,112 @@ .. Assertion("ShouldHold", "FaultAssertions", "composed", "Fault.Refused", "Task new("both branches of a conditional", "(choose ? [[{call}]] : [[{call}]]){w}", NeedsPassThrough: true), new("the right operand of ??", "(held ?? [[{call}]]){w}", NeedsPassThrough: true), new("both arms of a switch expression", "(choose switch { true => [[{call}]], false => [[{call}]] }){w}", NeedsPassThrough: true), + + new("Task.WhenAll of two calls", "Task.WhenAll({call}, {call}){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll of an array", "Task.WhenAll(new[] { {call}, {call} }){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll of a collection expression", "Task.WhenAll([{call}, {call}]){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll of a Select", "Task.WhenAll(Enumerable.Range(0, 2).Select(_ => {call})){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll beside a held task", "Task.WhenAll(held!, {call}){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll of a conditional", "Task.WhenAll(choose ? {call} : {call}){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAny of two calls", "Task.WhenAny({call}, {call}){w}", Combinator: "WhenAny", AwaitDrops: true), + new("Task.WhenAny of an array", "Task.WhenAny(new[] { {call}, {call} }){w}", Combinator: "WhenAny", AwaitDrops: true), + new("Task.WhenAny beside a held task", "Task.WhenAny(held!, {call}){w}", Combinator: "WhenAny", AwaitDrops: true), + new("Task.WhenAll of held tasks", "Task.WhenAll(held!, held!){w}", Combinator: "WhenAll", Silent: true), + new("Task.WhenAny of held tasks", "Task.WhenAny(held!, held!){w}", Combinator: "WhenAny", Silent: true), + new("Task.WhenAll of tasks discarded where they are made", "Task.WhenAll(Enumerable.Range(0, 2).Select(i => { _ = {call}; return Task.CompletedTask; })){w}", Combinator: "WhenAll", Silent: true), + new("Task.WhenAll of tasks made from the calls, not of them", "Task.WhenAll(Enumerable.Range(0, 2).Select(i => Task.FromResult(ReferenceEquals({call}, null)))){w}", Combinator: "WhenAll", Silent: true), + new("Task.WhenAll of a method group", "Task.WhenAll({group}){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll of async lambdas", "Task.WhenAll(Enumerable.Range(0, 2).Select(async i => await {call})){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll of async lambdas that catch", "Task.WhenAll(Enumerable.Range(0, 2).Select(async i => { try { await {call}; } catch (Exception) { } })){w}", Combinator: "WhenAll", Silent: true, AssertionsOnly: true), + new("Task.Run of a lambda", "Task.Run(() => {call}){w}", Combinator: "Run", AwaitDrops: false), + new("Task.Run of an async lambda", "Task.Run(async () => await {call}){w}", Combinator: "Run", AwaitDrops: false), + + new("Task.WaitAny of two calls", "Task.WaitAny([[{call}]], [[{call}]])", Value: FormValue.Index), + new("Task.WaitAny of held tasks", "Task.WaitAny(held!, held!)", Value: FormValue.Index, Silent: true), + new("Task.WaitAll of two calls", "Task.WaitAll([[{call}]], [[{call}]])", Value: FormValue.Nothing, AssertionsObserved: true), + new("Task.WaitAny of an array", "Task.WaitAny(new[] { [[{call}]], [[{call}]] })", Value: FormValue.Index), + new("Task.WaitAll of an array", "Task.WaitAll(new[] { [[{call}]], [[{call}]] })", Value: FormValue.Nothing, AssertionsObserved: true), + + new("Task.WhenAll of a query", "Task.WhenAll(from i in Enumerable.Range(0, 2) select {call}){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenAll of a list", "Task.WhenAll(Enumerable.Range(0, 2).Select(i => {call}).ToList()){w}", Combinator: "WhenAll", AwaitDrops: false), + new("Task.WhenEach of two calls", "Task.WhenEach({call}, {call})", Combinator: "WhenEach", Value: FormValue.AsyncSequence), + new("Task.WhenEach of held tasks", "Task.WhenEach(held!, held!)", Combinator: "WhenEach", Value: FormValue.AsyncSequence, Silent: true), + new("a Select of the calls", "Enumerable.Range(0, 2).Select(i => {call})", Combinator: "Select", Value: FormValue.Sequence, Lazy: true), ]; public static IReadOnlyList Uses { get; } = [ - new("dropped as a statement", "<<{e}>>;", Drops: true), - new("dropped as a lambda's body", "Action dropped = () => <<{e}>>;", Drops: true), + new("dropped as a statement", "<<{e}>>;", Drops: true, IsStatement: true), + new("dropped as a lambda's body", "Action dropped = () => <<{e}>>;", Drops: true, IsStatement: true), new("awaited", "<>;", Drops: null, Async: true), + new("converted to a plain Task", "Func converted = () => <<{e}>>;", Drops: null, NeedsTask: true), + new("waited for with Wait()", "<<{e}.Wait()>>;", Drops: null, NeedsTask: true, Chains: true), + new("the body of a method that returns nothing", "void Check() => <<{e}>>;", Drops: true, IsStatement: true), + new("drained by a foreach that never reads it", "foreach (var each in <<{e}>>) { }", Drops: true, Loops: FormValue.Sequence), + new("read by a foreach", "foreach (var each in {e}) { GC.KeepAlive(each); }", Drops: false, Loops: FormValue.Sequence), + new("drained by an await foreach that never reads it", "await foreach (var each in <<{e}>>) { }", Drops: true, Async: true, Loops: FormValue.AsyncSequence), + new("read by an await foreach", "await foreach (var each in {e}) { GC.KeepAlive(each); }", Drops: false, Async: true, Loops: FormValue.AsyncSequence), new("discarded with _ =", "_ = {e};", Drops: false), new("held in a local", "var kept = {e};", Drops: false), new("returned from a lambda", "Func returned = () => {e};", Drops: false), ]; /// - /// A file whose two run methods use in every form, through every pass-through, in - /// every use, and the spans a diagnostic is expected at, by markup location. + /// A file whose two run methods use in , through every + /// pass-through, in every use, and the spans a diagnostic is expected at, by markup location. One file per + /// form keeps a failure to the form, and the verifier's matching, which recurses once per diagnostic, + /// within its stack. /// - public static (string Source, int Expected) Use(PendingCall call) + public static (string Source, IReadOnlyList<(string Outcome, string Method)> Expected) Use(PendingCall call, DropForm form) { - var expected = 0; + var expected = new List<(string Outcome, string Method)>(); var body = new System.Text.StringBuilder(); var awaited = new System.Text.StringBuilder(); - foreach (var form in Forms) foreach (var passThrough in PassThroughs) foreach (var use in Uses) { // A statement that does not compile is no case. if (form.NeedsPassThrough && passThrough.Suffix.Length == 0) continue; - if (!Applies(form, call)) continue; // await x?.M().ConfigureAwait(false) awaits a nullable awaiter, which has no GetAwaiter, and await x?.M() - // a task that may be null (CS8602). - if (use.Async && form.Expression.Contains("{nc}", StringComparison.Ordinal)) continue; - - var reported = use.Drops ?? call.AwaitDrops; + // a task that may be null (CS8602); x?.M().Wait() waits inside the ?.. + if ((use.Async || use.Chains) && form.Expression.Contains("{nc}", StringComparison.Ordinal)) continue; + if (use.NeedsTask && !passThrough.KeepsTask) continue; + + // An index, nothing or a sequence takes no await, no pass-through and no conversion, nothing is no + // value, and a loop reads only the sequence it was written for. + if (use.Loops is { } loops ? loops != form.Value : form.Value != FormValue.Task && (use.Async || use.NeedsTask)) continue; + if (form.Value != FormValue.Task && passThrough.Suffix.Length > 0) continue; + if (form.Value == FormValue.Nothing && !use.IsStatement) continue; + + // Task.WaitAny and Task.WaitAll lose or keep the tasks at the call, whatever the statement does next. + var reported = !form.Silent + && !(form.AssertionsObserved && call.IsAssertion) + && !(form.Lazy && use.Loops is null) + && (form.Value is FormValue.Index or FormValue.Nothing || (use.Drops ?? (call.AwaitDrops || (form.AwaitDrops ?? false)))); var expression = form.Expression .Replace("{call}", call.Call) .Replace("{static}", call.Static) .Replace("{nc}", call.NullConditional) + .Replace("{group}", call.Group) .Replace("{w}", passThrough.Suffix); var statement = use.Statement.Replace("{e}", expression); - statement = Mark(statement, "<<", ">>", reported && !form.IsBranch, ref expected); - statement = Mark(statement, "[[", "]]", reported, ref expected); + var message = form.Combinator switch + { + null => (call.Outcome, call.Method), + "WhenEach" or "Select" => ($"sequence of {(call.IsAssertion ? "Task" : call.Outcome)}s", form.Combinator), + var all when call.IsAssertion => ("Task", all), + "WhenAll" => ($"collection of {call.Outcome}s", "WhenAll"), + var any => (call.Outcome, any), + }; + + statement = Mark(statement, "<<", ">>", reported && !form.IsBranch, message, expected); + statement = Mark(statement, "[[", "]]", reported, message, expected); - // ??'s left operand is a task the caller holds, declared where the form names it. - var held = expression.Contains("held ??", StringComparison.Ordinal) ? $"{call.Returns}? held = choose ? null : {call.Call};\n " : ""; + // A task the caller holds, declared where the form names it. + var held = expression.Contains("held", StringComparison.Ordinal) ? $"{call.Returns}? held = choose ? null : {call.Call};\n " : ""; (use.Async ? awaited : body).Append($" {{ // {form.Name}, {passThrough.Name}, {use.Name}\n ") .Append(held) @@ -193,6 +305,7 @@ public static (string Source, int Expected) Use(PendingCall call) $$""" #nullable enable using System; + using System.Linq; using System.Threading; using System.Threading.Tasks; using CodoMetis.TypeKit; @@ -215,13 +328,19 @@ public static async Task RunAsync({{Parameters}}) return (source, expected); } - /// Whether can be written in : a call has no static form, a ValueTask no ?.. + /// + /// Whether can be written in : a call has no static form, a + /// ValueTask no ?., an assertion with arguments no method group, and a Result stays out of + /// a form written for assertions. + /// public static bool Applies(DropForm form, PendingCall call) => - !(form.Expression.Contains("{static}", StringComparison.Ordinal) && call.Static is null) + !(form.AssertionsOnly && !call.IsAssertion) + && !(form.Expression.Contains("{group}", StringComparison.Ordinal) && call.Group is null) + && !(form.Expression.Contains("{static}", StringComparison.Ordinal) && call.Static is null) && !(form.Expression.Contains("{nc}", StringComparison.Ordinal) && call.NullConditional is null); /// Each span between and becomes the next markup location, or plain text where nothing is reported. - private static string Mark(string text, string open, string close, bool reported, ref int next) + private static string Mark(string text, string open, string close, bool reported, (string Outcome, string Method) message, List<(string Outcome, string Method)> expected) { var marked = new System.Text.StringBuilder(); var at = 0; @@ -229,7 +348,8 @@ private static string Mark(string text, string open, string close, bool reported for (var start = text.IndexOf(open, StringComparison.Ordinal); start >= 0; start = text.IndexOf(open, at, StringComparison.Ordinal)) { var end = text.IndexOf(close, start, StringComparison.Ordinal); - marked.Append(text, at, start - at).Append(reported ? $"{{|#{next++}:" : "").Append(text, start + open.Length, end - start - open.Length).Append(reported ? "|}" : ""); + marked.Append(text, at, start - at).Append(reported ? $"{{|#{expected.Count}:" : "").Append(text, start + open.Length, end - start - open.Length).Append(reported ? "|}" : ""); + if (reported) expected.Add(message); at = end + close.Length; } @@ -242,7 +362,9 @@ private static IEnumerable Assertion(string method, string @class, var name = kind is null ? method : $"{kind} {method}"; var tail = arguments.Length == 0 ? "" : ", " + arguments; - yield return new($"{name} on a Task", $"{receiver}.{method}({arguments})", $"{@class}.{method}({receiver}{tail})", $"{receiver}?.{method}({arguments})", returns, method, "Task", AwaitDrops: false); - yield return new($"{name} on a ValueTask", $"{receiver}Value.{method}({arguments})", $"{@class}.{method}({receiver}Value{tail})", null, returns, method, "Task", AwaitDrops: false); + string? Group(string held) => arguments.Length == 0 ? $"new[] {{ {held}, {held} }}.Select({@class}.{method})" : null; + + yield return new($"{name} on a Task", $"{receiver}.{method}({arguments})", $"{@class}.{method}({receiver}{tail})", $"{receiver}?.{method}({arguments})", returns, method, "Task", AwaitDrops: false, Group: Group(receiver)); + yield return new($"{name} on a ValueTask", $"{receiver}Value.{method}({arguments})", $"{@class}.{method}({receiver}Value{tail})", null, returns, method, "Task", AwaitDrops: false, Group: Group(receiver + "Value")); } } diff --git a/test/oracles/claims.json b/test/oracles/claims.json index dc47531..5f063e3 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -4774,6 +4774,52 @@ "DroppedTaskVariantTests" ] }, + { + "id": "T02-combined-assertions-are-reported", + "source": "src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0003", + "quote": "You get the report when you hand assertions to `Task.WhenAll`, `Task.Run` or `Task.WhenEach` and drop what it returns.", + "claim": "You get the report when you hand assertions to `Task.WhenAll`, `Task.Run` or `Task.WhenEach` and drop what it returns.", + "oracle": "claim", + "surfaces": [ + "OptionAssertions.*", + "ResultAssertions.*", + "PatchAssertions.*", + "FaultAssertions.*" + ], + "heldBy": [ + "DroppedTaskVariantTests" + ] + }, + { + "id": "T02-wait-all-observes-assertions", + "source": "src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0003", + "quote": "`Task.WaitAll` throws the failures, so it is silent.", + "claim": "`Task.WaitAll` throws the failures, so it is silent.", + "oracle": "claim", + "surfaces": [ + "OptionAssertions.*", + "ResultAssertions.*", + "PatchAssertions.*", + "FaultAssertions.*" + ], + "heldBy": [ + "DroppedTaskVariantTests" + ] + }, + { + "id": "T02-when-any-of-results-is-reported", + "source": "src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0003", + "quote": "A `WhenAny` of tasks the statement made is reported whether you drop it, await it, wait for it or return it as a plain `Task`.", + "claim": "A `WhenAny` of tasks the statement made is reported whether you drop it, await it, wait for it or return it as a plain `Task`.", + "oracle": "claim", + "surfaces": [ + "Result.*" + ], + "heldBy": [ + "IgnoredOutcomeAnalyzerTests", + "DroppedTaskVariantTests" + ] + }, { "id": "W01-answers-are-real", "source": "docs/walkthrough.md#walkthrough-one-orders-service", From e97af4062068fbc56eab22fa0afc4d01d1481f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Thu, 29 Oct 2026 19:14:45 +0100 Subject: [PATCH 3/3] docs: decision 119, a gap an Error has found before the tag ships in that release The maintainer widened decision 118's one exception: a form an Error misses, found before its version is tagged, is reported in that version when it loses what the rule protects in every build, though code built on the previous minor can contain it. 2.3.0's cases are #159 to #162; the rule stops at the tag, after which decision 34 holds. 0118's index row says it is widened by 0119. The Analyzers README's "What a new version may change" lists the four 2.2-writable forms and what to write instead, the changelog's Changed section says an upgrade can fail a build with them, the CMTK0003 row of design/analyzers.md lists every form the PR reaches, and claim T02-a-few-2-2-forms-are-reported quotes the README. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +- ...nd-before-the-tag-ships-in-that-release.md | 40 +++++++++++++++++++ design/adr/README.md | 3 +- design/analyzers.md | 6 +-- src/CodoMetis.TypeKit.Analyzers/README.md | 14 +++++-- test/oracles/claims.json | 14 +++++++ 6 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 design/adr/0119-a-gap-found-before-the-tag-ships-in-that-release.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 965e81b..72e37d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ section is the GitHub release's notes. - `TraverseAsync` on `Option` and `Patch`: an asynchronous check of an optional input or a patch field, with a token form; nothing is called without a value ([Collect many results](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/docs/result.md#collect-many-results)). - `Choose()`, `Choose(selector)`, `SequenceAsync` and `TraverseAsync` read an `IAsyncEnumerable`, in order and without a list first; the token reaches the stream, nothing is read after the first error, and the enumerator is disposed however they stop ([Chain steps](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit/README.md#chain-steps)). - `Fault.Cause`: `Fault.Of(reason, detail, exception)` keeps the exception underneath for your log; equality and `ToString()` leave it out, and `ToHttpResult()` hands it to the host as `IFaultFeature.Cause` and `FaultProblem.Cause`, never into the problem ([What failed underneath](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.AspNetCore/README.md#what-failed-underneath)). -- Testing: every assertion works on a `Task` or `ValueTask` of what it checks, `await service.FindAsync(id).ShouldBeSome()`; a task that throws fails the test with its own exception, and an assertion left without `await` is reported (CMTK0003), through `ConfigureAwait` or `WaitAsync` and in a branch of a `?:`, `??` or `switch` too ([CodoMetis.TypeKit.Testing](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Testing/README.md)). +- Testing: every assertion works on a `Task` or `ValueTask` of what it checks, `await service.FindAsync(id).ShouldBeSome()`; a task that throws fails the test with its own exception, and an assertion left without `await` is reported (CMTK0003): through `ConfigureAwait` or `WaitAsync`, in a branch of a `?:`, `??` or `switch`, and handed to `Task.WhenAll`, `Task.Run` or `Task.WhenEach` and dropped, or to `Task.WhenAny` or `Task.WaitAny` ([CodoMetis.TypeKit.Testing](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Testing/README.md)). - OpenAPI: `AddTypeKit(typeKit => typeKit.InlineValueObjects = true)` describes every value object in place by its underlying type's schema, in every position and in OpenAPI 3.1, 3.0 and 2.0, with no component of its own; closed sets keep theirs, and without the setting the document is unchanged ([Describe value objects as plain types](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/docs/aspnet-core.md#describe-value-objects-as-plain-types)). - Analyzers: CMTK0009 warns at `FirstOrNone`, `SingleOrNone` or `FirstOrError` on a query given a `Func` variable or a method group, which binds to the `IEnumerable` overload and reads every row; a lambda at the call or an `Expression` stays silent ([A condition that reads every row](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Analyzers/README.md#a-condition-that-reads-every-row)). - Docs: the guides, the package READMEs, the changelog and the API of every run-time package as one site with a menu and search, built from the latest release ([caffeinatedcoder.github.io/CodoMetis.TypeKit](https://caffeinatedcoder.github.io/CodoMetis.TypeKit/)). @@ -30,6 +30,7 @@ section is the GitHub release's notes. ### Changed +- Analyzers: CMTK0003 now reports four forms that code built on 2.2.0 can contain, so your build can fail with it when you upgrade. They are a `Task.WhenAny` of `Result` tasks you don't hold (dropped, awaited, waited for or returned as a plain `Task`), a dropped `Task.WhenEach` of them, `Wait()` on a `?:`, `??` or `switch` of them, and an array of them written in a `Task.WaitAll` or `Task.WaitAny` call. Each one loses results that nobody reads: handle them, or discard them with `_ =`. An awaited `Task.WhenAny` of tasks you hold is no longer reported ([What a new version may change](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Analyzers/README.md#configuration)). - EF Core: a default value-object key over `ulong` gets a temporary value at `Add`, as a plain `ulong` key does, where it threw `OverflowException` on SQL Server and PostgreSQL; a `char` key configured `ValueGeneratedOnAdd()` gets a temporary `char` on SQL Server, not a character of a GUID string. HiLo and your own generator still come first. A compiled model made with `--nativeaot`, and a key you configure with `ValueObjectConverter` yourself, still throw ([Keys](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.EntityFrameworkCore/README.md#keys)). - Packages: nuget.org's Release Notes tab and Visual Studio's Updates tab show the version's section of this changelog, as plain text, where they showed a link to the file. - Changelog: a released version's links name its tag, `blob/v2.2.0/`, so its notes open the pages as they were released, not as they read on `main` today. diff --git a/design/adr/0119-a-gap-found-before-the-tag-ships-in-that-release.md b/design/adr/0119-a-gap-found-before-the-tag-ships-in-that-release.md new file mode 100644 index 0000000..b48c8fd --- /dev/null +++ b/design/adr/0119-a-gap-found-before-the-tag-ships-in-that-release.md @@ -0,0 +1,40 @@ +# 0119. A gap an Error has, found before the tag, ships in that release + +Date: 2026-10-29 +Status: accepted + +Context: 2026-10-29, the sweep of CMTK0003 before 2.3.0 was tagged (#152, #157, #159 to #162); refines +[decision 34](0034-errors-fixed-within-major-version-warnings.md) and +[decision 118](0118-an-error-may-learn-a-form-only-new-api-can-write.md). Decision 118 lets a minor version's +Error report forms that only that version's new API can write, and names one exception the maintainer took +into 2.3.0: a dropped `Task.WhenAny` of tasks of results (#159). The sweep that followed found three more +forms of the same kind, each written with APIs 2.2.0 had. The maintainer decided, in these words: "decision +0119 widens 0118 to 'a gap found before the tag ships in that release'". + +## Decision + +**A form an Error misses, found before its version is tagged, is reported in that version**, when the form +loses what the rule protects in every build that contains it. A build that worked on the previous minor +version can then fail on update. The failure names a defect, never correct code, and the changelog's +Changed section says which forms and what to write instead. Decision 34 keeps an Error fixed within a major +so that an update never fails correct code; it does not keep a defect found before the tag unreported for a +major version. + +**What 2.3.0 reports that 2.2.0 code can contain**, CMTK0003 on tasks of results: + +- a `Task.WhenAny` of tasks the statement made, dropped, awaited, waited for or converted to a plain `Task` + (#159): `Tasks.PendingOutcome` looked one task deep, and the `Task>` hid the result; +- `Wait()` on a `?:`, `??` or `switch` expression, at each branch's call (#160): `AnalyzeWait` read only the + statement's own call, where every other path goes through `Producers`; +- an array of them written in a `Task.WaitAll` or `Task.WaitAny` call (#161), which converts whole, so no + element's call was converted on its own; +- a dropped `Task.WhenEach` of them (#162), which yields the tasks it was given. + +**It relaxes too.** A `Task.WhenAny` of tasks the caller holds completes with one of them and loses nothing. +2.2.0 reported it awaited; 2.3.0 is silent on it. + +**The rule stops at the tag.** Once a version is tagged, decision 34 holds for it: an Error that reports +more code its major version could already write waits for the next major, 3.0.0 for 2.x. Decision 118 +stands for forms only a minor version's new API writes, and its one exception is this decision's first +case. The Analyzers README's "What a new version may change" lists each form a minor version reports that +the previous one's code can contain. diff --git a/design/adr/README.md b/design/adr/README.md index 4d325d4..2f184f3 100644 --- a/design/adr/README.md +++ b/design/adr/README.md @@ -124,4 +124,5 @@ per area ([design](../plan.md)) link to the records they rest on. | [0115](0115-value-objects-can-be-inlined-everywhere.md) | `AddTypeKit(typeKit => typeKit.InlineValueObjects = true)` writes every value object in place, by its wrapped type's schema, in every position and version, with no component of its own; off by default, the document unchanged; closed sets and wrapped enums keep their components; an inlined value object claims no name (decision 55) and shares no schema between its JSON positions and its parameters (decision 106) | 2026-10-29 | accepted | | [0116](0116-the-docs-site-is-docfx-built-from-main.md) | The docs site is DocFX, published to GitHub Pages from `main` alone: the guides, the READMEs, the changelog and the API pages of the run-time packages, from the files as they are; a link to a file that is not a page becomes a GitHub address at the built commit; a pull request touching a page builds it with warnings as errors; `site/**` is prose; DocFX 2.81.0 lists the members of C# 14 extension blocks as extension methods | 2026-10-29 | accepted | | [0117](0117-a-seed-is-caught-against-a-passing-baseline.md) | `oracles seeds` credits a seed only when its oracle passes without it, in a baseline run with the same build and filter, and fails with it in a test of its own; a run that never reached the tests (a fixture xunit could not set up, an error counted outside any test, a crashed host, no TRX, every test skipped) or a failing baseline is `ERROR`, exit 4, never `CAUGHT` | 2026-10-29 | accepted | -| [0118](0118-an-error-may-learn-a-form-only-new-api-can-write.md) | An Error may report, in a minor version, a form that only an API new in that version can write, since no build that worked before contains it: CMTK0003's dropped assertion of `CodoMetis.TypeKit.Testing` on a `Task` or `ValueTask` in 2.3.0, its `ConfigureAwait`, `WaitAsync` and branch forms (#152) included, known by the package's assembly; the forms freeze at the tag; refines decision 34; one exception the maintainer took into 2.3.0, a dropped `Task.WhenAny` of tasks of results (#159) | 2026-10-29 | accepted | +| [0118](0118-an-error-may-learn-a-form-only-new-api-can-write.md) | An Error may report, in a minor version, a form that only an API new in that version can write, since no build that worked before contains it: CMTK0003's dropped assertion of `CodoMetis.TypeKit.Testing` on a `Task` or `ValueTask` in 2.3.0, its `ConfigureAwait`, `WaitAsync` and branch forms (#152) included, known by the package's assembly; the forms freeze at the tag; refines decision 34; one exception the maintainer took into 2.3.0, a dropped `Task.WhenAny` of tasks of results (#159) | 2026-10-29 | accepted; widened by [0119](0119-a-gap-found-before-the-tag-ships-in-that-release.md) | +| [0119](0119-a-gap-found-before-the-tag-ships-in-that-release.md) | A form an Error misses, found before its version is tagged, is reported in that version when it loses what the rule protects in every build, though code built on the previous minor can contain it: 2.3.0's CMTK0003 on a `Task.WhenAny` of results (#159), `Wait()` on a branch (#160), an array in `Task.WaitAll`/`WaitAny` (#161), a dropped `Task.WhenEach` (#162), and a `WhenAny` of held tasks silent; the rule stops at the tag, after which decision 34 holds; refines 34 and 118 | 2026-10-29 | accepted | diff --git a/design/analyzers.md b/design/analyzers.md index a81c471..4ff1a57 100644 --- a/design/analyzers.md +++ b/design/analyzers.md @@ -33,7 +33,7 @@ exist. |---|---|---|---| | CMTK0001 | No `default`/`default(T)`/`new()`/`new T()` of a value object or a `[RequireCustomInitialization]` type. An operand of `==`/`!=`, or an argument of a call whose name has the word `Equal`, `Equals` or `Compare` or ends in `Be` (`ThrowIfEqual`, `Assert.NotEqual`, `ShouldNotBe`), is exempt: a guard check is the only defence against the defaults the rule cannot see. So is a `default` assigned to an `out` parameter of a method returning `bool`, the Try pattern. Where a sibling branch does not bind (same-project value objects), the target type is taken from the enclosing conditional, switch arm or collection | Error | shipped in 1.0.0 | | CMTK0002 | Type implements `IValue<>`/`IValidatedValue<,,>`, but the compilation does not reference `CodoMetis.TypeKit.Generators`, so it is never woven | Error | shipped in 1.0.0 | -| CMTK0003 | `Result`/`Option` a call returns, dropped by an expression statement, awaited or not, or a `Task` of one converted to a plain `Task` (`Task Cancel() => CancelAsync();`); `_ =` opts out. `Tap`, `TapAsync`, `TapError` and `TapErrorAsync` on a stored result or task are exempt, since they return their receiver, and so is a chain of them on one (`r.Tap(log).TapError(warn)`, `(await pending).Tap(log)`, `pending.WaitAsync(t).TapAsync(log)`), measured as an Error's false report before 2.0.0 was tagged; so is such a tap converted to a plain `Task` or waited for (`Task Log() => pending.TapAsync(log);`, `held.TapAsync(log).Wait()`), on a `Result` and on a `Task` of one, which an independent review found reported on 2026-10-14. Also a collection of results the statement made (an array, `ICollection` or `IReadOnlyCollection`, such as `await Task.WhenAll(ids.Select(orders.CancelAsync))`), and a method group of a `Task` method converted to a delegate returning `Task`; tasks held in variables are silent, and so is a lazy `IEnumerable`, where nothing has run, unless a call that was given no result returned it and the statement materialises it. Also `Wait` on a task the expression made, tasks of results the statement made converted to plain ones (`Task.WaitAll`, `WaitAny`), and a `foreach` or `await foreach` over results its header made whose body never reads the iteration variable; a variable named `_` opts out. From 2.3.0, an assertion of `CodoMetis.TypeKit.Testing` on a `Task` or `ValueTask` of what it checks, dropped by an expression statement without `await`: `service.FindAsync(id).ShouldBeNone();`, the lowered static call, through `?.`, the body of a lambda that returns nothing, through `ConfigureAwait` or `WaitAsync` of the `Task` or `Task` it returns (every overload), and at each branch of a `?:`, `??` or `switch` expression so passed through, judged by the path every dropped call takes (#152, held by `DroppedTaskVariantTests` over `Variants/DroppedTaskShapes`); it fails the test only through its task. Known by the package's assembly, so a consumer's extension of the same name on a pending result is silent; awaited, held, returned or discarded with `_ =`, it is silent too. CA1806 can only enforce this per method name, not per return type | Error | shipped in 1.0.0 at Warning; Error in 2.0.0 ([decision 50](adr/0050-warnings-cmtk0003-alone-becomes-error-2.md)), its 1.x gaps closed first; the dropped pending assertion in 2.3.0, a form only 2.3.0's API writes ([decision 118](adr/0118-an-error-may-learn-a-form-only-new-api-can-write.md)) | +| CMTK0003 | `Result`/`Option` a call returns, dropped by an expression statement, awaited or not, or a `Task` of one converted to a plain `Task` (`Task Cancel() => CancelAsync();`); `_ =` opts out. `Tap`, `TapAsync`, `TapError` and `TapErrorAsync` on a stored result or task are exempt, since they return their receiver, and so is a chain of them on one (`r.Tap(log).TapError(warn)`, `(await pending).Tap(log)`, `pending.WaitAsync(t).TapAsync(log)`), measured as an Error's false report before 2.0.0 was tagged; so is such a tap converted to a plain `Task` or waited for (`Task Log() => pending.TapAsync(log);`, `held.TapAsync(log).Wait()`), on a `Result` and on a `Task` of one, which an independent review found reported on 2026-10-14. Also a collection of results the statement made (an array, `ICollection` or `IReadOnlyCollection`, such as `await Task.WhenAll(ids.Select(orders.CancelAsync))`), and a method group of a `Task` method converted to a delegate returning `Task`; tasks held in variables are silent, and so is a lazy `IEnumerable`, where nothing has run, unless a call that was given no result returned it and the statement materialises it. Also `Wait` on a task the expression made, tasks of results the statement made converted to plain ones (`Task.WaitAll`, `WaitAny`), and a `foreach` or `await foreach` over results its header made whose body never reads the iteration variable; a variable named `_` opts out. From 2.3.0, an assertion of `CodoMetis.TypeKit.Testing` on a `Task` or `ValueTask` of what it checks, dropped by an expression statement without `await`: `service.FindAsync(id).ShouldBeNone();`, the lowered static call, through `?.`, the body of a lambda that returns nothing, through `ConfigureAwait` or `WaitAsync` of the `Task` or `Task` it returns (every overload), and at each branch of a `?:`, `??` or `switch` expression so passed through, judged by the path every dropped call takes (#152); handed to `Task.WhenAll`, `Task.Run` or `Task.WhenEach` whose task is dropped, or to `Task.WhenAny` or `Task.WaitAny` in any use, as arguments, an array, a collection expression, a `Select`, a query, a `ToList`, a method group or an `async` lambda that awaits it with no `catch` between, and a `WhenAny` of them converted to a plain `Task` or waited for, and a loop that never reads the assertions its header made (#157); `Task.WaitAll` throws, so it is silent. It fails the test only through its task. Known by the package's assembly, so a consumer's extension of the same name on a pending result is silent; awaited, held, returned or discarded with `_ =`, it is silent too, and so are held tasks combined. Also from 2.3.0, on tasks of results, forms 2.2.0 code can contain: a `Task.WhenAny` of tasks the statement made, in any use (#159; of held tasks it is silent, where 2.2.0 reported it awaited), `Wait()` on a `?:`, `??` or `switch` (#160), an array written in `Task.WaitAll` or `Task.WaitAny` (#161) and a dropped `Task.WhenEach` (#162). All held by `DroppedTaskVariantTests` over `Variants/DroppedTaskShapes`, 830 cases. CA1806 can only enforce this per method name, not per return type | Error | shipped in 1.0.0 at Warning; Error in 2.0.0 ([decision 50](adr/0050-warnings-cmtk0003-alone-becomes-error-2.md)), its 1.x gaps closed first; the dropped pending assertion in 2.3.0, a form only 2.3.0's API writes ([decision 118](adr/0118-an-error-may-learn-a-form-only-new-api-can-write.md)); the 2.2-writable forms #159 to #162 in 2.3.0, gaps found before the tag ([decision 119](adr/0119-a-gap-found-before-the-tag-ships-in-that-release.md)) | | CMTK0004 | `IValueObjectMaterializer<,>.Materialize`, `ValueObjectConverter<,>.Materialize` or a hand-written value object's public `Materialize`, called or referenced (expression trees included). The satellite's own call is compiled in the satellite; EF's compiled model is generated code, whose diagnostics are not reported (measured: reported, it is six errors in the smoke test's AOT consumer). `StoredJsonConverterFactory` goes through `IStoredJsonConverterSource`, not `Materialize` | Error | shipped in 1.0.0 | | CMTK0005 | `new T[n]`, `stackalloc T[n]`, `GC.AllocateUninitializedArray/AllocateArray` or `Array.Resize` of a no-default struct: every new slot starts as `default`. A constant length of zero (for `Array.Resize` and the `GC` methods too) or initial elements are silent | Warning | shipped in 1.0.0 | | CMTK0006 | Field or auto-property of a no-default struct type in a class that no initializer, `required` or constructor sets; a non-public parameterless constructor (EF, serializers) and a record's copy constructor are exempt. Measured 2026-09-28 on the repository's EF entities and request types: 12 of 12 reports on settable properties of a type with only the implicit constructor, each a real way to a default. `required`, the fix the message names where it compiles, works with EF's compiled model, precompiled queries and Native AOT (the smoke test rerun with it). A `[SetsRequiredMembers]` constructor must assign the `required` members it promises | Warning | shipped in 1.0.0 | @@ -79,6 +79,6 @@ still runs on the source, where the name-based forms above remain the reach. ships at Warning or Info in a minor version and is raised to Error only in a major. Consumers build with warnings as errors, so a new Error rule in a minor version would break their builds on update. Within a major version an Error never reports more than it did, unless the form is one only -that minor version's new API can write ([decision 118](adr/0118-an-error-may-learn-a-form-only-new-api-can-write.md), -which also records 2.3.0's one exception, #159); +that minor version's new API can write ([decision 118](adr/0118-an-error-may-learn-a-form-only-new-api-can-write.md)), +or a gap found before the version's tag ([decision 119](adr/0119-a-gap-found-before-the-tag-ships-in-that-release.md)); a Warning or Info may learn more forms in a minor version ([decision 34](adr/0034-errors-fixed-within-major-version-warnings.md), stated in the analyzer README). diff --git a/src/CodoMetis.TypeKit.Analyzers/README.md b/src/CodoMetis.TypeKit.Analyzers/README.md index 9cbb67a..70b7ad7 100644 --- a/src/CodoMetis.TypeKit.Analyzers/README.md +++ b/src/CodoMetis.TypeKit.Analyzers/README.md @@ -778,9 +778,17 @@ write. In 2.3.0, CMTK0003 reports a `CodoMetis.TypeKit.Testing` assertion on a ` without `await`, and those assertions are new in 2.3.0. Your code from before the update can't contain one, so it builds as before. -2.3.0 makes one exception. CMTK0003 now also reports `Task.WhenAny` over tasks of results left without -`await`, which code from before 2.3.0 can contain. Such a line loses every result, so the build points at -a real bug. Handle the results, or discard the task with `_ =`. +2.3.0 makes an exception for a few forms that code from before 2.3.0 can contain. Each one loses every +result, so the build points at a real bug. CMTK0003 now also reports these, for tasks of results: + +- `Task.WhenAny` of tasks you don't hold, dropped, awaited, waited for or returned as a plain `Task`; +- a `Task.WhenEach` of them that you drop; +- `Wait()` on a `?:`, `??` or `switch` expression of them; +- an array of them that you write in a `Task.WaitAll` or `Task.WaitAny` call. + +Handle the results, or discard what you mean to drop with `_ =`. Only a gap found before a version's tag +is closed this way. After the tag, an Error that reports more of the code you could already write waits +for the next major version. A Warning or an Info may learn more forms in a minor version, such as another method that returns a default instance for CMTK0009. In a build that treats warnings as errors, such an update can report code diff --git a/test/oracles/claims.json b/test/oracles/claims.json index 5f063e3..df6f69b 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -4758,6 +4758,20 @@ "IgnoredOutcomeAnalyzerTests" ] }, + { + "id": "T02-a-few-2-2-forms-are-reported", + "source": "src/CodoMetis.TypeKit.Analyzers/README.md#configuration", + "quote": "2.3.0 makes an exception for a few forms that code from before 2.3.0 can contain.", + "claim": "2.3.0's CMTK0003 reports, on tasks of results, a Task.WhenAny of tasks the statement made in any use, a dropped Task.WhenEach, Wait() on a branch and an array written in Task.WaitAll or Task.WaitAny, all forms 2.2.0 code can contain; a WhenAny of held tasks stays silent.", + "oracle": "claim", + "surfaces": [ + "Result.*" + ], + "heldBy": [ + "DroppedTaskVariantTests", + "IgnoredOutcomeAnalyzerTests" + ] + }, { "id": "T02-a-wrapped-assertion-is-reported", "source": "src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0003",