Skip to content

build(analyzers): adopt StyleSharp, PerformanceSharp and SecuritySharp - #49

Open
glennawatson wants to merge 1 commit into
mainfrom
feature/roslyn-common-analyzers
Open

build(analyzers): adopt StyleSharp, PerformanceSharp and SecuritySharp#49
glennawatson wants to merge 1 commit into
mainfrom
feature/roslyn-common-analyzers

Conversation

@glennawatson

@glennawatson glennawatson commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Build / refactor - analyzer stack migration, dependency updates, and the source changes needed to satisfy the new rules.

What is the new behavior?

  • The analyzer set is StyleSharp, PerformanceSharp and SecuritySharp (3.39.2). The previous style analyzer package, its stylecop.json and its AdditionalFiles entry are gone.
  • .editorconfig is the one from the reactiveui repository, with this repo's copyright header substituted into file_header_template. src/tests/.editorconfig carries the test-scoped relaxations alongside it.
  • Dependencies are updated where the target frameworks allow: TUnit 1.61.38, Verify.TUnit 31.27.0, Verify.SourceGenerators 2.5.0, NSubstitute 6.0.0, Splat 20.2.0, System.Reactive 7.0.0, ReactiveUI 24.0.0, ReactiveUI.SourceGenerators 3.1.0, SourceLink 10.0.301, Basic.Reference 1.8.10, CodeAnalysis.Analyzers 5.6.0, MAUI 10.0.90 on net10.
  • The generator and analyzer compile against Roslyn 4.8.0 through VersionOverride, so consumers on the .NET 8 SDK (Visual Studio 2022 17.8) can still load them. The central Microsoft.CodeAnalysis.CSharp version stays at 4.14.0 because the test and benchmark tooling requires it (Basic.Reference needs 4.11 or later, BenchmarkDotNet needs 4.14 or later).
  • 372 classic extension methods are now C# 14 extension blocks, matching how the reactiveui repository writes them.
  • The abbreviated vmProperty, vmPropertyExpression, vmToViewConverter and viewToVmConverter parameters are now viewModelProperty, viewModelPropertyExpression, viewModelToViewConverter and viewToViewModelConverter, in both the runtime library and the generated dispatch stubs. The generator's parameter matchers accept the old spelling as well, so ReactiveUI's own API shape is still detected.
  • Lock resolves through a target-framework-conditional Using alias (the pattern used in the Primitives repo) instead of #if blocks at each declaration. Index and Range polyfills are added for the targets that predate them.

What is the current behavior?

The repository used the older style analyzer package with a locally maintained .editorconfig, pinned Roslyn at 4.14.0 for every project, declared Lock behind per-site #if NET9_0_OR_GREATER blocks, and exposed the binding APIs through classic extension methods with abbreviated view model parameter names.

What might this PR break?

  • Source-breaking for callers that pass the renamed parameters by name (vmProperty:, vmToViewConverter:). No call sites in this repository used named arguments for them.
  • Source-breaking for callers that supply explicit type arguments in reduced form on the converted extension methods, because the receiver's type parameter now sits on the extension block. No call sites in this repository do this.
  • Consumers below the .NET 8 SDK / Visual Studio 2022 17.8 remain unsupported; the Roslyn 4.8.0 floor is what keeps that line working rather than moving it forward.
  • System.Reactive 7.0.0 has no net462 asset. It resolves through netstandard2.0 for the .NET Framework targets, exactly as 6.1.0 did.

Checklist

  • I have read the Contribute guide
  • Tests have been added or updated (for bug fixes / features)
  • Docs have been added or updated (for bug fixes / features)
  • Changes target the main branch
  • PR title follows Conventional Commits

Additional information

This branch is not finished and CI will fail on it.

  • The solution compiles on every target framework with no compiler errors, but 192 analyzer violations remain, so the analyzer gate is still red. They are concentrated in ReactiveUI.Binding.Tests with a few in the generator: 59 test literals that need named constants, 31 methods over the length limit, 16 unused type parameters on test stubs that mirror ICreatesCommandBinding, and roughly 86 documentation, blank-line, long-line and visibility items.
  • The 13 generator snapshots under src/tests/ReactiveUI.Binding.SourceGenerators.Tests still contain the old parameter names and need regenerating once the build is green, using the AutoVerify flow described in CLAUDE.md.
  • Adopting the new rules surfaced violations in tiers: each project only reports its own once the projects it depends on compile. The runtime library, generator, analyzer, platform projects and test projects each contributed a wave, 4,655 at the peak.
  • Some fixes were applied by the analyzers' own code fixers and carry behavioural weight rather than being pure formatting: TaskCompletionSource instances now pass TaskCreationOptions.RunContinuationsAsynchronously, ignored return values are assigned to discards, and Disposable.Create calls use the state-carrying overload to avoid closure captures. These are worth a careful read during review.
  • Suppressions were added only where the rule cannot be satisfied: fixed-arity WhenAny overloads (parameter count and method length), CallerInfo dispatch stubs with optional parameters, interface-dictated type parameters, the marker interface, the compiler-required IsExternalInit type, and the polyfill enum that mirrors the framework's own flag values. Each carries a justification. Stale suppressions for rules the new configuration no longer enables were removed rather than translated.

@glennawatson
glennawatson force-pushed the feature/roslyn-common-analyzers branch from cbe989c to bfcd5fa Compare July 28, 2026 01:04
- Bring the .editorconfig over from the ReactiveUI repo, keeping this project's
  file header, and add a test-scoped file for the relaxations that only make
  sense in fixtures.
- Update dependencies: TUnit, Verify, NSubstitute, Splat, ReactiveUI 24 and
  System.Reactive 7. Pin the generator and analyzer to Roslyn 4.8 so they keep
  loading under VS 2022 17.8 and the .NET 8 SDK.
- Drop the unused System.Reactive reference from ReactiveUI.Binding and put the
  library on ReactiveUI.Primitives, leaving System.Reactive to
  ReactiveUI.Binding.Reactive.
- Fix a binding defect the analyzer work surfaced: the runtime stub named its
  second converter viewToVmConverter while the generated overload emitted
  viewToViewModelConverter, so a named argument bound to a different parameter
  depending on which path served the call. Both are now viewModelToViewConverter
  and viewToViewModelConverter, matching viewModelProperty.
- Seed the generator test compilations from ReactiveObject as well as
  IReactiveObject. ReactiveUI 24 moved the interface into another assembly, so
  walking references from it no longer reached the one declaring ReactiveObject
  and every ReactiveObject fixture failed to compile.
- Keep the runtime dispatch stubs as classic extension methods. As extension
  block members they outrank the generated concrete overload, so bindings fall
  through to the runtime path and throw instead of dispatching.
- Resolve the analyzer findings across the runtime library, generator, analyzer
  and tests, and regenerate the generator snapshots.
@glennawatson
glennawatson force-pushed the feature/roslyn-common-analyzers branch from bfcd5fa to e258427 Compare July 28, 2026 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant