Update/fix tfm selection with platform specific tfms

CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
+* Actually selected the most optimal fx for multiple projects when loading a solution:
+  - By @razzmatazz inhttps://github.com/razzmatazz/csharp-language-server/pull/286
+  - Reported by @badly-drawn-wizards in https://github.com/razzmatazz/csharp-language-server/issues/280
 * Actually ingest InitializeParams.rootPath and .rootUri to determine workspace root:
   - By @razzmatazz in https://github.com/razzmatazz/csharp-language-server/pull/283
 * Add `--diagnose` command option to run basic diagnostics interactively

Directory.Packages.props

@@ -34,8 +34,9 @@
     <PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="9.0.9" />
     <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
     <PackageVersion Include="Newtonsoft.Json" Version="13.0.4" />
+    <PackageVersion Include="NuGet.Frameworks" Version="6.14.0" />
     <PackageVersion Include="NUnit" Version="3.14.0" />
     <PackageVersion Include="NUnit3TestAdapter" Version="4.6.0" />
     <PackageVersion Include="System.Collections.Immutable" Version="9.0.0" />
   </ItemGroup>
-</Project>
+</Project>

src/CSharpLanguageServer/CSharpLanguageServer.fsproj

@@ -104,5 +104,6 @@
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
     <PackageReference Include="Microsoft.CodeAnalysis.Features" />
     <PackageReference Include="Microsoft.CodeAnalysis.Workspaces.MSBuild" />
+    <PackageReference Include="NuGet.Frameworks" />
   </ItemGroup>
 </Project>

src/CSharpLanguageServer/Roslyn/Solution.fs

@@ -15,17 +15,14 @@ open Microsoft.CodeAnalysis
 open Microsoft.CodeAnalysis.MSBuild
 open Microsoft.CodeAnalysis.Text
 open Microsoft.Extensions.Logging
+open NuGet.Frameworks
 
-open CSharpLanguageServer
 open CSharpLanguageServer.Lsp
 open CSharpLanguageServer.Logging
-open CSharpLanguageServer.Util
 open CSharpLanguageServer.Roslyn.WorkspaceServices
 
-
 let private logger = Logging.getLoggerByName "Roslyn.Solution"
 
-
 let initializeMSBuild () : unit =
     let vsInstanceQueryOpt = VisualStudioInstanceQueryOptions.Default
     let vsInstanceList = MSBuildLocator.QueryVisualStudioInstances(vsInstanceQueryOpt)
@@ -61,7 +58,6 @@ let initializeMSBuild () : unit =
 
     MSBuildLocator.RegisterInstance(vsInstance)
 
-
 let solutionLoadProjectFilenames (solutionPath: string) =
     assert Path.IsPathRooted solutionPath
     let projectFilenames = new List<string>()
@@ -75,49 +71,6 @@ let solutionLoadProjectFilenames (solutionPath: string) =
     projectFilenames |> Set.ofSeq
 
 
-type TfmCategory =
-    | NetFramework of Version
-    | NetStandard of Version
-    | NetCoreApp of Version
-    | Net of Version
-    | Unknown
-
-
-let selectLatestTfm (tfms: string seq) : string option =
-    let parseTfm (tfm: string) : TfmCategory =
-        let patterns =
-            [ @"^net(?<major>\d)(?<minor>\d)?(?<build>\d)?$", NetFramework
-              @"^netstandard(?<major>\d+)\.(?<minor>\d+)$", NetStandard
-              @"^netcoreapp(?<major>\d+)\.(?<minor>\d+)$", NetCoreApp
-              @"^net(?<major>\d+)\.(?<minor>\d+)$", Net ]
-
-        let matchingTfmCategory (pat, categoryCtor) =
-            let m = Regex.Match(tfm.ToLowerInvariant(), pat)
-
-            if m.Success then
-                let readVersionNum (groupName: string) =
-                    let group = m.Groups.[groupName]
-                    if group.Success then int group.Value else 0
-
-                Version(readVersionNum "major", readVersionNum "minor", readVersionNum "build")
-                |> categoryCtor
-                |> Some
-            else
-                None
-
-        patterns |> List.tryPick matchingTfmCategory |> Option.defaultValue Unknown
-
-    let rankTfm =
-        function
-        | Net v -> 3000 + v.Major * 10 + v.Minor
-        | NetCoreApp v -> 2000 + v.Major * 10 + v.Minor
-        | NetStandard v -> 1000 + v.Major * 10 + v.Minor
-        | NetFramework v -> 0 + v.Major * 10 + v.Minor
-        | Unknown -> -1
-
-    tfms |> Seq.sortByDescending (parseTfm >> rankTfm) |> Seq.tryHead
-
-
 let loadProjectTfms (projs: string seq) : Map<string, list<string>> =
     let mutable projectTfms = Map.empty
 
