Fix CA1000/MA0003/RCS1140/RCS1163 in Result and Try#94
Merged
Conversation
a83419e to
12742fd
Compare
Chris-Wolfgang
added a commit
that referenced
this pull request
May 1, 2026
Resolves the failures blocking PR #94's Stage 1 (Linux) CI: - Sync .editorconfig from canonical (picks up repo-template#329's RS0030 = none in [tests/**/*.cs] and [benchmarks/**/*.cs]). - ResultTests.cs (4 sites): add 'succeeded:' named argument to bool literal in TestResult constructor (MA0003). - CSharp.DotNet8.Example/Program.cs: add 'content:' named argument to GetWordCount(null) (MA0003). - TryBenchmarks.cs: replace 'async () => await Task.CompletedTask' with sync no-op '() => { }' for the Action overload of RunAsync. The async lambda was being coerced to async-void (Action signature is 'void Action()'), which AsyncFixer03 + VSTHRD101 correctly flagged. The Action benchmarks should pass sync delegates. - VB.DotNet8.Example/Program.vb: wrap MainAsync().GetAwaiter(). GetResult() in #Disable Warning RS0030 / #Enable Warning since this is the standard VB sync entry-point pattern (no async Main in VB) and canonical's [examples/**/*.cs] glob doesn't match .vb. Release build clean: 0 warnings, 0 errors across all TFMs (src, tests, benchmarks, C# and VB and F# examples). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses analyzer warnings/errors (CA1000/MA0003/RCS1140/RCS1163/RS0030) in the core Result/Try APIs and related examples/benchmarks so Release builds stay clean under stricter analyzer configurations.
Changes:
- Suppresses CA1000 for
Result<T>factory members and adds named arguments for boolean literals inResultconstruction paths. - Updates
Try.Run/RunAsyncXML docs forArgumentNullExceptionand explicitly discards unusedCancellationTokenparameters inRunAsync<T>overloads. - Adjusts examples/benchmarks and relaxes RS0030 severity for tests/benchmarks in
.editorconfig.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Wolfgang.TryPattern.Tests/ResultTests.cs | Uses named argument for succeeded to satisfy analyzer rule. |
| src/Wolfgang.TryPattern/Try.cs | Adds <exception> tags and explicitly discards unused token parameters in async generic overloads. |
| src/Wolfgang.TryPattern/Result.cs | Adds CA1000 suppression for Result<T> and uses named args for boolean literals in factory paths. |
| examples/VB.DotNet8.Example/Program.vb | Disables RS0030 around VB sync entry-point pattern (GetResult). |
| examples/CSharp.DotNet8.Example/Program.cs | Uses named argument for clarity when passing null. |
| benchmarks/Wolfgang.TryPattern.Benchmarks/TryBenchmarks.cs | Avoids async void-style lambdas by using synchronous Action delegates. |
| .editorconfig | Changes RS0030 severity to none in tests/benchmarks sections. |
3 tasks
Owner
Author
Chris-Wolfgang
added a commit
that referenced
this pull request
May 1, 2026
Resolves the code-side failures blocking PR #94's CI: - ResultTests.cs (4 sites): add 'succeeded:' named argument to bool literal in TestResult constructor (MA0003). - CSharp.DotNet8.Example/Program.cs: add 'content:' named argument to GetWordCount(null) (MA0003). - TryBenchmarks.cs: replace 'async () => await Task.CompletedTask' with sync no-op '() => { }' for the Action overload of RunAsync. The async lambda was being coerced to async-void (Action signature is 'void Action()'), which AsyncFixer03 + VSTHRD101 correctly flagged. The Action benchmarks should pass sync delegates. - VB.DotNet8.Example/Program.vb: wrap MainAsync().GetAwaiter(). GetResult() in #Disable Warning RS0030 / #Enable Warning since this is the standard VB sync entry-point pattern (no async Main in VB) and canonical's [examples/**/*.cs] glob doesn't match .vb. Note: the related .editorconfig sync (RS0030 = none in tests and benchmarks, picking up repo-template#329) is split into a separate PR because .editorconfig is a protected file. This PR depends on that one landing first to silence RS0030 violations on DateTime.Now in ResultOfTTests.cs and Task.Wait() in RunAsyncActionTests.cs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c571b7a to
51a9252
Compare
Chris-Wolfgang
added a commit
that referenced
this pull request
May 1, 2026
Replaces '_ = token;' (Roslynator RCS1163 fix that merely discards) with 'token.ThrowIfCancellationRequested();' so the parameter actually participates in cancellation. The delegate signature Func<Task<T?>> has no token parameter so the token can't be flowed into the callback; the best we can do is fail fast if cancellation was already requested before invocation. The existing OperationCanceledException catch already rethrows this correctly. Also clarifies the <param> doc to be honest about what "monitor" means here (checked before invocation, not flowed into the function) and adds a corresponding <exception cref="OperationCanceledException"> tag. Addresses Copilot review comments on PR #94 at Try.cs:144 and Try.cs:170. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Chris-Wolfgang
added a commit
that referenced
this pull request
May 1, 2026
…nonical Picks up the canonical change from repo-template#329: - [tests/**/*.cs]: RS0030.severity changed from warning to none. - [benchmarks/**/*.cs]: same change. The prior 'severity = warning' was meant to "warn but allow" the banned API analyzer in test/benchmark code, but Release builds enable TreatWarningsAsErrors which promoted it back to error — defeating the relaxation entirely. Setting to 'none' matches the existing pattern for every other relaxed rule in those folders. Surfaced when [#94]'s code-fix branch began enforcing the rule on test code that uses DateTime.Now and Task.Wait() — both legitimate test patterns. This change is split out of #94 because .editorconfig is a protected file and benefits from a focused, isolated review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Chris-Wolfgang
added a commit
that referenced
this pull request
May 1, 2026
Resolves the code-side failures blocking PR #94's CI: - ResultTests.cs (4 sites): add 'succeeded:' named argument to bool literal in TestResult constructor (MA0003). - CSharp.DotNet8.Example/Program.cs: add 'content:' named argument to GetWordCount(null) (MA0003). - TryBenchmarks.cs: replace 'async () => await Task.CompletedTask' with sync no-op '() => { }' for the Action overload of RunAsync. The async lambda was being coerced to async-void (Action signature is 'void Action()'), which AsyncFixer03 + VSTHRD101 correctly flagged. The Action benchmarks should pass sync delegates. - VB.DotNet8.Example/Program.vb: wrap MainAsync().GetAwaiter(). GetResult() in #Disable Warning RS0030 / #Enable Warning since this is the standard VB sync entry-point pattern (no async Main in VB) and canonical's [examples/**/*.cs] glob doesn't match .vb. Note: the related .editorconfig sync (RS0030 = none in tests and benchmarks, picking up repo-template#329) is split into a separate PR because .editorconfig is a protected file. This PR depends on that one landing first to silence RS0030 violations on DateTime.Now in ResultOfTTests.cs and Task.Wait() in RunAsyncActionTests.cs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Chris-Wolfgang
added a commit
that referenced
this pull request
May 1, 2026
Replaces '_ = token;' (Roslynator RCS1163 fix that merely discards) with 'token.ThrowIfCancellationRequested();' so the parameter actually participates in cancellation. The delegate signature Func<Task<T?>> has no token parameter so the token can't be flowed into the callback; the best we can do is fail fast if cancellation was already requested before invocation. The existing OperationCanceledException catch already rethrows this correctly. Also clarifies the <param> doc to be honest about what "monitor" means here (checked before invocation, not flowed into the function) and adds a corresponding <exception cref="OperationCanceledException"> tag. Addresses Copilot review comments on PR #94 at Try.cs:144 and Try.cs:170. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ad8642c to
50e6340
Compare
Result.cs: - CA1000 (Do not declare static members on generic types): suppresses on Result<T> with a justification — Failure / Success factories are central to the public API and consumers explicitly specify T at the call site by design. - MA0003 (Name the parameter): adds 'succeeded:' to 6 boolean literal arguments at constructor / factory call sites. Try.cs: - RCS1140 (Add exception to documentation comment): adds <exception cref="ArgumentNullException"> to the doc comments of Run, Run<T>, RunAsync, and RunAsync<T> so the existing throws are documented (4 doc blocks, covering 6 throw sites across NET5+ and pre-NET5 #if branches). - RCS1163 (Unused parameter 'token'): explicitly discards the token parameter in RunAsync<T> with '_ = token;' since it can't be flowed into Func<Task<T?>> / Func<Task<T>> (no token parameter on the delegate). Discard documents the intent. Currently surfaces as warnings under main but becomes Release errors once the centralized analyzer config in chore/template-drift-fixes lands. Landing this fix first keeps that PR's CI green. Release build clean: 0 warnings, 0 errors across all TFMs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves the code-side failures blocking PR #94's CI: - ResultTests.cs (4 sites): add 'succeeded:' named argument to bool literal in TestResult constructor (MA0003). - CSharp.DotNet8.Example/Program.cs: add 'content:' named argument to GetWordCount(null) (MA0003). - TryBenchmarks.cs: replace 'async () => await Task.CompletedTask' with sync no-op '() => { }' for the Action overload of RunAsync. The async lambda was being coerced to async-void (Action signature is 'void Action()'), which AsyncFixer03 + VSTHRD101 correctly flagged. The Action benchmarks should pass sync delegates. - VB.DotNet8.Example/Program.vb: wrap MainAsync().GetAwaiter(). GetResult() in #Disable Warning RS0030 / #Enable Warning since this is the standard VB sync entry-point pattern (no async Main in VB) and canonical's [examples/**/*.cs] glob doesn't match .vb. Note: the related .editorconfig sync (RS0030 = none in tests and benchmarks, picking up repo-template#329) is split into a separate PR because .editorconfig is a protected file. This PR depends on that one landing first to silence RS0030 violations on DateTime.Now in ResultOfTTests.cs and Task.Wait() in RunAsyncActionTests.cs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces '_ = token;' (Roslynator RCS1163 fix that merely discards) with 'token.ThrowIfCancellationRequested();' so the parameter actually participates in cancellation. The delegate signature Func<Task<T?>> has no token parameter so the token can't be flowed into the callback; the best we can do is fail fast if cancellation was already requested before invocation. The existing OperationCanceledException catch already rethrows this correctly. Also clarifies the <param> doc to be honest about what "monitor" means here (checked before invocation, not flowed into the function) and adds a corresponding <exception cref="OperationCanceledException"> tag. Addresses Copilot review comments on PR #94 at Try.cs:144 and Try.cs:170. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Fix 'cancelled' → 'canceled' on the OperationCanceledException XML doc for consistency with the .NET type name - Update misleading inline comment near token.ThrowIfCancellationRequested: it runs before the try/catch, not 'rethrown by the catch below'. The comment now correctly states the OCE propagates directly to the caller. - Add 2 tests covering the 'token already canceled' path: - RunAsync_Func_when_token_is_already_canceled_* - RunAsync_Func_nullable_when_token_is_already_canceled_* Both assert OperationCanceledException is thrown AND that the function delegate is not invoked, locking in the fail-fast semantics. Verified: dotnet build succeeds with 0 warnings, 97/97 tests pass on net10.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0cc8854 to
ffd10e7
Compare
The Windows CI runner enforces a stricter analyzer set than local builds
(.editorconfig [tests/**/*.cs] relaxations don't fully apply in CI). To
make the test-project policy explicit and consistent, add a <NoWarn>
block to the test csproj covering:
Style / test-pattern false positives:
- VSTHRD200, VSTHRD003, VSTHRD103 (threading/async naming style)
- AsyncFixer01 (single-await async methods are normal in tests)
- MA0004, S6966 (ConfigureAwait & sync/async — tests need flexibility)
- S2190 (false positive on loops that exit via thrown exception)
- S2930 (CTS dispose — tests are short-lived; cts goes out of scope)
Test-fixture exception illustration:
- S3928 / MA0015 (ArgumentException without paramName — fixture style)
- MA0012 (tests intentionally throw NullReferenceException to verify
the Try wrapper catches/swallows it)
Real test bugs fixed in code:
- ResultTests.cs (S2699 / S1481): the two 'does_not_throw' tests now use
Record.Exception(...) + Assert.Null instead of an unused local variable
- RunFuncTests.cs (S4144): Run_Func_WithNullableStringFunction was
byte-identical to Run_Func_WithStringFunction. Now actually exercises
Func<string?> returning null and asserts result.Value is null.
Verified: 0 warnings/errors local clean build, all 97 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per review: the test name refers to the function *signature* being nullable (Func<string?>), not the return *value* being null. The function should still return a real string and the test should assert on it. This keeps the original "Hello, World!" coverage while fixing the S4144 duplicate-method complaint by using Func<string?> instead of Func<string>. The other test (Run_Func_WithStringFunction_ReturnsResult) tests the non-nullable Func<string> signature; this one tests Func<string?>. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per review: revert the Record.Exception rewrites in ResultTests.cs. The original 'var unused = new TestResult(...)' tests are valid xUnit 'does not throw' patterns — xUnit reports unhandled exceptions during test execution as failures, so an explicit assertion is not required. Add S2699 (no-assertion) and S1481 (unused-local) to the test csproj NoWarn block to satisfy the analyzers without changing the test code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address Copilot review: - RunAsync(Action, CancellationToken): the token flows into Task.Run and cancellation is explicitly rethrown via 'catch (OperationCanceled) throw;'. The XML docs only mentioned ArgumentNullException, which could surprise consumers. Add an <exception> entry for OCE and expand the <param> doc to describe both fail-paths (before-start and during-execution). - RunAsync<T>(Func<Task<T>>, CancellationToken): the existing token doc only described the 'already canceled' fail-fast. But the catch also rethrows OCE if 'function' observes cancellation during execution (e.g. await Task.Delay(_, token) inside the lambda). Expand both <param> and <exception> docs to cover both paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI surfaced remaining analyzer errors in benchmarks and examples that my earlier fix only addressed in the test csproj. Same root cause: the Windows CI runner doesn't fully honor .editorconfig section relaxations for [benchmarks/**/*.cs] and [examples/**/*.cs], so we move the suppressions to the project level where MSBuild applies them reliably. Benchmarks NoWarn: MA0004, VSTHRD200, S6966 Examples NoWarn: MA0004, S6966 Verified: 0 warnings/errors local clean build. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4840cf6 to
fc46885
Compare
…ise-benchmarks-examples Suppress noisy analyzer rules in benchmarks and examples csproj
Per Copilot review: Task.Run(action, token) only honors cancellation *before* the action is scheduled — once the synchronous action starts running, the token can't interrupt it. Cancellation during execution only happens if the action itself cooperatively checks the token. Updated <param name="token"> and the <exception> entry to spell this out so callers don't expect Task.Run to forcibly cancel running work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Chris-Wolfgang
added a commit
that referenced
this pull request
May 2, 2026
Resolves the code-side failures blocking PR #94's CI: - ResultTests.cs (4 sites): add 'succeeded:' named argument to bool literal in TestResult constructor (MA0003). - CSharp.DotNet8.Example/Program.cs: add 'content:' named argument to GetWordCount(null) (MA0003). - TryBenchmarks.cs: replace 'async () => await Task.CompletedTask' with sync no-op '() => { }' for the Action overload of RunAsync. The async lambda was being coerced to async-void (Action signature is 'void Action()'), which AsyncFixer03 + VSTHRD101 correctly flagged. The Action benchmarks should pass sync delegates. - VB.DotNet8.Example/Program.vb: wrap MainAsync().GetAwaiter(). GetResult() in #Disable Warning RS0030 / #Enable Warning since this is the standard VB sync entry-point pattern (no async Main in VB) and canonical's [examples/**/*.cs] glob doesn't match .vb. Note: the related .editorconfig sync (RS0030 = none in tests and benchmarks, picking up repo-template#329) is split into a separate PR because .editorconfig is a protected file. This PR depends on that one landing first to silence RS0030 violations on DateTime.Now in ResultOfTTests.cs and Task.Wait() in RunAsyncActionTests.cs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Chris-Wolfgang
added a commit
that referenced
this pull request
May 2, 2026
Replaces '_ = token;' (Roslynator RCS1163 fix that merely discards) with 'token.ThrowIfCancellationRequested();' so the parameter actually participates in cancellation. The delegate signature Func<Task<T?>> has no token parameter so the token can't be flowed into the callback; the best we can do is fail fast if cancellation was already requested before invocation. The existing OperationCanceledException catch already rethrows this correctly. Also clarifies the <param> doc to be honest about what "monitor" means here (checked before invocation, not flowed into the function) and adds a corresponding <exception cref="OperationCanceledException"> tag. Addresses Copilot review comments on PR #94 at Try.cs:144 and Try.cs:170. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
Chris-Wolfgang
added a commit
that referenced
this pull request
May 3, 2026
Picks up changes since v0.2.0: - Result.cs / Try.cs: CA1000/MA0003/RCS1140/RCS1163 fixes (#94) - Try.cs: RunAsync<T> token observation via ThrowIfCancellationRequested - Drift sync (.editorconfig, Directory.Build.props, workflow files) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Result<T>(factory methods are intentional public API), addssucceeded:named argument to 6 boolean literals (MA0003).<exception cref="ArgumentNullException">toRun/RunAsyncdoc comments (RCS1140 — covering 6 throw sites).RunAsync<T>now observes the token viatoken.ThrowIfCancellationRequested()before invoking the delegate (replaces the earlier_ = token;discard). Fail-fast semantics: if the token is already canceled, no delegate runs andOperationCanceledExceptionpropagates to the caller.OperationCanceledExceptionon both async overloads with accurate Task.Run cancellation semantics.RunAsync_Func_when_token_is_already_canceled_*) lock in the new fail-fast behavior on bothFunc<Task<int>>andFunc<Task<int?>>overloads.RunFuncTests.cs—Run_Func_WithNullableStringFunctionwas byte-identical to the non-nullable variant; now exercisesFunc<string?>(S4144 fix).<NoWarn>: project-level suppressions for stylistic / test-pattern false positives that the Windows CI runner reports as errors despite being relaxed in.editorconfig's[tests/**/*.cs]section. Rules suppressed:VSTHRD200,VSTHRD003,VSTHRD103,AsyncFixer01,MA0004,S6966,S2190,S2930,S3928,MA0015,MA0012,S2699,S1481. Each entry is annotated.Why now
Under main these surface as warnings; under #93's synced canonical .editorconfig + bumped Meziantou/Sonar they become Release errors. Splitting this code fix from #93 keeps the drift PR pure template-sync; merge this first, then rebase #93 onto main.
The benchmarks and examples csproj
<NoWarn>cleanup has been split out into #97.Test plan
🤖 Generated with Claude Code