Support .NET Core 3 projects

[adamnew123456]

This covers the issues with framework assembly detection in netcoreapp3.0 that came up in #66. The other changes are:

• New NetCore3 test and fixes found by failing tests
• Update VS Code build files to handle new netcoreapp3.0 binary paths

[georgewfraser]

Thanks for doing this! When I build this, install locally, and open ProjectCracker.fs, I get a giant red error:

The type 'ValueType' is required here and is unavailable. You must add a reference to assembly 'netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'.(1108: typecheck) The type referenced through 'System.Object' is defined in an assembly that is not referenced. You must add a reference to assembly 'netstandard'.(74: typecheck)

Have you tried installing this locally with ./scripts/build.sh and verified that you can open files and the language server works?

[adamnew123456]

@georgewfraser I did see that error at one point, and I believe it comes from a failure to find the matching runtime assemblies. Did you install .NET Core 3.1 or .NET Core 3.0?

Right now the version of the runtime we grab assemblies from is derived from the framework reference within project.assets.json (".NETCoreApp,Verison=v3.0" for "netcoreapp3.0"). Both the minor and major version has to match for the project cracker to consider it; otherwise it's ignored. You should see in the stderr of the server that it finds a Microsoft.NETCore.App 3.1.x but rejects it:

dotnet/
  shared/
    Microsoft.NETCore.App/
      3.1.x  # Rejected because 3.0 does not match 3.1.x

I'm not sure of a good way to fix this outside of saying that the runtime you have installed has to be >= the version of the framework you target. That could cause problems though if you develop under .NET Core 3.1 and get completions for 3.1-only classes or members which you can't actually use in your netcoreapp3.0 project.

There are also a bunch of MSBuild files we could parse to get a (possibly) more exact answer, but I don't want to go down that road if we don't have to. The ones included in the framework look pretty hairy in terms of build-time variables and embedded conditional logic.

[georgewfraser]

I installed 3.1.

[adamnew123456]

@georgewfraser https://github.com/fsprojects/fsharp-language-server/pull/70/commits/e52e731ac985bfbce73cbf379b35f316e8a8538a should cover this. Apparently there is backwards compatibility within a major version, so if you have .NET Core 3.1 then you get the following:

• netcoreapp3.1 will work (exact match)
• netcoreapp3.0 will work (major version match, minor framework version >= minor target version)
• netcoreapp2.0 will not work (major version mismatch)
• netcoreapp3.2 will not work (minor framework version not >= minor target version)

[georgewfraser]

OK, I'm no longer getting giant errors, but now I'm getting incorrect autocomplete info:

[adamnew123456]

It looks like there's an error message here we're not showing. For reference, you can get the full error listed in the completion window by updating asCompletionItem. It might be useful to have as a separate change later if there's a better way to report these:

/// Convert an F# `FSharpDeclarationListItem` to an LSP `CompletionItem`
let private asCompletionItem(i: FSharpDeclarationListItem): CompletionItem =
    if i.IsResolved then
        { defaultCompletionItem with 
            label = i.Name 
            insertText = Some(i.NameInCode)
            kind = Some(asCompletionItemKind(i.Glyph))
            detail = Some(i.FullName)
            // Stash FullName in data so we can use it later in ResolveCompletionItem
            data = JsonValue.Record [|"FullName", JsonValue.String(i.FullName)|]
        }
    else
        let (FSharpToolTipText [FSharpStructuredToolTipElement.CompositionError message]) =
            i.StructuredDescriptionText

        { defaultCompletionItem with 
            label = message
            insertText = Some(i.NameInCode)
            kind = Some(asCompletionItemKind(i.Glyph))
            detail = Some(i.FullName)
            // Stash FullName in data so we can use it later in ResolveCompletionItem
            data = JsonValue.Record [|"FullName", JsonValue.String(i.FullName)|]
        }

If I do a completion on types that come from the base framework, it gives this error message. Apparently the runtimes include some assemblies that can't be loaded outside of Windows. I'll have to look at one of the alternative assembly locations, maybe some of them are more tailored to the current runtime.

