chore(inspectcode): pre-InspectCode hygiene — analyzer noise tuning + file-scoped usings - #246
Merged
Merged
Conversation
Ran `jb inspectcode TryPattern.sln --severity=WARNING --no-build` locally against the pr.yaml job's exact command line before merging #245. Initial run: 177 findings (0 errors, 177 warnings). Categorization: - ~154 were noise: R#'s copy of PublicApiAnalyzer (RS0016/RS0037) fires in test/benchmark projects where MSBuild's PublicApiAnalyzer is gated by `<AdditionalFiles Condition="Exists('PublicAPI.Shipped.txt')" />` — but R# doesn't honor that MSBuild gating. Similarly Sonar/Meziantou/ VSTHRD rules already NoWarn'd in tests/csproj don't propagate to R#'s own rule engine. - ~10 were false positives for TFM-conditional patterns (RedundantUsingDirective on `using System;` etc. — needed on net462 / netstandard2.0 / net5-7 where ImplicitUsings is off; needed on the polyfill file where CheckNamespace complains because the namespace is intentionally `System.Diagnostics.CodeAnalysis`). - 2 were real: 1. Result.cs L283 — `[System.Diagnostics.CodeAnalysis.SuppressMessage(...)]` fully-qualified when `using System.Diagnostics.CodeAnalysis;` at L2 already imports the type. Shortened to `[SuppressMessage(...)]`. 2. examples/VB.DotNet462.Example/Program.vb L13 — `File.ReadAllText` is on the BannedSymbols list, but net462 has no async equivalent so the example cannot use one. Added inline VB `<SuppressMessage("ApiDesign", "RS0030:...", Justification:="...")>` on the module so the finding is a deliberate opt-out, not an analyzer miss. TryPattern.sln.DotSettings (new): tunes InspectCode's noise floor per the #208 issue-body requirement. Silences the ~154 categorized-noise rules with a top-level comment explaining each group. RS0030 is intentionally NOT globally silenced so any future banned-API use still lights up (the VB example uses inline suppression instead). Result after tuning: 0 findings. The CI job's `jq` gate on `level=="error"` would trivially pass, and the "success as long as main is clean, so the first PR finding is always actionable" policy now has an actual clean-main baseline.
…vbproj Replaces the inline VB <SuppressMessage> attribute on Program.vb with a localized <NoWarn>...;RS0030</NoWarn> entry in VB.DotNet462.Example.vbproj (both Debug and Release PropertyGroups). InspectCode honors MSBuild <NoWarn> for Roslyn-analyzer-emitted rules (RS0030 is BannedApiAnalyzers), so this suppresses the finding locally without cluttering the source. Verified: `jb inspectcode` reports 0 findings after the change. The library src still has RS0030 fully active — a future banned-API use in src/ or any non-net462 example project lights up as before. Only this one example is scoped out, since net462 has no async File API.
Removes <ImplicitUsings>enable</ImplicitUsings> from the src, tests, and
benchmarks csproj files, and adds file-scoped `using` directives
explicitly to every source file that needed them. Rationale: any
`using System;` etc. now compiles as needed on every TFM (net462 through
net10.0) rather than being "redundant" on the modern TFMs where the
SDK would have provided them implicitly. Removes RedundantUsingDirective
noise from InspectCode without needing to silence the rule in
.DotSettings.
Changes:
- src/csproj: removed the conditional ImplicitUsings PropertyGroup
(only net8.0/net10.0 branch); replaced with a comment explaining the
policy.
- tests/csproj: removed the unconditional <ImplicitUsings>enable</…>
line; kept <Using Include="Xunit" /> since removing that would require
`using Xunit;` in every test file (out of scope; can revisit if
desired).
- benchmarks/csproj: removed <ImplicitUsings>enable</…>.
- 4 test files (RunActionTests, RunAsyncActionTests, RunAsyncFuncTests,
RunFuncTests): removed the `#if !NET6_0_OR_GREATER using System; …
#endif` guards; the usings are now unconditional. Added
`using System.Threading;` where CancellationTokenSource is referenced
(was silently provided by ImplicitUsings on net6+; now explicit).
- 2 test files (ResultTests, ResultOfTTests): added `using System;`
(had no using at all; relied on ImplicitUsings for ArgumentException /
InvalidOperationException etc.).
- benchmarks/TryBenchmarks.cs: added `using System;` +
`using System.Threading.Tasks;` (had only `using System.Diagnostics.
CodeAnalysis;`; needed Task on net10 build without ImplicitUsings).
- src/NotNullAttribute.cs: removed `using System;` inside the
`namespace System.Diagnostics.CodeAnalysis { … }` block — `Attribute`
and friends resolve via parent-namespace lookup; no using needed.
Added a comment explaining.
- TryPattern.sln.DotSettings: removed the RedundantUsingDirective
silence entry — no longer needed.
Verified locally: dotnet build -c Release across all 10 test TFMs
(net462, net472, net48, net481, net5.0, net6.0, net7.0, net8.0, net9.0,
net10.0) — all Build succeeded, 0 errors. dotnet test -c Release -f
net10.0 — 88/88 passing. jb inspectcode TryPattern.sln
--severity=WARNING --no-build — 0 findings.
…ks .editorconfig Trims TryPattern.sln.DotSettings to only src/-facing rules (just CheckNamespace for the NotNullAttribute polyfill). Everything else moves to the folder .editorconfig that it actually pertains to, so the scope of each suppression is documented at the code it affects. Moved to tests/.editorconfig (test-only): - S1481 (unused local 'unused') - S2190 (loop without break) - S2930 (dispose CTS) - S3928 (paramName in ArgumentException) - MA0012 (NullReferenceException reserved) - MA0015 (paramName in ArgumentException) - VSTHRD003 (awaiting external Task) - RS0016 / RS0037 (PublicApiAnalyzer noise — tests have no PublicAPI.Shipped.txt to opt in) - resharper_access_to_disposed_closure_highlighting - resharper_redundant_suppress_nullable_warning_expression_highlighting - resharper_return_type_can_be_not_nullable_highlighting Moved to benchmarks/.editorconfig (benchmark-only): - RS0016 / RS0037 (same rationale) Kept in TryPattern.sln.DotSettings: - CheckNamespace (the ONE finding on src/): the polyfill file at src/Wolfgang.TryPattern/NotNullAttribute.cs declares its type inside `namespace System.Diagnostics.CodeAnalysis` deliberately. Renaming to the src file location's namespace would break the polyfill. R# .DotSettings has no file-scoped syntax, so this one is global. Verified locally: `jb inspectcode` reports 0 findings after the migration.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prepares the solution to be clean under JetBrains ReSharper InspectCode by removing reliance on SDK implicit usings, adding explicit file-scoped using directives where needed, and scoping analyzer suppressions to the appropriate folders/projects.
Changes:
- Removed
<ImplicitUsings>from src/tests/benchmarks projects and added explicitusingdirectives to affected source/test/benchmark files. - Scoped analyzer/InspectCode suppressions into
tests/.editorconfig,benchmarks/.editorconfig, and a targeted<NoWarn>in the VB net462 example project. - Added a solution-wide ReSharper
.DotSettingsentry to suppressCheckNamespacefor theNotNullAttributepolyfill case, and simplified aSuppressMessageattribute usage inResult<T>.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TryPattern.sln.DotSettings | Adds solution-wide ReSharper/InspectCode severity override for CheckNamespace. |
| src/Wolfgang.TryPattern/Wolfgang.TryPattern.csproj | Removes implicit usings to require explicit per-file usings across TFMs. |
| src/Wolfgang.TryPattern/Result.cs | Simplifies SuppressMessage usage to rely on existing using System.Diagnostics.CodeAnalysis;. |
| src/Wolfgang.TryPattern/NotNullAttribute.cs | Removes a redundant using System; inside the polyfill context. |
| tests/Wolfgang.TryPattern.Tests.Unit/Wolfgang.TryPattern.Tests.Unit.csproj | Removes implicit usings and documents rationale for explicit per-file usings. |
| tests/Wolfgang.TryPattern.Tests.Unit/RunFuncTests.cs | Makes using System; unconditional. |
| tests/Wolfgang.TryPattern.Tests.Unit/RunActionTests.cs | Makes using System; unconditional. |
| tests/Wolfgang.TryPattern.Tests.Unit/RunAsyncFuncTests.cs | Makes framework usings unconditional; adds threading usings needed for token tests. |
| tests/Wolfgang.TryPattern.Tests.Unit/RunAsyncActionTests.cs | Makes framework usings unconditional; adds threading usings needed for token tests. |
| tests/Wolfgang.TryPattern.Tests.Unit/ResultTests.cs | Adds explicit using System; required without implicit usings. |
| tests/Wolfgang.TryPattern.Tests.Unit/ResultOfTTests.cs | Adds explicit using System; required without implicit usings. |
| tests/.editorconfig | Adds InspectCode-aligned suppressions for test-only analyzer noise. |
| benchmarks/Wolfgang.TryPattern.Benchmarks/Wolfgang.TryPattern.Benchmarks.csproj | Removes implicit usings to match src conventions. |
| benchmarks/Wolfgang.TryPattern.Benchmarks/TryBenchmarks.cs | Adds explicit System/Tasks usings needed without implicit usings. |
| benchmarks/.editorconfig | Adds InspectCode-aligned suppressions for benchmark-only analyzer noise. |
| examples/VB.DotNet462.Example/VB.DotNet462.Example.vbproj | Adds scoped RS0030 suppression for the net462 VB sync-IO example. |
Chris-Wolfgang
commented
Jul 13, 2026
Per review — the code speaks for itself, no comment needed.
7 tasks
This was referenced Jul 28, 2026
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
Non-protected companion to #245 (which adds the ReSharper InspectCode CI job — protected file, admin-bypass). This PR carries all the source + config hygiene that surfaced from running
jb inspectcodelocally against the solution BEFORE merging #245. Split out perprotected-file-pr-splitso the analyzer/source changes go through normal review + status checks, and #245 only needs admin-bypass for the one workflow-file line-count.What's in this PR
Source: 1 real InspectCode finding fix
src/Wolfgang.TryPattern/Result.cs— shortened[System.Diagnostics.CodeAnalysis.SuppressMessage(...)]to[SuppressMessage(...)]; theusing System.Diagnostics.CodeAnalysis;at the top already imports it.File-scoped explicit
usingdirectives (drop<ImplicitUsings>)<ImplicitUsings>enable</ImplicitUsings>from the src, tests, and benchmarks csproj files.usingdirectives to every file that needed them:Try.cs,Result.cs, 6 test files,TryBenchmarks.cs.#if !NET6_0_OR_GREATER using System; #endifconditional guards in 4 test files (usings are now unconditional and needed on all TFMs).using System;insideNotNullAttribute.cs'snamespace System.Diagnostics.CodeAnalysis { ... }block (Attributeresolves via parent-namespace lookup).Rationale: the
using System;etc. now compile as needed on every TFM (net462 through net10.0), not "redundant" on modern TFMs where ImplicitUsings would silently provide them. RemovesRedundantUsingDirectivefrom InspectCode noise without needing to silence the rule.Analyzer suppressions — moved to their scoped folder
Every suppression now lives next to the code it applies to:
tests/.editorconfig— S1481 / S2190 / S2930 / S3928 / MA0012 / MA0015 / VSTHRD003 (already NoWarn'd in tests csproj; duplicated here because InspectCode doesn't read csproj<NoWarn>), + RS0016 / RS0037 (PublicApiAnalyzer noise — tests have noPublicAPI.Shipped.txt), + 3 ReSharper-native rules (access-to-disposed-closure, redundant-suppress-nullable, return-type-can-be-not-nullable).benchmarks/.editorconfig— RS0016 / RS0037 added.examples/VB.DotNet462.Example.vbproj<NoWarn>— RS0030 (sync-IO ban) scoped to the one project that legitimately cannot use async File APIs on net462.TryPattern.sln.DotSettings— trimmed to only the ONE truly-solution-wide entry:CheckNamespaceon the src polyfill file.Test plan
dotnet build -c Releaseclean across all 10 test TFMs (net462, net472, net48, net481, net5.0, net6.0, net7.0, net8.0, net9.0, net10.0)dotnet test -c Release -f net10.0— 88/88 passingjb inspectcode TryPattern.sln --severity=WARNING --no-build— 0 findingsMerge sequence
Merge THIS PR first, then #245 (admin-bypass). That way when the new InspectCode CI job runs its first cycle on main, main is already InspectCode-clean.