@@ -156,6 +109,38 @@ let loadProjectTfms (projs: string seq) : Map<string, list<string>> =
 
     projectTfms
 
+let compatibleTfmsOfTwoSets afxs bfxs = seq {
+    for a in afxs |> Seq.map NuGetFramework.Parse do
+        for b in bfxs |> Seq.map NuGetFramework.Parse do
+            if DefaultCompatibilityProvider.Instance.IsCompatible(a, b) then
+                yield a.GetShortFolderName()
+            else if DefaultCompatibilityProvider.Instance.IsCompatible(b, a) then
+                yield b.GetShortFolderName()
+}
+
+let compatibleTfmSet (fxSets: list<Set<string>>) : Set<string> =
+    match fxSets.Length with
+    | 0 -> Set.empty
+    | 1 -> fxSets |> List.head
+    | _ ->
+        let firstSet = fxSets |> List.head
+
+        fxSets |> List.skip 1 |> Seq.fold compatibleTfmsOfTwoSets firstSet |> Set.ofSeq
+
+let bestTfm (frameworks: string seq) : string option =
+    let frameworks = frameworks |> Seq.map NuGetFramework.Parse |> List.ofSeq
+
+    match frameworks with
+    | [] -> None
+    | fxes ->
+        let compatibleWithAllOtherFxes candidate =
+            fxes
+            |> Seq.forall (fun f -> DefaultCompatibilityProvider.Instance.IsCompatible(candidate, f))
+
+        fxes
+        |> Seq.maxBy (fun fx -> compatibleWithAllOtherFxes fx, fx.HasPlatform, fx.Version)
+        |> _.GetShortFolderName()
+        |> Some
 
 let applyWorkspaceTargetFrameworkProp (tfmsPerProject: Map<string, list<string>>) props : Map<string, string> =
     let selectedTfm =
