Analyzers should build against Microsoft.CodeAnalysis matching SDK

[sbomer]

In #123503 we encountered a problem caused by us shipping an analyzer that referenced a removed preview Roslyn API (see #123509 for details). The analyzer was built against an older version of Microsoft.CodeAnalysis that still had the API.

Some details on the current setup:

The older version in this case comes from MicrosoftCodeAnalysisVersion_LatestVS:

runtime/eng/Versions.props

Lines 70 to 77 in 52bc52c

	The exact version is always a moving target. This version should never go ahead of the version of Roslyn that is included in the most recent
	public Visual Studio preview version. If it were to go ahead, then any components depending on this version would not work in Visual Studio
	and would cause a major regression for any local development that depends on those components contributing to the build.
	This version must also not go ahead of the most recently release .NET SDK version, as that would break the source-build build.
	Source-build builds the product with the most recent previously source-built release. Thankfully, these two requirements line up nicely
	such that any version that satisfies the VS version requirement will also satisfy the .NET SDK version requirement because of how we ship.
	-->
	<MicrosoftCodeAnalysisVersion_LatestVS>4.14.0</MicrosoftCodeAnalysisVersion_LatestVS>

Analyzers typically reference either the dependency-flow version (in source-only build) or this LatestVS version otherwise:

runtime/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj

Lines 45 to 46 in 52bc52c

	<PackageReference Condition="'$(DotNetBuildSourceOnly)' != 'true'" Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion_LatestVS)" PrivateAssets="all" />
	<PackageReference Condition="'$(DotNetBuildSourceOnly)' == 'true'" Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion)" PrivateAssets="all" />

To prevent this, we should ensure that the MS product build references a recent version compatible with what we ship in the SDK. Additional constraints we try to keep, as noted above:

• The version should be <= the Roslyn from the latest VS to enable local dev using VS
• The version should be <= the latest SDK preview release to enable local dev using that SDK

It sounds like it's safe to use the dependency-flow version of Microsoft.CodeAnalysis in our release/servicing branches. What's a good way to stabilize the dependency versions? Should we use a policy similar to what we do for the compiler version (always use toolset version when building in VMR)?:
https://github.com/dotnet/dotnet/blob/867522e22452ccfb35203fc1dcf6b8a1a2486ba2/repo-projects/Directory.Build.targets#L26-L27

Requirements for the fix:

• Shipping (release/servicing) analyzers should reference a version of Microsoft.CodeAnalysis that is source-compatible and binary-compatible with the version included in that SDK's Roslyn.
• This version should be kept up-to-date automatically, or at least validated automatically.

@jkoritzinsky @ViktorHofer @jjonescz

[sbomer]

@jjonescz it looks like there was some mechanism that let source-build reference an older Roslyn, is this related? dotnet/dotnet@08ce30f#diff-71ef6732f45a1a305c26cbd854f053bccb0b2b5132e27386006789a9b190001fR4-R20

Is there a mechanism that ensures coherency in the VMR (so that analyzers reference the same Microsoft.CodeAnalysis that ships with the SDK) or do we just rely on VMR doing a GA build using P2 bits to validate compatibility?

[jjonescz]

dotnet/dotnet@08ce30f#diff-71ef6732f45a1a305c26cbd854f053bccb0b2b5132e27386006789a9b190001fR4-R20

GitHub UI for diffs isn't reliable, and I can't see what this link points to, consider linking the code directly instead.

Is there a mechanism that ensures coherency in the VMR (so that analyzers reference the same Microsoft.CodeAnalysis that ships with the SDK) or do we just rely on VMR doing a GA build using P2 bits to validate compatibility?

I don't know how this works, TBH, my guess is it's the latter.

[sbomer]

@jjonescz here's the bit I was trying to link, it looks like one of the dependency updates introduced a separate version of the Roslyn dependency for analyzers:

https://github.com/dotnet/dotnet/blob/08ce30f5870a413c655ffd78cd3063b1a948514f/src/roslyn/eng/Version.Details.xml#L4-L7

I don't have context on that, but was wondering if dependency flow tooling or the area owners had some prior art for dealing with analyzers requiring older Roslyn versions.

[jjonescz]

The exact commit you linked to was when we referenced a fixed old Roslyn as a result of merging roslyn-analyzers to roslyn.

Now we reference some new version which is auto updated by maestro (I think it's just the previously built version):

https://github.com/dotnet/dotnet/blob/45d1ed746b88c992e7bd1a2ebe2b6e6bd531d3be/src/roslyn/eng/Version.Details.xml#L5-L9

[sbomer]

The LatestVS property was introduced so that the latest VS/SDK preview can be used for local dev. But local dev with VS/SDK previews can break when a repo adopts a new language feature that hasn't been released in a preview.

I wonder if we should adopt a similar policy for analyzers/generators. That is, always build against the latest (dependency flow version) Roslyn APIs. This might require updating the global.json SDK to run these analyzers (same as it would to consume a new language feature).

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

@sbomer looks like the SDK has the matching Roslyn version set in the _NetFrameworkHostedCompilersVersion property, but that's definitely not meant for this usage. It might be worth introducing a new property in the SDK's BundledVersions.props to handle this scenario.