diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f92163..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) ([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 b0a2bc6..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();`, `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 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/IgnoredOutcomeAnalyzer.cs b/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs index 5d4b154..d0369b0 100644 --- a/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs +++ b/src/CodoMetis.TypeKit.Analyzers/IgnoredOutcomeAnalyzer.cs @@ -141,29 +141,121 @@ 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; + + 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)); - reported |= own; + } + } + + /// + /// 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; + } } - // 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)); + 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 /// 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/0118). /// private static bool IsPendingAssertion(IMethodSymbol method, Tasks tasks) => TypeKitSymbols.IsTestingAssembly(method.ContainingAssembly) @@ -212,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); } @@ -268,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); @@ -275,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; @@ -291,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)); - context.Report(Diagnostic.Create(Rule, wait.Syntax.GetLocation(), outcome, invocation.TargetMethod.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, (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)); } /// @@ -385,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) { @@ -399,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; @@ -446,16 +603,40 @@ 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)); + + /// + /// 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; - /// 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)); + /// 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 58b6e7f..70b7ad7 100644 --- a/src/CodoMetis.TypeKit.Analyzers/README.md +++ b/src/CodoMetis.TypeKit.Analyzers/README.md @@ -189,8 +189,17 @@ 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. + +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: @@ -236,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. @@ -763,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/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs b/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs new file mode 100644 index 0000000..4f3544e --- /dev/null +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/DroppedTaskVariantTests.cs @@ -0,0 +1,70 @@ +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, 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 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(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 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( + 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, 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 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 use in DroppedTaskShapes.Uses) + source.ShouldContain($", {use.Name}\n", customMessage: $"{call}: no case is {use}."); + + 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 new file mode 100644 index 0000000..6a9eb36 --- /dev/null +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/Variants/DroppedTaskShapes.cs @@ -0,0 +1,370 @@ +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. +/// +/// 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; +} + +/// 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. +/// 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; +} + +/// 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. +/// +/// 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. +/// 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; +} + +/// +/// 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 ?., 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, #157 through the combinators, +/// and #159 a WhenAny of results dropped unawaited. +/// +/// +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, IsAssertion: false), + ]; + + public static IReadOnlyList PassThroughs { get; } = + [ + new("no pass-through", ""), + 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)", KeepsTask: 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), + + 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, 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 , 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, IReadOnlyList<(string Outcome, string Method)> Expected) Use(PendingCall call, DropForm form) + { + var expected = new List<(string Outcome, string Method)>(); + var body = new System.Text.StringBuilder(); + var awaited = new System.Text.StringBuilder(); + + 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; + + // await x?.M().ConfigureAwait(false) awaits a nullable awaiter, which has no GetAwaiter, and await x?.M() + // 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); + + 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); + + // 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.Linq; + 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 ?., 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.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, (string Outcome, string Method) message, List<(string Outcome, string Method)> expected) + { + 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 ? $"{{|#{expected.Count}:" : "").Append(text, start + open.Length, end - start - open.Length).Append(reported ? "|}" : ""); + if (reported) expected.Add(message); + 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; + + 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 344b875..df6f69b 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -4758,6 +4758,82 @@ "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", + "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": "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",