Add PlatformNeutralAssembly property for targeted builds of cross platform assembly

[buyaa-n]

Fixes dotnet/roslyn-analyzers#5023

When cross-platform assembly having targeted builds for specific platforms MSBuild would add an assembly-level attribute for that target which disables the API annotations inside the assembly by the "child APIs should not extend parent platform support" rule. Even though it is an expected scenario by design it is unfortunate to not get any warning about this inconsistency or possible reference of incompatible APIs within the same assembly

For getting warnings in targeted builds in cross-platform assembly we need some way to distinguish between real platform-specific assembly and cross-platform assembly with platform-specific builds. For that, I used MSBuild PlatformNeutralAssembly property which we are adding into targeted builds of cross-platform assembly with this PR and updating the analyzer version which supports this property

[ManickaP]

I tried this change locally and unfortunately it doesn't work as expected.

I commented out the check for "not android" in here:
https://github.com/buyaa-n/runtime/blob/d19d03b23a0be370a9e87db6db28b15ce7503f36/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs#L289

Making it look like:

[SupportedOSPlatformGuard("linux")]
[SupportedOSPlatformGuard("macOS")]
[SupportedOSPlatformGuard("Windows")]
internal static bool IsHttp3Supported() => OperatingSystem.IsLinux() || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS();

And the build didn't produce any warnings/errors for Android. And obviously the resulted dll contains the calls into the guarded methods.
I also tried the same for Windows with the same result.

I also noticed that the dll still contains this:

[assembly: TargetPlatform("Android1.0")]
[assembly: SupportedOSPlatform("Android")]

Which I think plays some role in making this work.

I did fresh rebuild on @buyaa-n branch (git clean -dfx), should I clean something else in the system as well?

[buyaa-n]

I tried this change locally and unfortunately it doesn't work as expected.

I commented out the check for "not android" in here:
https://github.com/buyaa-n/runtime/blob/d19d03b23a0be370a9e87db6db28b15ce7503f36/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs#L289

Making it look like:

[SupportedOSPlatformGuard("linux")]
[SupportedOSPlatformGuard("macOS")]
[SupportedOSPlatformGuard("Windows")]
internal static bool IsHttp3Supported() => OperatingSystem.IsLinux() || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS();

And the build didn't produce any warnings/errors for Android. And obviously the resulted dll contains the calls into the guarded methods.
I also tried the same for Windows with the same result.

I am not that sure what you exactly did, but the value/expression returned from the method has no effect, only think matters is the guard attributes. Now no matter if you have or not !OperatingSystem.IsAndroid() in the expression it will not guard calls in android build, therefore it should warn for the Quic API usages in Android build, make sure you are building in android.

For windows removing OperatingSystem.IsWindows() has no effect, instead you could remove the [SupportedOSPlatformGuard("Windows")] attribute then you should see warning in windows in case it is using Quic APIs, also need to do Windows build

I also noticed that the dll still contains this:

[assembly: TargetPlatform("Android1.0")]
[assembly: SupportedOSPlatform("Android")]

Yes the dll would still have those, we did not change anything in the assembly attribute

I did fresh rebuild on @buyaa-n branch (git clean -dfx), should I clean something else in the system as well?

I don't think so

[ManickaP]

I think that I now understand the limitations here, although I still think that they are unfortunate. It seems like SupportedOSPlatformGuard was created without any connection to OperatingSystem.Is* methods, which under other circumstances work with the analyzer. This seems very fragile because it depends on the attributes and the property expression to be in sync.

What I'd love to see here is something like this:

// No `SupportedOSPlatformGuard` attribute.
internal static bool IsHttp3Supported() => (OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS();
...
// Code guarded with IsHttp3Supported
if (IsHttp3Supported())
{
    _http3Enabled = _poolManager.Settings._maxHttpVersion >= HttpVersion.Version30 && (_poolManager.Settings._quicImplementationProvider ?? QuicImplementationProviders.Default).IsSupported;
}

Basically have IsHttp3Supported replace the (OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() in code we had originally (before #52689), without any additional attributes. You must be able to do that, since it works if you put the expression ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) in code directly.

[buyaa-n]

Yes, guard attributes have no connection to OperatingSystem.Is* methods and they should not, because the guard expression not always formed by OperatingSystem.Is* methods, it could be a custom expression depending on the repo and also could have some other additional codes for capabilities check.

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs

Lines 45 to 46 in 0d7c498

	[UnsupportedOSPlatformGuard("browser")]
	public static bool IsSupported => true;

It is static analysis, should/could not depend and on runtime value, for your example, it is clear that the expression could be used as a guard, but that is not true for all cases

[ManickaP]

LGTM, thanks!