breaking: satisfy the analyzer gate across every target framework - #4432
Merged
Conversation
- Every dotnet_diagnostic severity is now error. - Adds the newest analyzer rules: SST1535, SST1536, SST1537, SST2019, SST2497, SST2498 and PSH1023. - The file is now identical across refit, reactiveui, Primitives, splat and Akavache apart from file_header_template.
glennawatson
force-pushed
the
build/analyzer-gate-all-rules-error
branch
from
August 11, 2026 12:32
8d9ed71 to
3bcedde
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4432 +/- ##
==========================================
+ Coverage 93.35% 93.89% +0.54%
==========================================
Files 347 359 +12
Lines 14921 15104 +183
Branches 1553 1568 +15
==========================================
+ Hits 13929 14182 +253
+ Misses 731 679 -52
+ Partials 261 243 -18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
glennawatson
force-pushed
the
build/analyzer-gate-all-rules-error
branch
11 times, most recently
from
August 12, 2026 22:21
5fa7280 to
b0e6499
Compare
ChrisPulman
approved these changes
Aug 12, 2026
glennawatson
force-pushed
the
build/analyzer-gate-all-rules-error
branch
from
August 13, 2026 00:06
b0e6499 to
1be8ded
Compare
- Bump the analyzer suite to 3.43.2, activating 18 rules the .editorconfig already set to error but the pinned version did not ship. - Fix the resulting violations across the Apple, Android, MAUI, WinUI and shared sources: missing [DebuggerDisplay] and AggressiveInlining, XML doc element ordering, undocumented <exception> elements, single-use locals, trailing guard inversion and explicit enum values. - Complete the PascalCase tuple element rename. Several declarations were renamed earlier while their use sites were left camelCase, which broke binding and in turn made dead-code and discard rules fire spuriously. - Replace check-then-set dispose flags with an Interlocked int latch, reading it through Volatile.Read where the type also swaps it. - Declare the MemberNotNull polyfill in System.Diagnostics.CodeAnalysis. The compiler matches that attribute by fully qualified name, so in the global namespace it bound but performed no null-state analysis wherever the BCL lacks the type, which is why RoutingState needed a CS8618 pragma. - Push each navigation stack entry when restoring the initial stack. The loop resolved the current view model every iteration, so an N-deep stack came back as the same view model N times. - Build the auto data template against a namespace WinUI can resolve. It used WPF's clr-namespace prefix, which WinUI's parser rejects, and hardcoded the namespace so it was also wrong under REACTIVE_SHIM - so the hook threw in exactly the case it exists for, an ItemsSource binding with no ItemTemplate. - Return null from the table view source title overloads by declaring them nullable, and assert it where UIKit declares the view overloads non-null. Product code now carries no warning suppressions. - Share the activation-signal plumbing the platform view types each repeated. They extend unrelated platform base classes, so the teardown, the manual activation scheduling and the appear/disappear handling move to one helper under src/Shared, linked into each leaf so every assembly compiles its own internal copy. - Turn off the Android resource designer. None of these libraries ship Android resources, so it only added an empty public Resource type to ten assemblies; control wiring resolves identifiers by reflection against the consuming app. - Add ReactiveUI.WinUI.Tests, which hosts a real WinUI application on a dedicated STA thread and marshals each test onto its dispatcher queue. The WinUI control surface previously had no test project at all. - Cover the activation helper, the Blazor component state and reactivity sink, and the MAUI suspend helper to 100% line and branch. The Blazor types are only reachable from their own assembly, so ReactiveUI.Blazor now grants InternalsVisibleTo to its test project. - Exclude ReactiveUI.AndroidX from coverage measurement. Its activities and fragments only construct on a device, so it has no test project and always reported 0%. ReactiveUI.Maui stays measured - its tests run on the CI leg. - Relax SST2334 and PSH1410 for tests only, and silence two rules whose intent is already enforced by equivalents elsewhere in the configured set. Most of this code had never been analysed: the Apple target frameworks failed to compile, so every project downstream of them was skipped. Unblocking those took the solution from 82 assembly outputs to 16000. BREAKING CHANGE: tuple element names on public APIs are now PascalCase, and ReactiveTableViewSource.TitleForHeader/TitleForFooter now return string?.
glennawatson
force-pushed
the
build/analyzer-gate-all-rules-error
branch
from
August 13, 2026 00:36
1be8ded to
86e6dd6
Compare
glennawatson
changed the title
glennawatson
changed the title
glennawatson
enabled auto-merge (squash)
August 13, 2026 00:43
|
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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, and breaking. Raises the analyzer gate to error across the whole repo, fixes everything it reports, and fixes five real defects found along the way.
What is the new behavior?
Every analyzer rule is enforced as an error on every target framework, and the sources satisfy it.
.editorconfigalready set 18 rules toerrorthat the pinned version did not ship, so those severity lines were silently inert. 3.43.1 also stopped requiring[DebuggerDisplay]on types with no members, so ~118 content-free[DebuggerDisplay("TypeName")]attributes and 71 now-orphanedusing System.Diagnostics;directives are gone; 54 remain where the type genuinely has members.[MemberNotNull]actually works off .NET Core. The polyfill declared the attribute in the global namespace; the compiler matches it by fully qualified name, so wherever the BCL lacks the type it bound but performed no null-state analysis. It now lives inSystem.Diagnostics.CodeAnalysislike every other polyfill in that folder.ReactiveUI.Reactiveno longer collides withReactiveUI.Platforms/net/ComponentModelFallbackConverter.cswas missing its#if REACTIVE_SHIMnamespace switch, soReactiveUI.Reactive.dllpublishedReactiveUI.ComponentModelFallbackConverter-- the exact full nameReactiveUI.dllpublishes -- on 11 target frameworks. Referencing both packages gave CS0433.RoutedViewHostresolved the current view model on every iteration of the stack loop, so an N-deep stack came back as the same view model repeated N times.AutoDataTemplateBindingHookbuilt its template with WPF'sclr-namespace:prefix, which WinUI's parser rejects, and hardcoded the namespace so it was also wrong underREACTIVE_SHIM. The hook threw in exactly the case it exists for.ReactiveUI.WinUI.Testshosts a real WinUI application on a dedicated STA thread and marshals each test onto its dispatcher queue; that surface previously had no tests at all.SwitchSignal<T>/MergeSignal<T>from ReactiveUI.Primitives, and several event-subscription sinks, converters and registrations were folded together.What is the current behavior?
Most of this code was never analysed, because it was never compiled.
The Apple target frameworks failed to build, so every project downstream of them was skipped. Unblocking them took the solution from 82 assembly outputs to 16000, and each newly-reachable project then reported violations for the first time.
What might this PR break?
x.self,x.section,x.requestCode-- must use the PascalCase form. This is the main reason the PR is marked breaking.ReactiveTableViewSource.TitleForHeaderandTitleForFooternow returnstring?. They always could return null; the signature said otherwise and a suppression hid it.<RootNamespace>.Resourceno longer exists in the Android assemblies. It was generated automatically, was empty, and was never declared public API. A project that needs it can setAndroidGenerateResourceDesigner=true.ReactiveUI.Reactivenow publishesReactiveUI.Reactive.ComponentModelFallbackConverteron the 11 target frameworks where it previously published the collidingReactiveUI.name. Code referencing the old full name must update; code referencing both packages compiles for the first time.[MemberNotNull]-annotated members. OnlyRoutingStateuses the attribute, and it is clean.UpdateTypemembers now carry explicit values, preserving the existing numbering.Checklist
mainbranchAdditional information
Most of the diff is mechanical attribute and doc-comment churn. The parts worth reviewing are:
src/Polyfills/MemberNotNullAttribute.cs-- the namespace correction that makes the attribute functional.src/ReactiveUI/Platforms/net/ComponentModelFallbackConverter.csand the 11 correctedReactiveUI.Reactivebaselines -- the type-collision fix.src/ReactiveUI/Platforms/uikit-common/RoutedViewHost.cs-- the navigation stack restore.src/ReactiveUI.Maui/Common/AutoDataTemplateBindingHook.cs-- the WinUI namespace fix, with a regression guard that materialises the template.src/ReactiveUI.Shared/Internal/WhenAny/-- the sinks now delegate to ReactiveUI.Primitives. This depends on the generation-safety fixes in ReactiveUI.Primitives 7.1.1; against 7.1.0 three concurrency tests fail.src/Shared/ActivationSignals.csand the other newsrc/Shared/helpers -- linked into each leaf rather than shared through a project reference, so every assembly compiles its own internal copy.src/tests/ReactiveUI.WinUI.Tests/-- the new test project and its WinUI host.Verification: a clean rebuild of
reactiveui.slnxon a real Windows host produces 16002 assembly outputs with 0 errors across every target framework, including Apple, Android, Windows desktop, .NET Framework and WinUI.