@@ -164,14 +149,14 @@ let applyWorkspaceTargetFrameworkProp (tfmsPerProject: Map<string, list<string>>
         | _ ->
             tfmsPerProject.Values
             |> Seq.map Set.ofSeq
-            |> Set.intersectMany
-            |> selectLatestTfm
+            |> List.ofSeq
+            |> compatibleTfmSet
+            |> bestTfm
 
     match selectedTfm with
     | Some tfm -> props |> Map.add "TargetFramework" tfm
     | None -> props
 
-
 let resolveDefaultWorkspaceProps projs : Map<string, string> =
     let tfmsPerProject = loadProjectTfms projs
 
@@ -188,7 +173,6 @@ let solutionGetProjectForPath (solution: Solution) (filePath: string) : Project
 
     solution.Projects |> Seq.filter fileOnProjectDir |> Seq.tryHead
 
-
 let solutionTryAddDocument (docFilePath: string) (text: string) (solution: Solution) : Async<Document option> = async {
     let projectOnPath = solutionGetProjectForPath solution docFilePath
 
@@ -211,7 +195,6 @@ let solutionTryAddDocument (docFilePath: string) (text: string) (solution: Solut
     return newDocumentMaybe
 }
 
-
 let selectPreferredSolution (slnFiles: string list) : option<string> =
     let getProjectCount (slnPath: string) =
         try
@@ -229,7 +212,6 @@ let selectPreferredSolution (slnFiles: string list) : option<string> =
         |> Seq.map snd
         |> Seq.tryHead
 
-
 let solutionTryLoadOnPath (lspClient: ILspClient) (solutionPath: string) =
     assert Path.IsPathRooted solutionPath
     let progress = ProgressReporter lspClient
@@ -282,7 +264,6 @@ let solutionTryLoadOnPath (lspClient: ILspClient) (solutionPath: string) =
             return None
     }
 
-
 let solutionTryLoadFromProjectFiles (lspClient: ILspClient) (logMessage: string -> Async<unit>) (projs: string list) =
     let progress = ProgressReporter lspClient
 
@@ -324,7 +305,6 @@ let solutionTryLoadFromProjectFiles (lspClient: ILspClient) (logMessage: string
         return Some msbuildWorkspace.CurrentSolution
     }
 
-
 let solutionFindAndLoadOnDir (lspClient: ILspClient) dir = async {
     let fileNotOnNodeModules (filename: string) =
         filename.Split Path.DirectorySeparatorChar |> Seq.contains "node_modules" |> not
@@ -374,7 +354,6 @@ let solutionFindAndLoadOnDir (lspClient: ILspClient) dir = async {
     | Some solutionPath -> return! solutionTryLoadOnPath lspClient solutionPath
 }
 
-
 let solutionLoadSolutionWithPathOrOnCwd (lspClient: ILspClient) (solutionPathMaybe: string option) (cwd: string) =
     match solutionPathMaybe with
     | Some solutionPath -> async {

tests/CSharpLanguageServer.Tests/InternalTests.fs

@@ -5,17 +5,19 @@ open NUnit.Framework
 
 open CSharpLanguageServer.Roslyn.Solution
 
-
 [<TestCase("1.csproj:net8.0", "net8.0")>]
 [<TestCase("1.csproj:net8.0,net10.0", "net10.0")>]
 [<TestCase("1.csproj:net8.0,netstandard2.0", "net8.0")>]
 [<TestCase("1.csproj:netstandard1.0,netstandard2.0", "netstandard2.0")>]
 [<TestCase("1.csproj:net40,net462,net6.0,net8.0,netcoreapp3.1,netstandard2.0", "net8.0")>]
+[<TestCase("1.csproj:net40,net462,net6.0,net8.0,net8.0-windows,netcoreapp3.1,netstandard2.0", "net8.0-windows")>]
 [<TestCase("1.csproj:net40,net462", "net462")>]
 [<TestCase("1.csproj:net8.0 2.csproj:net8.0", "net8.0")>]
-[<TestCase("1.csproj:net8.0,net10.0 2.csproj:netstandard2.0,net462", null)>]
+[<TestCase("1.csproj:net8.0,net10.0 2.csproj:netstandard2.0,net462", "net10.0")>]
 [<TestCase("1.csproj:net8.0,net10.0 2.csproj:net8.0,net10.0", "net10.0")>]
-[<TestCase("1.csproj:net8.0 2.csproj:net8.0,net10.0", "net8.0")>]
+[<TestCase("1.csproj:net8.0 2.csproj:net8.0,net10.0", "net10.0")>]
+[<TestCase("1.csproj:net8.0 2.csproj:net9.0-windows", "net9.0-windows")>]
+[<TestCase("1.csproj:net9.0 2.csproj:net9.0-windows", "net9.0-windows")>]
 let testApplyWorkspaceTargetFrameworkProp (tfmList: string, expectedTfm: string | null) =
 
     let parseTfmList (projectEntry: string) : string * list<string> =
@@ -38,7 +40,6 @@ let testApplyWorkspaceTargetFrameworkProp (tfmList: string, expectedTfm: string
 
     Assert.AreEqual(expectedTfm |> Option.ofObj, props |> Map.tryFind "TargetFramework")
 
-
 [<TestCase>]
 let testApplyWorkspaceTargetFrameworkPropWithEmptyMap () =
     let props = Map.empty |> applyWorkspaceTargetFrameworkProp Map.empty