chore: enable the expanded StyleSharp analyzer rule set and resolve all violations#4385
Merged
Conversation
- Bump StyleSharp.Analyzers 3.11.2 -> 3.13.2, Primitives 5.6.0, TUnit 1.56.25 - Enable the new SST rule set and resolve every violation across the core, shared, and platform libraries and the test projects, without suppressions - Convert production LINQ to explicit iteration (SST2233), fusing passes and pre-sizing buffers rather than translating one-to-one - Replace the WhenAny Arity11/12 readiness AND-chains with an O(1) ready counter to satisfy the branch-complexity limit (SST1442) - Collapse return-only branches, assign ignored results to discards, prefer nameof, and apply the remaining modern-usage rules - Disable CA1040, S2357, and SST2205 as duplicates of canonical SST/IDE rules - Resolve multi-TFM name-ambiguity from name shortening via using aliases (System.Linq.Expressions.Expression in WPF) Verified: ReactiveUI.Tests (1872), Builder (217), AOT (49), Blazor (22), Splat (6), Routing (10) all green; libraries and Windows test projects compile clean.
CI surfaced violations in platform TFMs not buildable in the prior Linux pass (net8.0 / net8.0-windows only): - android FlexibleCommandBinder: LINQ affinity lookups -> explicit loops (SST2233) - Maui: discards, nameof, target-typed new, collapsed branch, const message; resolve the multi-TFM Application cref ambiguity (Android.App.Application) with a using alias - benchmarks: target-typed new (SST2202) - fix a net462/net481 nullable regression in ValidationBindingWpf by narrowing with a property pattern (CS8604) Verified: ReactiveUI android (net11.0-android37), ReactiveUI.Maui (net9.0 + android), benchmarks (net9.0), Wpf (net8.0-windows) all clean.
Verified on the dockur Windows guest across net10.0-ios, -maccatalyst, -tvos, and -macos (ReactiveUI, ReactiveUI.Reactive, ReactiveUI.Maui) — all 0/0. - apple-common / uikit-common: target-typed new, discards, null-guard throws folded into ?? throw, and Enumerable.Range/Select chains (CommonReactiveSource, IndexNormalizer, KVOObservableForProperty) rewritten as explicit loops - KVOObservableForProperty: extract reflection lookups into helpers to stay under the nested-flow complexity limit - CommonReactiveSource: concrete List<Update> return types (CA1859) - ios/tvos LinkerOverrides: rewrite the self-referential get+set linker stubs as initializer (setter) + discard read (getter), preserving both accessors - mac: discards and unbound nameof generic argument - RoutedViewHost: rename a lambda parameter so the discard does not collide with the '_' parameter name - UIViewControllerMixins: internal (not private) extension member used across sibling extension blocks
3.13.3 fixes the SST2218 false positive on overload-disambiguating lambda parameters. ReactiveUI.Wpf.Tests and ReactiveUI.WinForms.Tests now build clean without the temporary NoWarn — no analyzer suppressions remain.
ReactiveUI.Primitives ships analyzers; without ExcludeAssets="analyzers" they flow transitively to anyone installing ReactiveUI's platform packages, forcing our analyzers on consumers who didn't opt in. Core/ReactiveUI/Reactive already excluded them — add the same to the 8 platform projects (Wpf, Winforms, WinUI, Maui and their .Reactive variants) so it's consistent. ExcludeAssets="analyzers" drops only the analyzer asset; compile/runtime stay. README documents that analyzers are opt-in.
- ReactiveUI.AndroidX (+ .Reactive) and the shared ChangeSetBinder: discards - ReactiveUI.Maui.Tests / Builder.Maui.Tests: discards, shorten names, move assignment to initializer, drop redundant parens - Example apps (BlazorServer, WpfApp, Wpf, Winforms, Samples.Maui): discards and LINQ FirstOrDefault -> explicit loop Verified clean: AndroidX (android), Maui test projects (net9.0), and four of the five examples. BlazorServer still trips SST1440 on Razor @OnClick handlers that the analyzer can't see in markup (pending analyzer fix).
A full `dotnet build reactiveui.slnx` surfaced TFM-specific files the per-project net8.0 builds missed: - Samples.Maui (LoginPage.xaml.cs, MauiProgram.cs) and Testing.Tests: discards - benchmarks ObservableBenchmarkViewModel: add `using ReactiveUI.Primitives.Signals` so `Signal<int>` resolves by arity (the non-generic Signal factory is in scope globally), keeping the shortened name - bump StyleSharp.Analyzers to 3.13.4 (fixes the SST1440 Blazor @OnClick FP) Full slnx now builds clean (0 errors); Apple TFMs verified separately in dockur.
glennawatson
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4385 +/- ##
==========================================
+ Coverage 89.69% 93.07% +3.37%
==========================================
Files 341 343 +2
Lines 14948 14924 -24
Branches 1502 1570 +68
==========================================
+ Hits 13408 13890 +482
+ Misses 1208 770 -438
+ Partials 332 264 -68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…sal and ThrownExceptions
- Remove ReactiveUI.Core/Mixins/CompatMixins.cs: ForEach/SkipLast had no production callers and SkipLast was shadowed by BCL's Enumerable.SkipLast - Add tests for BindingFallbackConverterRegistry and SetMethodBindingConverterRegistry (GetAllConverters empty/registered, TryGetConverter on an empty registry)
…nd changing-notification filter/dispose
…to the default The switch listed every ExpressionType explicitly only to throw the same NotSupportedException the default arm already produces. Collapse the ~80 redundant arms into a single default that interpolates the node type, keeping the supported Index/MemberAccess cases and the message wording.
…ctly Lift MessageBus's nested SkipFirstObserver into a top-level internal class so its skip-first/error/completion forwarding can be unit-tested directly.
…l intermediate, read-only set)
…n file The shared UIKit control-preservation code was duplicated between the ios and tvos LinkerOverrides (flagged as duplicated new code by Sonar). Move the common controls into a single Platforms/uikit-common/LinkerOverrides.cs (already compiled into ios, tvos and maccatalyst) and keep the iOS-only controls (UISlider, UIRefreshControl, UISwitch) in a separate IosLinkerOverrides class. Verified ios, tvos and maccatalyst compile 0/0 in the dockur guest.
…visual tree The existing binding tests construct the binding against an unrealized Window whose visual tree has no children, so FindControlByName fails and the constructor throws (swallowed by try/catch), leaving Bind() and the binding setup uncovered. Realize a UserControl's visual tree with Measure/Arrange/ UpdateLayout so the control is found and the two-way binding is exercised end-to-end. Verified passing on real Windows in the dockur guest.
…Hook branches) Verified passing on real Windows in the dockur guest.
…andler overloads The existing tests only exercised the default-event overload; add coverage for the named-event, non-generic add/remove, and generic add/remove overloads. Verified passing on real Windows in the dockur guest.
Verified passing on real Windows in the dockur guest.
…er success path The previous test wrongly assumed a read-only property yields a null setter and returns false; GetValueSetterForProperty returns PropertyInfo.SetValue (non-null) for read-only properties, which throws on invoke. Test the writable-member success path instead, which exercises the same reflection API.
view is null-checked before line 259 in ViewModelViewHost; section is dereferenced at line 163 in CommonReactiveSource, so the ?. on it is redundant.
…ndRemove The three AutoPersistCollection overloads each repeated the same dispose-and-remove cleanup callback (flagged as duplicated by Sonar). Extract a shared DisposeAndRemove helper.
ReactiveAppCompatActivity and ReactiveFragmentActivity each nested an identical ActivityResultAwaiter (flagged as duplicated by Sonar), differing only in the Intent nullability of the result tuple. Lift it into a single shared ActivityResultAwaiter and unify ReactiveFragmentActivity's ActivityResult / StartActivityForResultAsync to Intent? (matching ReactiveAppCompatActivity) — a correctness fix, since activity results can legitimately carry a null intent (its OnActivityResult no longer throws on null data). PublicAPI baselines updated for the net10/net11 android TFMs; both build clean.
Add IViewFor<TViewModel> wrappers for two MAUI host types that previously had no reactive equivalent: - ReactiveWindow<TViewModel> — mirrors the existing WPF/WinUI/Mac ReactiveWindow and aligns with .NET 9's CreateWindow direction (Window is now the primary-page host after Application.MainPage was deprecated). - ReactiveTitleBar<TViewModel> — wraps the .NET 9 TitleBar control for VM-bound custom title bars on Windows / Mac Catalyst. Both follow the established Reactive* pattern (ViewModel bindable property, two-way ViewModel<->BindingContext sync). PublicAPI baselines updated (Shipped) across all MAUI TFMs; net9.0 + net10.0-android36.0 build clean. Adds full unit coverage for both controls (12 tests).
…nd new MAUI controls The ReactiveUI.AndroidX.Reactive and ReactiveUI.Maui.Reactive shim projects compile the same sources under the ReactiveUI.Reactive.* namespace and carry their own PublicAPI baselines, which were missed: - AndroidX.Reactive: apply the ReactiveFragmentActivity Intent! -> Intent? result-tuple change (net10/net11 android). - Maui.Reactive: add ReactiveWindow/ReactiveTitleBar entries (shipped). Both projects now build clean.
- ReactiveProperty.Dispose: drop the redundant null-conditional on _disposables (the guard above already proves it non-null). - RoutedViewHost (MAUI): extract page resolution into ResolveCurrentPage so the null-check operates on a method result — Sonar can't see the page being set inside the inline Subscribe callback and wrongly flagged the branch unreachable. - InteractionHandleObservable: move the "_stepRanInline" pending check into an AdoptIfPending helper. The field is set by a callback that ScheduleOrInline may run inline or defer; Sonar assumes scheduler.Schedule runs synchronously and flagged the condition constant. Behaviour is identical.
- Adds coverage for Activation, Builder, ChangeSet, Interaction, RoutingState, DependencyResolver, and other core mixin/extensions. - Validates correct functionality for corner cases such as fallback conversions, lifecycle activation, event handling, and state navigation. - Introduces mock components and specialized stubs to exercise and verify framework behavior across platforms (WinForms, WPF, etc.).
…l assertions - Add validation for the `Initialized` property of `ParameterlessType` alongside type checks. - Update `ParameterlessType` to use `Initialized` instead of `Created`, reflecting its construction state.
…overage - mark ValueChangeRelay and AsyncProjectObservable.Sink OnError/OnCompleted with [ExcludeFromCodeCoverage]: these interface-required callbacks are never invoked because the subscription is always disposed before the source can complete or error, so they were permanently-uncovered defensive code
…NetworkService - `CHAT_MULTICAST_ADDRESS` allows overriding the default hardcoded address. - Updated `MulticastAddress` to dynamically configure from the environment variable if set. - Retains fallback to the default address.
…minal - remove the UDP/multicast chat sample, including its hard-coded multicast address (resolves the SonarCloud S1313 hard-coded-IP finding by deletion) - add a self-contained mock EFTPOS terminal that exercises a lot of the ReactiveUI surface with no networking: the builder, routing between a terminal and a journal screen, a typed suspension host that persists the transaction journal to JSON, an async ReactiveCommand payment flow with IsExecuting/canExecute, ObservableAsPropertyHelper-derived outputs and the message bus
Push the page before mutating the router stack so the headless navigation stack sync never PushAsync-es into an empty navigation stack. net10 MAUI rejected that with an out-of-range List.Insert (net9 tolerated it), failing InvalidateCurrentViewModel_NullPageViewModel_DoesNotUpdate in CI.
- Added a `Flush` method to drain the dispatcher queue and ensure all marshalled work is processed before assertions. - Updated `WpfPropertyBinderImplementationTests` to invoke `Flush` for consistent synchronization in tests
Ensure PayCommand notifications are observed on the main UI scheduler by using PayCommand.WitnessOn(RxSchedulers.MainThreadScheduler) so UI-bound state (Journal, LastTransaction) is updated safely. Also call ApplicationConfiguration.Initialize() before building the WinForms app (and remove the duplicate call) so application configuration is applied early.
|
ChrisPulman
approved these changes
Jun 24, 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
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 / chore + behaviour-preserving refactor. Enables the expanded StyleSharp 3.13.x rule set across the repo, fixes every resulting violation (no
#pragma/SuppressMessagesuppressions), and stops the ReactiveUI.Primitives analyzers from flowing to downstream consumers.What is the new behavior?
reactiveui.slnxbuilds clean —dotnet build reactiveui.slnx -c Release→ 0 errors across all libraries, test projects, examples and benchmarks. Apple TFMs (ios/maccatalyst/tvos/macos) verified separately on a Windows guest.System.Linqconverted to explicit iteration (fusing passes / pre-sizing, not a 1:1 translation), theWhenAnyArity11/12 readiness&&-chains replaced with an O(1)_readycounter, return-only branches collapsed, discards,nameof, target-typednew, expression-bodied accessors, etc.ReactiveUI.Primitives*PackageReference carriesExcludeAssets="analyzers", so installing ReactiveUI no longer drags the Primitives analyzers into a downstream project. The README documents how to opt in.StyleSharp.Analyzers3.11.2 → 3.13.4,ReactiveUI.Primitives→ 5.6.0,TUnit→ 1.56.25.CA1040,S2357,SST2205disabled in.editorconfigas duplicates of the canonical SST/IDE rules already enforced.What is the current behavior?
A narrower analyzer rule set, with a backlog of violations the new rules surface. The Primitives analyzers were excluded on only
ReactiveUI/ReactiveUI.Core/ReactiveUI.Reactive, so the eight platform packages (Wpf, Winforms, WinUI, Maui + their.Reactivevariants) still flowed those analyzers to downstream users.What might this PR break?
Checklist
mainbranchAdditional information
Several of the new rules had false positives, each reported and fixed in a StyleSharp patch release rather than worked around:
this.Log())this.WhenAnyValue)@onclickhandlers bound in markupVerification: full
reactiveui.slnxbuild is green on Linux (Windows TFMs viaEnableWindowsTargeting); Apple TFMs ofReactiveUI,ReactiveUI.ReactiveandReactiveUI.Mauibuild 0/0 on a real Windows guest acrossnet10.0-ios/-maccatalyst/-tvos/-macos.