Packaging changes for UI framework support

.editorconfig

@@ -258,4 +258,10 @@ csharp_space_between_square_brackets = false
 dotnet_diagnostic.IDE0290.severity = none
 
 # Namespace style.
-csharp_style_namespace_declarations = block_scoped
+csharp_style_namespace_declarations = block_scoped
+dotnet_diagnostic.IDE0290.severity = none
+
+# Target-typed new expressions
+# We will probably adopt these at some point, but for some reason the IDE only just started complaining about them,
+# and I don't want to deal with all these while in the middle of the Slight Deunification.
+dotnet_diagnostic.IDE0090.severity = none

Rx.NET/Documentation/adr/0003-uap-targets.md

@@ -127,6 +127,18 @@ Without these explicit settings, those first two values would have become `UAP,V
 
 However, only _some_ properties should use the old name. We need to set _all_ of these properties, because otherwise, other parts of the build system get confused (e.g., NuGet handling). So we need the ".NETCore" name in some places, and the "UAP" name in others.
 
+Also note that the package validation tooling (which we use to ensure that `System.Reactive` continues to present the same API as it always did despite now being mostly a facade DLL) turns out not to understand the `.NETCore,Version=v5.0` TFM. So for that to work, we need to put the TFM back how it was later in the build process, which is why we have this target:
+
+```xml
+<Target Name="_SetUwpTfmForPackageValidation" BeforeTargets="RunPackageValidation">
+<ItemGroup>
+    <PackageValidationReferencePath Condition="%(PackageValidationReferencePath.TargetFrameworkMoniker) == '.NETCore,Version=v5.0'" TargetFrameworkMoniker="UAP,Version=10.0.18362.0" TargetPlatformMoniker="Windows,Version=10.0.18362.0" />
+</ItemGroup>
+</Target>
+```
+
+This also sets the target platform moniker to indicate that this is a Windows-specific TFM, something that the package validation tooling doesn't seem to understand otherwise. (But we do need the target platform identifier to be `UAP` earlier on in the build for various other things to work, which is why we only switch this just before package validation runs.)
+
 
 #### Compiler Constants
 

Rx.NET/Documentation/adr/0006-uwp-legacy-threadpoolscheduler-in-facade.md

@@ -0,0 +1,75 @@
+# The `System.Reactive` legacy facade contains the UWP-specific `ThreadPoolScheduler`
+
+[`System.Reactive` NuGet package](https://www.nuget.org/packages/System.Reactive/), which used to be the main Rx.NET package, now exists for backwards compatibility. It is mostly a 'facade' containing type forwarders. However, the `uap10.0.18362` target (the target for UWP applications that are _not_ using the .NET runtime support for UWP that was added in .NET 9) includes a `ThreadPoolScheduler` class, and does not use a type forwarder for that type. This document explains why.
+
+## Status
+
+Proposed
+
+
+## Authors
+
+@idg10 ([Ian Griffiths](https://endjin.com/who-we-are/our-people/ian-griffiths/)).
+
+
+## Context
+
+As described in [ADR 0005](0005-package-split.md), the [`System.Reactive` NuGet package](https://www.nuget.org/packages/System.Reactive/) is no longer the main Rx.NET package. `System.Reactive.Net` is now the main package, with all UI-framework-specific functionality moved into separate packages. This means applications only get support for a UI framework if they asked for it. This fixes a long-standing problem in which self-contained applications using Rx would get a complete copy of the WPF and Windows Forms frameworks even if they used neither.
+
+The [`System.Reactive`](https://www.nuget.org/packages/System.Reactive/) package still exists of course, but its purpose is now backwards compatibility. It is marked as obsolete, to encourage people to move on to the new `System.Reactive.Net` component (and, if required, to add reference to whichever UI-framework-specific Rx.NET integration components they require).
+
+`System.Reactive` needs to retain the same API surface area as in previous versions, so that when an application using components build for, say, Rx 6.0, ends up using a later version of Rx, those older components will still run. If a library has a reference to `System.Reactive` and uses the `System.Reactive.Linq.Observable` class from that component, the CLR will discover that `System.Reactive` does not define this type, and instead contains a type forwarder entry referring to the type of that name in `System.Reactive.Net`.
+
+The situation is similar for UI-framework-specific types. If a library has a reference to `System.Reactive` and uses the `System.Reactive.Concurrency.ControlScheduler` type, again the CLR will discover the type forwarder in `System.Reactive`. But this time, the forwarder will refer to `System.Reactive.For.WindowsForms`, because that is the new home of this UI-framework-specific type. If an application still references `System.Reactive` (or uses libraries that reference this), it will end up with implicit transitive dependencies on all of the UI-framework-specific packages. This is the direct equivalent to how things were back in Rx 6.0, because `System.Reactive` contained all the UI-framework-specific code. It's just that this fact is now visible in the NuGet package dependency structure. The important change here is that once an application move off `System.Reactive` and onto `System.Reactive.Net` (and once all of its Rx.NET-using dependencies have also done so) it will no longer get UI-frameworkspecific code unless it explicitly asks for it with a suitable package reference.
+
+There's one wrinkle in this: UWP's specialized `ThreadPoolScheduler`.
+
+`ThreadPoolScheduler` should be a UI-framework-independent type. It is available in all Rx.NET targets, including `netstandard2.0` and the no-UI-framework-available `netX.0` targets. (E.g., the `net6.0` target in Rx 6.0,.) So it belongs in `System.Reactive.Net`. The problem is that the `System.Reactive` UWP target (the `uap10.0.18362` TFM) contains a slightly different version of this type than all the other targets. It has:
+
+* Three public constructors
+  * a default constructor
+  * a constructor accepting a `WorkItemPriority` argument
+  * a constructor accepting `WorkItemPriority` and `WorkItemOptions` arguments
+* Read-only `Priority` and `Options` properties that report the `WorkItemPriority` and `WorkItemOptions` supplied at construction
+
+It makes these available because it is implemented on top of the Windows Runtime [`Windows.System.Threading.ThreadPool`](https://learn.microsoft.com/en-us/uwp/api/windows.system.threading.threadpool?view=winrt-18362). All the other target use the .NET runtime library's [`System.Threading.ThreadPool`](https://learn.microsoft.com/en-us/dotnet/api/system.threading.threadpool). This was unavailable in early versions of UWP, necessitating a different implementation of `ThreadPoolScheduler` on that platform. UWP has supported `netstandard2.0` since Windows 10.0.16299 (aka 1709, aka the 'Windows 10 Fall Creators Update'), released in 2017, so there's no longer an absolute requirement for a UWP-specific `ThreadPoolScheduler`: the `netstandard2.0` Rx.NET implementation now works just fine.
+
+However, by the time UWP did get support for .NET `ThreadPool`, it was not possible to modify the UWP implementation to use it. This is because those additional public members described above can only be offered when using the `Windows.System.Threading.ThreadPool`: the `WorkItemPriority` and `WorkItemOptions` and types are specific to that particular thread pool.
+
+Legacy code written for UWP using Rx 6.0 or older may expect `ThreadPoolScheduler` to offer these members. Therefore it is absolutely necessary for the `ThreadPoolScheduler` obtained through a reference to `System.Reactive` when targetting UWP to provide the UWP-specific implementation.
+
+There are a few ways we could achieve this:
+
+1. Continue to have `System.Reactive.Net` offer a UWP-specific target, and have `System.Reactive` forward to the `ThreadPoolScheduler` in `System.Reactive.Net`
+2. Define a `System.Reactive.Concurrency.ThreadPoolScheduler` in `System.Reactive.For.Uwp`, and have `System.Reactive` forward to the `ThreadPoolScheduler` in `System.Reactive.Net`
+3.
+
+Note that in options 2 and 3, the `System.Reactive.Net` assembly would define its own `ThreadPoolScheduler`. UWP applications would use the `netstandard2.0` target, so if they reference `System.Reactive.Net` directly, they'll get that `ThreadPoolScheduler`, which will not have 
+
+
+## Decision
+
+We have chosen option 3.
+
+Our view is that it was a mistake to add UWP-specific members to the `ThreadPoolScheduler`. We do not want that to be a feature of Rx.NET in normal use. Furthermore, the whole point of the repackaging, of which this change forms a part, was to remove all UI-framework-specific code from the main Rx.NET package. For these reasons, we reject option 1 above.
+
+Since we consider the incorporation of UWP-specific members into `ThreadPoolScheduler` to be a mistake, we want to deprecate their use. To support the UWP-specific functionality, we define a new `UwpThreadPoolScheduler` in the `System.Reactive.For.Uwp` library, so anyone requiring either the UWP-specific constructors or properties, or who rely on some difference in behaviour between the .NET thread pool used by the `netstandard2.0` `ThreadPoolScheduler` and the `Windows.System.Threading.ThreadPool` can have this, they just need to ask for it explicitly. In order to discourage 
+
+But it is necessary to support legacy code. So anything built against `System.Reactive` v6 or earlier that targets UWP must continue to get the
+
+
+
+## Consequences
+
+Positive
+* Legacy code built against `System.Reactive` continues to run with no change in behaviour
+* The main Rx.NET component, `System.Reactive.Net` is completely free from any UI-framework-specific code
+* Any developer using the UWP-specific members in the legacy `System.Reactive` component will be told to use the new `UwpThreadPoolScheduler` when they upgrade to a new version of Rx.NET (but can continue to use the old one if they really want to)
+
+Negative:
+* Two identically-named types
+
+The only way to avoid that would have been to continue to offer a `uap10.0.18362` target in `System.Reactive.Net`. Since new application development on UWP is strongly discouraged, this would seem like a misstep. Moreover, the continued presence of UWP in our build has caused increasing levels of pain over the year, so we really don't want to offer `uap10.0.18362` targets except in cases where they are absolutely required (e.g. in the UWP-specific component and test suite); we hope to move those out into a completely separate project at some point so that we can finally avoid all the problems the UWP causes for the build process.
+
+
+What about .NET 9 on UWP?

Rx.NET/Documentation/adr/0007-api-compatibility-verification.md

@@ -0,0 +1,109 @@
+# Use .NET SDK Package Validation To Ensure Backwards Compatibility
+
+ Migration from PublicApiGenerator to .NET SDK Package Validation
+
+
+## Status
+
+Proposed
+
+
+## Authors
+
+@idg10 ([Ian Griffiths](https://endjin.com/who-we-are/our-people/ian-griffiths/)).
+
+
+## Context
+
+The `System.Reactive` component was previously the main Rx.NET component. Unfortunately, for the reasons described in [ADR-0005](0005-package-split.md), it was necessary to demote this package, to enable the UI-framework-specific components to be removed from the main Rx.NET package. `System.Reactive` now has just one job: to provide backwards compatibility. This means it must offer the same API surface area as the v6.0 release.
+
+Additionally, we require that the new main Rx.NET component, `System.Reactive.Net`, maintains backwards compatibility with previous versions. Of course, the very first version to ship will be a special case: there are no older versions with which to maintain binary compatibility. For this first version, the mechanism guaranteeing that `System.Reactive.Net` provides full Rx.NET functionality is the fact that the v7 `System.Reactive` legacy component is compatible with its v6 predecessor: in order for that component to provide full backwards compatibility, all of the Rx.NET v6 functionality must be present _somewhere_ in Rx.NET v7. But once v7.0.0 ships, we will need a mechanism to ensure that all future versions are compatible.
+
+### Problems with the existing API Compatibility Tests
+
+Rx.NET has for years executed tests intended to verify that the public API does not change by accident. These test have used `[PublicApiGenerator`](https://github.com/PublicApiGenerator/PublicApiGenerator) to generate a C# source file that contains the public-facing API of an assembly. For example, here's a fragment of the output it generates:
+
+```cs
+namespace System.Reactive
+{
+    public sealed class AnonymousObservable<T> : System.Reactive.ObservableBase<T>
+    {
+        public AnonymousObservable(System.Func<System.IObserver<T>, System.IDisposable> subscribe) { }
+        protected override System.IDisposable SubscribeCore(System.IObserver<T> observer) { }
+    }
+```
+
+As this illustrates, this isn't technically legal C#: the compiler would reject that `SubscribeCore` method because the method has a return type but the body has no `return` statement. However, the output of this tool isn't meant to be compiled: its job is only to capture the public-facing types and methods that an assembly defines, so it uses just enough C# syntax to do that.
+
+The Rx test suite included a `Tests.System.Reactive.ApiApprovals` project which generated files of this form for various Rx.NET assemblies and compared them with files containing the expected results. This prevented us from changing the public API accidentally.
+
+This worked fairly well but it had some significant shortcomings:
+
+* only the .NET Framework target was verified
+* the legacy facade packages weren't tested (unless you count the slightly curious `System.Reactive.Observable.Aliases` as a facade, which arguably it is, but it provides wrapper types not type forwarders)
+* the `PublicApiGenerator` library is incapable of understanding type forwarders
+
+The first issues could be overcome with a bit of work, but the second issue is an upshot of the third: legacy facade packages expose public types that are actually defined in other packages, using `TypeForwardedToAttribute` assembly-level attributes. These have a completely different representation in the low-level metadata format, and are also slightly tricksy when used through reflection. Tools aiming to describe or compare public APIs need to take special steps to process type forwarders correctly, and `PublicApiGenerator` does not do this.
+
+This type forwarder problem means we can't simply add the existing legacy facades to the API Approvals test project, because it is constitutionally incapable of dealing with them. It also means that this existing test project is entirel unsuited to the important job of ensuring that the v7 version of `System.Reactive`, which is now a hollowed-out shell consisting almost entirely of type forwarders, has the exact same API as v6 of `System.Reactive`.
+
+### Possible Replacements for `PublicApiGenerator`
+
+Options:
+
+1. Fix `PublicApiGenerator`
+2. Write our own alternative to `PublicApiGenerator`
+3. Use the reference assembly generation tools that Microsoft uses
+4. Use the package validation built into the .NET SDK and after we ship 7.0.0, enable baselining relative to that release
+5. The .publicApi thing with explicit listing of all known APIs. I think this is what Aspire and Orleans are using?
+
+Option 4 looks pretty good for the legacy package, `System.Reactive`, because our intention is for that never to change ever again. (In fact we should also enable it for the other legacy facade packages.)
+
+However, when it comes to ongoing maintenance of the new `System.Reactive.Net`, option 4 has one shortcoming: it doesn't provide a great way to add new items deliberately. By default, baseline checks verify that no existing features have been changed or removed, but will permit new features. You can enable a strict mode that reports changes of any kind, which will flag additions as well as removals. This prevents accidental additions, but what are you supposed to do when you mean to add a new item?
+
+We can add suppressions to prevent warnings when adding new items, but this is an awkward mechanism when the intention is to add new public members. The old `PublicApiGenerator` approach had an advantage here: there was a specific artifact that reflected precisely what we intend the API to be. When adding new items, we always edit that file, meaning that it's always perfectly clear that a new API feature was added intentionally.
+
+TBD: look more into the .publicApi feature to see if it will work for us.
+
+```xml
+    <PackageReference Include="Microsoft.CodeAnalysis" Version="4.14.0" PrivateAssets="All" />
+    <!--<PackageReference Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="4.14.0">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageReference Include="Microsoft.DotNet.ApiCompat.Task" Version="9.0.301" />-->
+    <PackageReference Include="Microsoft.DotNet.GenAPI.Task" Version="9.0.301-servicing.25272.5" PrivateAssets="All" />
+```
+
+
+
+
+### As Usual, Legacy UWP Makes Things Awkward
+
+
+Currently need `<SuppressTfmSupportBuildWarnings>` and I'm not quite sure why.
+
+Also doing this:
+
+```xml
+  <Target Name="_SetUwpTfmForPackageValidation" BeforeTargets="RunPackageValidation">
+    <ItemGroup>
+      <PackageValidationReferencePath Condition="%(PackageValidationReferencePath.TargetFrameworkMoniker) == '.NETCore,Version=v5.0'" TargetFrameworkMoniker="UAP,Version=10.0.18362.0" TargetPlatformMoniker="Windows,Version=10.0.18362.0" />
+    </ItemGroup>
+  </Target>
+```
+
+It's possible that this is necessary only because I got my hackery for making UWP projects build in the modern SDK wrong. I could try specifying these monikers from the start. However, I have a recollection that some bits of the build actually depend on the wrong TFM being set. So this technique of modifying it just before package validation may be the only way.
+
+
+
+## Decision
+
+* Legacy `System.Reactive` package to use `<EnablePackageValidation>` and `<PackageValidationBaselineVersion>` to ensure compatibility with Rx v6.0 (the last version in which `System.Reactive` was the main package)
+
+Need `<ApiCompatSuppressionFile>` because the tooling detects not just inconsistency with a baseline version but also inconsistencies between targets within the package. Rx 6 already has a couple of internal consistencies as a result of the `ThreadPoolScheduler` it supplied for UWP being slightly different from the type of the same name supplied for all other platforms. 
+
+
+For the new `System.Reactive.Net`: TBD.
+
+## Consequences

Rx.NET/Integration/LinuxTests/LinuxTests.csproj

@@ -28,8 +28,8 @@
     <PackageReference Include="MSTest.TestAdapter" Version="3.0.2" />
     <PackageReference Include="MSTest.TestFramework" Version="3.0.2" />
     <PackageReference Include="xunit.assert" Version="2.4.2" />
-    <PackageReference Include="System.Reactive" Version="6.1.0-preview*" />
-    <PackageReference Include="Microsoft.Reactive.Testing" Version="6.1.0-preview*" />
-    <PackageReference Include="System.Reactive.Observable.Aliases" Version="6.1.0-preview*" />
+    <PackageReference Include="System.Reactive" Version="7.0.0-preview*" />
+    <PackageReference Include="Microsoft.Reactive.Testing" Version="7.0.0-preview*" />
+    <PackageReference Include="System.Reactive.Observable.Aliases" Version="7.0.0-preview*" />
   </ItemGroup>
 </Project>