.NET 10: PlatformCompatibilityAnalyzer not working

[jonathanpeppers]

Analyzer

Diagnostic ID: CA1416: Validate platform compatibility

Analyzer source

• https://github.com/dotnet/roslyn-analyzers/blob/5ed336762c6260a83ece35cd1f6749251452bad0/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

Describe the bug

For Android, we have an API such as:

[SupportedOSPlatform("android22.0")]
public virtual int AccessibilityTraversalAfter { get; set; }

Using the API with SupportedOSPlatformVersion=21.0 does not result in a CA1416 warning.

We first noticed this here, getting the Android workload on a .NET 10 SDK:

• [main] Update dependencies from dotnet/sdk android#9395

In the following integration test:

• https://github.com/dotnet/android/blob/208c52986a9369b0dab196eb6ab0618b521e2f98/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ManifestTest.cs#L1071

Steps To Reproduce

• dotnet new android
• Use button.AccessibilityTraversalAfter
• Observe no warnings

Here is a .binlog if that is helpful: supportedosplatformversion.zip

Expected behavior

Using the API with SupportedOSPlatformVersion=21.0 results in a CA1416 warning.

Actual behavior

Using the API with SupportedOSPlatformVersion=21.0 does not result in a CA1416 warning.

Additional context

I'm guessing the code here would not work as expected for TargetFramework=net10.0, as it has 1 more digit:

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

Lines 284 to 289 in 5ed3367

	var tfmString = options.GetMSBuildPropertyValue(MSBuildPropertyOptionNames.TargetFramework, compilation);
	if (tfmString?.Length >= 4 &&
	tfmString.StartsWith(Net, StringComparison.OrdinalIgnoreCase) &&
	int.TryParse(tfmString[3].ToString(), out var major) &&
	major >= 5)

[ViktorHofer]

Yes the code that you shared looks faulty. Just wanted to add that a component should never parse and interpret the TargetFramework property. Instead, the deferred properties (i.e. TFI and TFV) should be read.

Why? Because with the eventually coming TFM aliasing feature, this analyzer will stop working:

<PropertyGroup>
  <TargetFrameworks>vs17.12;vs17.13</TargetFrameworks>
</PropertyGroup>

<PropertyGroup>
  <TargetFrameworkIdentifier>.NETCoreApp</TargetFrameworkIdentifier>
  <TargetFrameworkVersion>10.0</TargetFrameworkVersion>
</PropertyGroup>

<PropertyGroup>
  <RoslynAnalyzerVersion Condition="'$(TargetFramework)' == 'vs17.12'">4.12</RoslynAnalyzerVersion>
  <RoslynAnalyzerVersion Condition="'$(TargetFramework)' == 'vs17.13'">4.13</RoslynAnalyzerVersion>
</PropertyGroup>

[ericstj]

cc @jeffhandley @dotnet/dotnet-analyzers -- this is going to be a blocking issue for .NET 10.0. We should get it fixed ASAP.

I think the fix should be to pass in the parsed values for TargetFrameworkIdentifier and TargetFrameworkVersion as @ViktorHofer suggests and use those instead (rather than taking a NuGet dependency to parse the framework value). Looks to me like that might mean adding to

roslyn-analyzers/src/Utilities/Compiler/Options/MSBuildPropertyOptionNames.cs

Lines 15 to 24 in 5ed3367

	internal static class MSBuildPropertyOptionNames
	{
	public const string TargetFramework = nameof(TargetFramework);
	public const string TargetPlatformMinVersion = nameof(TargetPlatformMinVersion);
	public const string UsingMicrosoftNETSdkWeb = nameof(UsingMicrosoftNETSdkWeb);
	public const string ProjectTypeGuids = nameof(ProjectTypeGuids);
	public const string InvariantGlobalization = nameof(InvariantGlobalization);
	public const string PlatformNeutralAssembly = nameof(PlatformNeutralAssembly);
	public const string EnforceExtendedAnalyzerRules = nameof(EnforceExtendedAnalyzerRules);
	}