From e37e374d5aaa8a2f27967f11a97ff5098d8f9ff9 Mon Sep 17 00:00:00 2001 From: George Wall <84231788+georgepwall1991@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:52:10 +0100 Subject: [PATCH] fix: detect implicit await forms so CC019 catches cancellation loss Async enumeration and async disposal store their await keywords on statement syntax rather than AwaitExpressionSyntax. Recognizing those forms prevents broad catches from silently swallowing cancellation during iteration or disposal. --- CHANGELOG.md | 8 ++ docs/ANALYZER_HEALTH.md | 7 +- .../CancelCop.Analyzer.Package.csproj | 2 +- .../SwallowedCancellationAnalyzer.cs | 23 ++++- .../SwallowedCancellationAnalyzerTests.cs | 86 +++++++++++++++++++ 5 files changed, 119 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86fa422..3f594f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.27.175] - 2026-07-21 + +### Fixed + +- **CC019** false negative: broad catches around `await foreach` or either `await using` form are + now analyzed as cancellation-capable awaited work. These constructs carry await keywords on their + statement syntax rather than producing `AwaitExpressionSyntax`, which previously left them silent. + ## [1.27.174] - 2026-07-21 ### Fixed diff --git a/docs/ANALYZER_HEALTH.md b/docs/ANALYZER_HEALTH.md index 3db020f..ee0cba9 100644 --- a/docs/ANALYZER_HEALTH.md +++ b/docs/ANALYZER_HEALTH.md @@ -1,6 +1,6 @@ # Analyzer Health -Reviewed: 2026-07-21 (refreshed through the v1.27.174 hardening loop) +Reviewed: 2026-07-21 (refreshed through the v1.27.175 hardening loop) A deliberately harsh health audit for the twenty-eight implemented CancelCop rule IDs (CC001–CC006, CC009–CC028). Scores are 1–5, where `5` means reference-quality and hard to improve, `3` means usable but @@ -49,7 +49,7 @@ Calibration notes: | CC022 | Prefer `CancelAsync()` over `Cancel()` in async | Usage | Info | 4 | 4 | 4 | 4 | 3 | 3 | Low | **v1.18.0 (new):** flags a parameterless `CancellationTokenSource.Cancel()` inside async code; fixer rewrites to `await cts.CancelAsync()`. Info (`Cancel()` is still valid). The `Cancel(bool)` overload and sync contexts are excluded. Modern .NET 8 guidance — `Cancel()` runs callbacks synchronously on the caller. | | CC021 | `HttpContext.RequestAborted` not observed | Usage | Info | 4 | 3 | n/a | 4 | 3 | 3 | Low | **v1.16.0 (new):** the HttpContext parallel of CC020. Flags a method with a `Microsoft.AspNetCore.Http.HttpContext` parameter that does async work but never reads `context.RequestAborted` and never passes the context on. Info because HttpContext is often taken for non-cancellation reasons (hence FP score 3). Shares `AccessesMember`/`ParameterEscapesAsArgument` with CC020. | | CC020 | gRPC method ignores `ServerCallContext.CancellationToken` | Usage | Warning | 4 | 4 | n/a | 4 | 3 | 3 | Low | **v1.15.0 (new):** flags a method with a `Grpc.Core.ServerCallContext` parameter that does async work but never reads `context.CancellationToken` and never passes the context on. Fills a genuine gap — the token is a property, not a parameter, so CC002 can't see it (cf. CC017 for BackgroundService). Analyzer-only; gated by parameter type name+namespace (tests use a stub). | -| CC019 | Broad catch swallows `OperationCanceledException` | Usage | Info | 4 | 3 | 4 | 4 | 3 | 3 | Low | **v1.14.0 (new); fixes v1.17.0/v1.27.174:** flags a catch-all/`catch (Exception)` with no `when` filter, over a `try` containing an `await` in the current function scope, whose body never rethrows. Awaits inside nested local functions/lambdas are ignored because that deferred work does not execute in the `try` itself. Info because boundary handlers are sometimes intended. Conservative (filter/rethrow/specific-type/no-await all suppress). The fix inserts `if (ex is OperationCanceledException) throw;` (typed catches only). | +| CC019 | Broad catch swallows `OperationCanceledException` | Usage | Info | 4 | 3 | 4 | 4 | 3 | 3 | Low | **v1.14.0 (new); fixes v1.17.0/v1.27.174/v1.27.175:** flags a catch-all/`catch (Exception)` with no `when` filter, over a `try` containing awaited work in the current function scope, whose body never rethrows. Covers explicit `await`, `await foreach`, and both `await using` forms; awaits inside nested local functions/lambdas are ignored because that deferred work does not execute in the `try` itself. Info because boundary handlers are sometimes intended. Conservative (filter/rethrow/specific-type/no-await all suppress). The fix inserts `if (ex is OperationCanceledException) throw;` (typed catches only). | | CC018 | SignalR hub method missing `CancellationToken` | Usage | Warning | 4 | 4 | 4 | 4 | 3 | 4 | Low | **v1.13.0 (new):** SignalR analogue of CC005B. Flags a public non-static async method on a `Microsoft.AspNetCore.SignalR.Hub`/`Hub` subclass without a token; excludes lifecycle overrides + externally-controlled signatures. Reuses the shared add-token-parameter fixer. Base-type gated by name+namespace (tests use a faithful Hub stub, no package). | | CC017 | `BackgroundService.ExecuteAsync` ignores stopping token | Usage | Warning | 4 | 4 | n/a | 4 | 3 | 4 | Low | **v1.12.0 (new):** flags an `override` of `ExecuteAsync(CancellationToken)` on a `Microsoft.Extensions.Hosting.BackgroundService` subclass whose body never references the stopping token — the override case CC016 skips. Analyzer-only; token passed to a helper or observed in a loop counts as used. Framework-gated to BackgroundService by base-type walk. | | CC016 | Unused `CancellationToken` parameter | Usage | Info | 4 | 4 | n/a | 4 | 3 | 3 | Low | **v1.11.0 (new):** flags a method/local function that does async work (has `await`) but never references its `CancellationToken` parameter; excludes externally-controlled signatures and sync bodies. Analyzer-only by design. A token used in a nested lambda/local function counts as used. **v1.27.11 (FP fix):** a token marked `[EnumeratorCancellation]` is excluded — the async-iterator infrastructure delivers a consumer's `WithCancellation` token to it, so it is observed even without a body reference (cf. CC011). | @@ -168,6 +168,9 @@ Grading: **P0** = release-blocking; **P1** = next hardening loop; **P2** = oppor ## Verification Baseline +- v1.27.175: 629 tests, green locally. **CC019 FN fix:** `await foreach`, `await using var`, and + `await using (...)` now count as awaited work in the current `try` scope, closing the syntax gap + where their await keywords were invisible to the prior `AwaitExpressionSyntax`-only check. - v1.27.174: 626 tests, green locally. **CC019 FP fix:** an `await` owned by a local function or lambda declared inside a `try` no longer makes the enclosing broad catch report; only awaited work executed in the current function scope can establish the cancellation-swallowing risk. diff --git a/src/CancelCop.Analyzer.Package/CancelCop.Analyzer.Package.csproj b/src/CancelCop.Analyzer.Package/CancelCop.Analyzer.Package.csproj index 20fb24c..a214640 100644 --- a/src/CancelCop.Analyzer.Package/CancelCop.Analyzer.Package.csproj +++ b/src/CancelCop.Analyzer.Package/CancelCop.Analyzer.Package.csproj @@ -13,7 +13,7 @@ CancelCop.Analyzer - 1.27.174 + 1.27.175 George Wall A surgical Roslyn analyzer focused on CancellationToken best practices: propagation, parameter positioning, loop cancellation checks, and more. Includes automatic code fixes for public APIs, handlers, EF Core, HTTP calls, and Minimal APIs. roslyn;analyzer;cancellationtoken;async;code-fix;loops;efcore;httpclient diff --git a/src/CancelCop.Analyzer/SwallowedCancellationAnalyzer.cs b/src/CancelCop.Analyzer/SwallowedCancellationAnalyzer.cs index f8d6ca6..f1cb0ff 100644 --- a/src/CancelCop.Analyzer/SwallowedCancellationAnalyzer.cs +++ b/src/CancelCop.Analyzer/SwallowedCancellationAnalyzer.cs @@ -26,8 +26,9 @@ namespace CancelCop.Analyzer; /// /// What it detects: a catch with no exception type, or one catching /// System.Exception, that has no when filter, whose try block contains an -/// await in the current function scope, and whose body never rethrows. Awaits owned by a -/// nested local or anonymous function do not execute as part of the try itself. +/// await in the current function scope (including await foreach and +/// await using), and whose body never rethrows. Awaits owned by a nested local or anonymous +/// function do not execute as part of the try itself. /// /// /// @@ -103,8 +104,22 @@ private static bool ContainsAwaitInCurrentScope(SyntaxNode block) return block.DescendantNodes(descendIntoChildren: child => child is not LocalFunctionStatementSyntax && child is not AnonymousFunctionExpressionSyntax) - .OfType() - .Any(); + .Any(IsAwaitedOperation); + } + + private static bool IsAwaitedOperation(SyntaxNode node) + { + return node switch + { + AwaitExpressionSyntax => true, + CommonForEachStatementSyntax forEach => + forEach.AwaitKeyword.IsKind(SyntaxKind.AwaitKeyword), + LocalDeclarationStatementSyntax declaration => + declaration.AwaitKeyword.IsKind(SyntaxKind.AwaitKeyword), + UsingStatementSyntax usingStatement => + usingStatement.AwaitKeyword.IsKind(SyntaxKind.AwaitKeyword), + _ => false, + }; } private static bool CatchesEverything( diff --git a/tests/CancelCop.Analyzer.Tests/SwallowedCancellationAnalyzerTests.cs b/tests/CancelCop.Analyzer.Tests/SwallowedCancellationAnalyzerTests.cs index 95f819c..d40c360 100644 --- a/tests/CancelCop.Analyzer.Tests/SwallowedCancellationAnalyzerTests.cs +++ b/tests/CancelCop.Analyzer.Tests/SwallowedCancellationAnalyzerTests.cs @@ -49,6 +49,92 @@ public async Task RunAsync() await VerifyCS.VerifyAnalyzerAsync(test, expected); } + [Fact] + public async Task CatchException_OverAwaitForeach_ShouldReportDiagnostic() + { + var test = new CSharpAnalyzerTest + { + ReferenceAssemblies = ReferenceAssemblies.Net.Net90, + TestCode = @" +using System; +using System.Collections.Generic; +using System.Threading.Tasks; + +public class TestClass +{ + public async Task RunAsync(IAsyncEnumerable source) + { + try + { + await foreach (var item in source) + { + } + } + {|#0:catch|} (Exception) { } + } +}", + }; + test.ExpectedDiagnostics.Add(VerifyCS.Diagnostic("CC019").WithLocation(0)); + + await test.RunAsync(); + } + + [Fact] + public async Task CatchException_OverAwaitUsingDeclaration_ShouldReportDiagnostic() + { + var test = new CSharpAnalyzerTest + { + ReferenceAssemblies = ReferenceAssemblies.Net.Net90, + TestCode = @" +using System; +using System.Threading.Tasks; + +public class TestClass +{ + public async Task RunAsync(IAsyncDisposable resource) + { + try + { + await using var owned = resource; + } + {|#0:catch|} (Exception) { } + } +}", + }; + test.ExpectedDiagnostics.Add(VerifyCS.Diagnostic("CC019").WithLocation(0)); + + await test.RunAsync(); + } + + [Fact] + public async Task CatchException_OverAwaitUsingStatement_ShouldReportDiagnostic() + { + var test = new CSharpAnalyzerTest + { + ReferenceAssemblies = ReferenceAssemblies.Net.Net90, + TestCode = @" +using System; +using System.Threading.Tasks; + +public class TestClass +{ + public async Task RunAsync(IAsyncDisposable resource) + { + try + { + await using (resource) + { + } + } + {|#0:catch|} (Exception) { } + } +}", + }; + test.ExpectedDiagnostics.Add(VerifyCS.Diagnostic("CC019").WithLocation(0)); + + await test.RunAsync(); + } + [Fact] public async Task CatchException_Rethrows_ShouldNotReportDiagnostic() {