[adamnew123456]

@georgewfraser can you try it with https://github.com/fsprojects/fsharp-language-server/pull/70/commits/864a6d95df3228d25c7a42a12b0f70a9d870e4cd? The cracker prefers using pack assemblies to runtime assemblies now, since packs are specific to each platform and only include assemblies which can be fully loaded.

[georgewfraser]

OK, it's working for me now. I'd like to understand what has been changed in ProjectCracker.fs a little bit better before I merge. I'm also a little concerned that it took so many fixes just to get it working on both your machine and my machine---if there's any way we can make ProjectCracker.fs less brittle, we should do it.

[adamnew123456]

OK, it's working for me now. I'd like to understand what has been changed in ProjectCracker.fs a little bit better before I merge.

Sure. Basically, the problem these changes are trying to solve is that starting with .NET Core 3, references to the underlying framework are implied rather than written out explicitly like normal packages. The spec has the full details, but the main point is that this breaks the assumption that the framework assemblies are listed with the normal package dependencies. That's why you get assembly load failures if you just s/netcoreapp2.0/netcoreapp3.0/ within the project files and then open up a netcoreapp3.0 project in FSLS.

Instead, we get the frameworks via the new frameworkReferences section. However, these don't include full lists of assemblies so we have to hunt down the framework DLLs in the filesystem.

