Project with overloaded TargetFrameworks property fails to load

[ViktorHofer]

Visual Studio Version: Version 16.7.0 Preview 1.0 [30022.13.master]

Summary:
When opening https://github.com/dotnet/runtime/blob/master/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj in VS (16.7 P1) I get the following error:

5/18/2020 7:36:02 PM
Recoverable
System.AggregateException: Project system data flow 'DataflowBlockSlim (ActionBlockSlimAsync`1 : 29763181)' closed because of an exception: System.AggregateException: One or more errors occurred. ---> System.ArgumentException: Must contain activeTargetFramework (Unsupported,Version=v0.0).
Parameter name: dependenciesByTargetFramework
   at Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.DependenciesSnapshot..ctor(String projectPath, ITargetFramework activeTargetFramework, ImmutableDictionary`2 dependenciesByTargetFramework)
   at Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.DependenciesSnapshot.SetTargets(ImmutableArray`1 targetFrameworks, ITargetFramework activeTargetFramework)
   at Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Subscriptions.DependenciesSnapshotProvider.<>c__DisplayClass38_0.<UpdateProjectContextAndSubscriptionsAsync>b__2(DependenciesSnapshot previousSnapshot)
   at Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Subscriptions.DependenciesSnapshotProvider.SnapshotUpdater.TryUpdate(Func`2 updateFunc, CancellationToken token)
   at Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Subscriptions.DependenciesSnapshotProvider.<<UpdateProjectContextAndSubscriptionsAsync>b__38_0>d.MoveNext()

VsProjectFault_0d8d4bb6-1033-44ff-8927-b8559ebf600c.failure.txt

Steps to Reproduce:

1. Clone https://github.com/dotnet/runtime

2. Build: .\build.cmd clr.corelib+libs

3. Open src/libraries/System.Private.Xml/src/System.Private.Xml.csproj

Expected Behavior:
No errors.

Actual Behavior:

User Impact:
Project fails to load, VS is unusable.

cc @davkean

[ViktorHofer]

Possibly related to #6203? cc @drewnoakes

[drewnoakes]

@VictorHofer similar code, but this is actually a dupe of #6204 which was fixed in #6205.

I really appreciate this issue though as we only saw the error in telemetry and didn't have a repro. I can use this to validate the fix 👍

[ViktorHofer]

Sure. IIRC, the TargetFramework/s properties are "user-facing" ones which should never be validated. The sdk team is considering allowing any random input to make multi-targeting easier. cc @dsplaisted @ericstj

[dsplaisted]

Yes, in most cases the project system (and other components) should not read the TargetFramework directly, but should instead use the component properties: TargetFrameworkIdentifier, TargetFrameworkVersion, TargetFrameworkProfile, TargetPlatformIdentifier, and TargetPlatformVersion (the last two are new for .NET 5 and not yet implemented).

There will probably be a lot of onion peeling to fix this entirely.

See NuGet/Home#5154 and https://github.com/dotnet/project-system/blob/master/docs/repo/coding-conventions.md#data for more context.

[drewnoakes]

We are tracking proper handling of implicit configurations (i.e. dimensions such as target framework) in #6183 which I believe will help here by leaning on CPS's logic for handling such dimensions rather than duplicating it (incorrectly) in the dependencies node.