Use right hand side references even when not compatible

[ericstj]

Fixes #46834
Related to #35214

Not using references at all creates a problem when comparing against API that requires a core-assembly for resolving syntax (EG: Nullable<> -> ?)

Whenever a package is replacing a more specific assembly with a less specific assembly it still needs to be compatible with the less specific references.

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

Using rightContentItems[0] without checking if the list is non-empty may lead to an exception when rightContentItems is empty. Consider adding a check to ensure that rightContentItems contains at least one element before accessing its first element.

Suggested change

	if (fallbackAssemblyReferences is null && rightContentItems[0].Properties.TryGetValue("tfm", out tfmObj) && tfmObj is NuGetFramework rightTargetFramework)
	if (fallbackAssemblyReferences is null && rightContentItems.Count > 0 && rightContentItems[0].Properties.TryGetValue("tfm", out tfmObj) && tfmObj is NuGetFramework rightTargetFramework)

[carlossanlop]

Looks like Copilot is unaware of nullability checks. We already have a Debug.Assert(rightContentItems.Count > 0); in line 26.

[ericstj]

We might want to emit a diagnostic here. So long as all the types that were in the more-specific framework are in the less specific framework and are forwarded from the assembly name (excluding version) roslyn will still load them.

[carlossanlop]

What do you mean by "less-specific" and "more-specific"?

[ViktorHofer]

I remember discussing this with Santi and later Akhil. Back then we were not convinced that using the current assemblies for the baseline is the right thing to do. We have this comment in the AssemblySymbolLoader which talks about checking assembly versions during the load:

sdk/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs

Lines 461 to 463 in 2ae52ef

	// TODO: add version check and add a warning if it doesn't match?
	using FileStream resolvedStream = File.OpenRead(potentialPath);
	CreateAndAddReferenceToCompilation(assemblyReferenceName, rootAssemblyDisplayString, resolvedStream, referenceAssemblyNamesToIgnore, assemblyName);

I assume that this wouldn't work then anymore?
Can you imagine a case in which this wouldn't work and might result in incorrect execution or even an exception?

[ericstj]

Yeah if we introduced a check on assembly versions of dependencies it wouldn't work, because left in this case references System.Runtime 8.x.x.x and right has System.Runtime 4.x.x.x.

If someone actually tried to take the assembly referencing S.R 8 and run it in an environment with only S.R 4 it would of course result in exceptions. That's not what we're trying to check for here though - we're trying to say that if we had a project targeting net8 and upgraded to the new package which only targets netstandard2.0, is the surface area exposed by net8 assembly the same as the that now exposed by the netstandard2.0 assembly. We just don't have the net8.0 references to use to evaluate the difference.

I think for certain it's more correct to resolve references against the exact framework targeted. I tend to think it's also more correct to run with some references rather than none at all.