I borrowed the method for doing this from Buildalyzer, which also parses the output of dotnet --info to determine information about the full list of available runtimes. I don't think there's a way to this in .NET directly, at least if you want to consider all frameworks and not just the one FSLS is running on (which would break things like ASP.NET Core which we don't use).

If FSLS can find one that matches the active project's target framework then it can use that framework's assemblies directly. You can do that either via targeting packs, which are meant for these kinds of dev scenairos, or the runtime directly which includes extra assemblies. Doing path manipulation to get these is kind of weird, but the location is stable and mentioned in the spec.

I'm also a little concerned that it took so many fixes just to get it working on both your machine and my machine---if there's any way we can make ProjectCracker.fs less brittle, we should do it.

That was more a failure of testing on my part. It was equally broken for me too but most of what I work with leans on FSharp.Core more than the underlying framework, so the failures didn't show up.

After some further testing with my clean VM, I think there's only one potential location for these assemblies that needs to be added. Everything's covered within the .NET Core root directory since we can find both the runtimes and packs:

$ cd '/c/Program Files/dotnet'
$ /bin/find -name 'System.Core.dll'
./packs/Microsoft.NETCore.App.Ref/3.1.0/ref/netcoreapp3.1/System.Core.dll
./packs/NETStandard.Library.Ref/2.1.0/ref/netstandard2.1/System.Core.dll
./shared/Microsoft.NETCore.App/3.1.0/System.Core.dll

However, if I build with an empty netcoreapp3.0 project then there is an entry for Microsoft.NETCore.App.Ref 3.0.0 within ~/.nuget/packages. I can add this bit to the framework finder so that we can get use it as well.

$ cd ~/.nuget/packages
$ /bin/find -name 'System.Core.dll'
./microsoft.netcore.app.ref/3.0.0/ref/netcoreapp3.0/System.Core.dll

[adamnew123456]

I pushed in support for discovering the NuGet runtime packs and just fixed the merge conflict.

For reference, this is what it finds on a machine with .NET Core 2.2 and .NET Core 3.1 installed when working on a netcoreapp3.0 project. You can see it considering the full set of available frameworks, bundled runtime packs and NuGet runtime packs.

In this instance, it picks the NuGet one and you get working completion and hover for the core framework classes:

Discovered framework runtime: Microsoft.AspNetCore.All v2.2.8 at C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All
Discovered framework runtime: Microsoft.AspNetCore.App v2.2.8 at C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App
Discovered framework pack: Microsoft.AspNetCore.App v3.1.0 at C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\3.1.0\ref\netcoreapp3.1
Discovered framework runtime: Microsoft.NETCore.App v2.2.8 at C:\Program Files\dotnet\shared\Microsoft.NETCore.App
Discovered framework pack: Microsoft.NETCore.App v3.1.0 at C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\3.1.0\ref\netcoreapp3.1
Discovered framework pack: Microsoft.WindowsDesktop.App v3.1.0 at C:\Program Files\dotnet\packs\Microsoft.WindowsDesktop.App.Ref\3.1.0\ref\netcoreapp3.1
Chose C:\Users\chris\.nuget\packages\microsoft.netcore.app.ref\3.0.0\ref\netcoreapp3.0 as the final path for runtime Microsoft.NETCore.App version (3, 0)

[PhilT]

@georgewfraser, @adamnew123456 if you're thinking about making changes to the Project Cracker you may want to consider an alternative https://www.nuget.org/packages/Dotnet.ProjInfo.Workspace.FCS/ .

Related info here: https://github.com/fsharp/FSharp.Compiler.Service/issues/905

[adamnew123456]

@PhilT Thanks, that's a good find! It's curious that FSAC still has some custom assembly finding code after integrating that. I wonder what they use it for if ProjInfo can resolve the framework references?

@georgewfraser Given that this would involve dropping the built-in ProjectCracker in favor of ProjInfo, it would make more sense to do this separately. If you're interested I'll try to find some time soon to rewrite the relevant bits of ProjectManager and see how ProjInfo does compared to the dotnet-3 ProjectCracker. We can kill one PR in favor of the other depending upon how it does.

[baronfel]

I wonder what they use it for if ProjInfo can resolve the framework references?

We use that custom code because projinfo a few versions back was having issues generating the entire necessary assembly set for F# scripts only in a .net core context. We dropped it for that use case and started using the raw FCS apis for deriving project options for scripts, and then realized that those APIs only get the set of reference assemblies for the currently executing version of .net core, and so I wrote the FSAC code to allow us to run on .netcoreapp2.1 but find reference assemblies for any .net core SDK version we want to.

We still use ProjInfo for project-files, though.

[adamnew123456]

@baronfel

[PhilT]

Is it worth me giving this a test or should I wait for the final test to be fixed before giving it a go?

[adamnew123456]

@PhilT It looks like an intermittent issue, I wouldn't worry about it. It seems to have failed because it couldn't read an assembly, but that shouldn't be a fatal error (I've seen it locally and it's never killed the LSP server). It hasn't been a problem in other builds.

[PhilT]

Thanks @adamnew123456. I've just cloned and built this PR now and it's working really well for me on Neovim (Windows) with .NET Core 3.0 and FZF. Errors/linting, autocomplete, rename, goto definition and references all seem to be working.

I'll probably stick with this setup for the next few days as I have a feeling its a bit better than my current setup of using Coc-nvim and fsharp plugin. I'll report back any issues.

Seems really solid though, thanks.

[PhilT]

I'm seeing cases where I'll change some code and a bunch of gutter errors will appear. I'll then correct the code but the gutter is not clearing correctly. A LanguageClientStop then LanguageClientStart does not fix anything. I've just started using .NET Core 3.1 so maybe that is the issue but I think I saw the same issue on 3.0. Sorry to be a bit vague.

[adamnew123456]

@PhilT I've seen something similar, but it usually ends up bound to some internal limit in the default lsp-mode configuration (after a point it disables checks if too many errors get reported). If you have any way to reproduce it, can you see what's getting logged in the server's stderr around the time of the hang?

It should say something like this after each edit if it's successful. I think it's possible for that process to timeout, although I've never seen it kill the server outright.

Checked Backend.fs in 378ms

[adamnew123456]

@PhilT Also, could you try checking out/cherrypicking the change I made in https://github.com/adamnew123456/fsharp-language-server/commit/c9132dfe82a0e17bae434a63d9fda4e26cfd43f1 to see if it helps?

I haven't tried it with coc-nvim, but lsp-mode has a tendency to get wedged if it expects a response from the server but it neglects to send one. The TODO() implementations are the most common cause of this that I've seen, which that commit changes to return a default empty value.

[georgewfraser]

Thanks very much! I'll publish a new minor version shortly.