feat(analyzers): adopt SecuritySharp and enable the full RoslynCommonAnalyzers 3.33.0 rule set#138
Merged
Merged
Conversation
…r set to 3.29.1 - Add SecuritySharp.Analyzers, sharing RoslynCommonAnalyzersVersion with StyleSharp and PerformanceSharp; bump all three to 3.29.1 - Enable all 47 SecuritySharp rules at error - Enable 43 newly available StyleSharp and PerformanceSharp rules at error - Turn off 119 rules in other analyzer groups that duplicate or conflict with the platform rules, each annotated with the rule that covers it - Re-enable SST1121 and SST1200, which were disabled in favour of third-party equivalents - Narrow dead public modifiers on members of internal types, and move usings outside the namespace in PartitionObservable
- Bump RoslynCommonAnalyzersVersion to 3.30.0 - Remove CA1822 suppressions left dangling by retiring the rule, and make NSRunloopSequencer.ScheduleDelayed static now PSH1414 owns the shape - Narrow dead public modifiers on members of internal types across the polyfills, platform sequencers and test helpers - Rewrite literal concatenations as interpolated strings and drop inferable explicit type arguments in the test projects - Assert explicitly in two tests that previously verified nothing, and replace a Thread.Sleep race widener with Thread.SpinWait - Turn off CA1019, which conflicts with SST2324 on internal attributes
Clears the two analyzer false positives that blocked the test projects: SST2505 on TUnit's injected CancellationToken, and SST2324 on a TUnit lifecycle hook that the framework requires to be public.
…al members - DispatchWorkItemBase.Run must stay public: derived types use it to implicitly implement IDispatchWorkItem/IReadyWorkItem, which the compiler rejects on a non-public member (CS0737) - Move the public Dispose above the internal members in the WasmScheduler work items so accessibility ordering holds
- Narrow dead public modifiers in the benchmarks project, the Async.Reactive sequencer seam, and the remaining test helpers - Move the now-static ScheduleDelayed above the instance members - Rewrite literal concatenations and drop inferable type arguments in the benchmarks and Reactive test projects
Clears the PSH1021 false positive on the allocation-measurement harness, which needs GC.Collect to settle the heap before sampling allocations.
Turn off 42 SonarAnalyzer rules whose behaviour is already provided by the in-box .NET (CA)/compiler analyzers, or that target obsolete tech (WCF, CAS, COM, APM, legacy serialization). WinForms STAThread (S4210) stays enabled.
Append the in-box CA/compiler rule (or "obsolete") that replaces each of the 42 Sonar rules disabled in the previous commit, for tracking.
Enable SES1108 at error: a custom server-certificate validation callback that unconditionally returns true disables TLS server authentication.
- PSH1500-PSH1503, PSH1505: ASP.NET Core rules, inert here but keep the set complete - SES1007, SES1008: hand-rolled crypto primitives and no-key XML signature checks - SES1203: a connection string naming a user with an empty password - SST2326, SST2327: narrowing an interface to its implementation, and a type testing its own runtime type instead of dispatching virtually
glennawatson
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
==========================================
+ Coverage 98.07% 98.13% +0.06%
==========================================
Files 676 676
Lines 20972 20958 -14
Branches 2538 2530 -8
==========================================
- Hits 20569 20568 -1
+ Misses 201 191 -10
+ Partials 202 199 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Disable every dotnet_diagnostic.S#### rule. Each disabled line is annotated with the rule that now covers it (an SST/PSH/SES rule, an in-box CA rule, or the compiler/an obsoletion), or marked "no equivalent" for the genuine gaps we do not yet replace (mostly framework-only checks that do not apply to this library, plus WinForms STAThread). Bump RoslynCommonAnalyzersVersion to 3.33.0 ahead of publish.
…ode clarity in PooledDelaySource and WasmScheduler test: update CurrentThreadSequencerTests to use StrongBox for state management
glennawatson
changed the title
ChrisPulman
approved these changes
Jul 20, 2026
DelayUntilClampsElapsedDueTimestampsToZero asserted a still-pending delay against a 100ms due time captured before the preceding assertion ran. A loaded runner can spend longer than that inside the first assertion, so the deadline had already passed and DelayUntil clamped to zero. Assert against a due time no scheduling pause can elapse instead.
|
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
2 participants
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.
What kind of change does this PR introduce?
Build / analyzer configuration, plus the code changes needed to satisfy the newly enabled rules.
What is the new behavior?
SecuritySharp.Analyzers, sharingRoslynCommonAnalyzersVersionwithStyleSharp.AnalyzersandPerformanceSharp.Analyzers. All three move from 3.28.1 to 3.33.0.error: 521 StyleSharp, 110 PerformanceSharp and 54 SecuritySharp rules. Only the four opt-in PerformanceSharp rules stay commented out.SST1121andSST1200, which had been disabled in favour of third-party equivalents.publicmodifiers on members ofinternaltypes, reorders members the accessibility rules now sort differently, and applies the interpolation and type-argument rewrites the new rules ask for.WasmSchedulerdrain so the concurrency retry shells are the only excluded code.What is the current behavior?
Analyzers sit at 3.28.1 with no security analyzer, roughly 130 platform rules unconfigured, and the Sonar, Roslynator and CA groups reporting defects the platform rules already report.
What might this PR break?
error.internalorprivatetypes, so no public API surface changes and the baselines are untouched.Checklist
mainbranchAdditional information
The analyzer packages moved repeatedly while this branch was open, each release clearing a false positive this repo surfaced:
SST2464on reference-equality operators;SST2324on private nested typesSST2505on TUnit's injectedCancellationToken;SST2324on a TUnit lifecycle hookSST2324on an inherited interface implementation and a Blazor[Parameter]PSH1021on an allocation-measurement harnessSST2326scoped to cross-assembly targets, plus an allow-list optionTwo rules are deliberately not enforced, both recorded inline in
.editorconfig:SST2326(and its retired counterpartS3215) - narrowing an interface to a concrete implementation. This is a high-performance core library rather than strictly SOLID code, and narrowing to a known runtime type is a normal fast-path technique here. Measured:foreachoverIEnumerable<T>boxesList<T>'s struct enumerator at 40 bytes per call, where the indexed loop behind the type test allocates nothing.CA3001-CA3012and neighbours) stay on rather than being retired. SecuritySharp reports local shapes only and does no interprocedural taint tracking, so those rules are strictly broader.SonarAnalyzer.CSharpis still referenced while every one of its rules is off. Left in place deliberately for the SonarCloud CI integration; worth a follow-up decision if that turns out not to need it.Local verification covered all 36 projects and the runnable test suites on
net10.0. The Apple, Android, Windows andnet462-net481legs cannot be built on this machine, so CI is the gate for those.