ionide / FsAutoComplete

F# language server using Language Server Protocol
Other
416 stars 154 forks source link

fsi variable is unavailable when parsing script files #227

Closed drvink closed 5 years ago

drvink commented 7 years ago

With FSAC 0.34.0 and F# 4.1.18 on Linux:

arch-vm[1353]:~% mono ~/.emacs.d/elpa/fsharp-mode-20170918.842/bin/fsautocomplete.exe -v
{"Kind":"info","Data":"Started (PID=109693)"}
parse "FsiBug.fsx"
do printfn "%A" fsi
<<EOF>>
{"Kind":"info","Data":"Background parsing started"}
{"Kind":"errors","Data":{"File":"/home/mdl/FsiBug.fsx","Errors":[{"FileName":"/home/mdl/FsiBug.fsx","StartLine":1,"EndLine":1,"StartColumn":17,"EndColumn":20,"Severity":"Error","Message":"The value or constructor 'fsi' is not defined. Maybe you want one of the following:\n   fst","Subcategory":"typecheck"}]}}
parse "FsiBug2.fsx"
#r @"/usr/lib/mono/fsharp/FSharp.Compiler.Interactive.Settings.dll"
do printfn "%A" fsi
<<EOF>>
{"Kind":"info","Data":"Background parsing started"}
{"Kind":"errors","Data":{"File":"/home/mdl/FsiBug2.fsx","Errors":[]}}

Notice that it works if you explicitly reference FSharp.Compiler.Interactive.Settings.dll--but you should not have to do this. With the same version of FSAC running on the CLR on Windows, FsiBug.fsx parses fine, which is the correct/expected behavior.

Mono version info, just in case it's useful:

arch-vm[1002]:~% mono --version
Mono JIT compiler version 5.7.0 (master/5825342717c Mon Oct  2 17:12:43 JST 2017)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
        TLS:           __thread
        SIGSEGV:       normal
        Notifications: epoll
        Architecture:  amd64
        Disabled:      none
        Misc:          softdebug
        LLVM:          supported, not enabled.
        GC:            sgen (concurrent by default)
drvink commented 7 years ago

Pinging @dsyme since this involves some hairy stuff that isn't limited to FSAC, and I'm not sure what the best course of action is.

I've tracked this down to (as usual) Mono vs. the CLR, combined with FCS's procedures for determining the "default F# binary path", which ultimately results in FSharp.Compiler.Interactive.Settings.dll not being found on Mono.

The problem code is FSharpEnvironment.BinFolderOfDefaultFSharpCompiler:

    let BinFolderOfDefaultFSharpCompiler(probePoint:string option) = 
#if FX_NO_WIN_REGISTRY
        ignore probePoint
#if FX_NO_APP_DOMAINS
        Some System.AppContext.BaseDirectory
#else
        Some System.AppDomain.CurrentDomain.BaseDirectory // <- OUR PROBLEM
#endif
#else
    // a zillion other ways to try to find the bin path

Since the assembly we need doesn't get bundled by FSAC (i.e. is not in the same directory as fsautocomplete.exe), unless the assembly happens to be in Mono's search path (and using MONO_PATH in the environment doesn't help), it won't get found, and our bug occurs. Here's how it works on Windows/the CLR (continuing from the previous fragment):

        // Check for an app.config setting to redirect the default compiler location
        // Like fsharp-compiler-location
        try 
            // FSharp.Compiler support setting an appkey for compiler location. I've never seen this used.
            let result = tryAppConfig "fsharp-compiler-location"
            match result with 
            | Some _ ->  result 
            | None -> 

            let safeExists f = (try File.Exists(f) with _ -> false)
            // Look in the probePoint if given, e.g. look for a compiler alongside of FSharp.Build.dll
            match probePoint with 
            | Some p when safeExists (Path.Combine(p,"FSharp.Core.dll")) -> Some p 
| _ -> // continues to look, via the registry

Our .config doesn't have an entry for fsharp-compiler-location, so we proceed to the next search method, which tries to look for FSharp.Core.dll adjacent to a user-provided hint path. That hint path is:

let defaultFSharpBinariesDir = FSharpEnvironment.BinFolderOfDefaultFSharpCompiler(Some(typeof<FSharpCheckFileAnswer>.Assembly.Location)).Value

...which comes from defaultFSharpBinariesDir. However, this lookup will always fail, because the Location property of the assembly is the full path (including filename) of the assembly, so Path.Combine(aloc, "FSharp.Core.dll") produces something like C:\blah\blah\FSharp.Compiler.Service.dll\FSharp.Core.dll. So, the search continues, with the next method being a registry key lookup, and at this point we get the path of the actual F# installation (not the path of the FSAC-bundled FSharp.Compiler.Service.dll), which does contain FSharp.Compiler.Interactive.Settings.dll.

FSAC asks to parse the contents of a .fsx using GetProjectOptionsFromScript. Notice that it defaults to adding a reference to the assembly we want, which is why things happen to work on Windows despite FSAC not explicitly requesting to reference it--FCS has handled it for us. Unfortunately, as described above, FCS picks the wrong path on Mono, and the remainder of this god-forsaken process (follow the call into ComputeClosureOfSourceText if you wish to see how the binary path gets used) relies on it being correct if we are to get an fsi variable in the editing session for our .fsx script, but since it is not, we lose. (It should be obvious if you read through the remaining code why manually referencing the assembly from within the script succeeds as a workaround.)

I can think of a few options to fix this:

  1. We could try to locate FSharp.Compiler.Interactive.Settings.dll from FSAC somehow. This is trickier than one would hope, because with fsharp (i.e. fsharp/fsharp, not Microsoft/visualfsharp), generally fsharpi is in your PATH, but fsi.exe is buried in some Mono directory, so we can't just look for something next to fsharpi, and we don't know where fsi.exe might actually live, so there's a chicken-and-egg problem. In a truly stunning moment of irony, xbuild/msbuild would actually save the day here, because we could reliably assume their presence and simply ask for the value of $(FscToolPath).
  2. The above, except fix it in FCS instead.
  3. We could distribute FSharp.Compiler.Interactive.Service.dll with FSAC, assuming that wouldn't cause other problems or introduce ugly dependency issues.
enricosada commented 7 years ago

I am somewhat in favor of 1: asking msbuild, a bit slower but reliable. and can be cached for whole session. And pass it as additional ref for mono.

maybe because i prefer --simpleresolution instead of magic resolution from FCS.

I have a related issue trying to find the reference assemblies on .net core parsing an fsx, and i am really really tired of this. So i am doing that (and can be reused for getting $(FscToolPath) afaik)

FCS ihmo is a lower level, and shouldnt care about this stuff, just use the reference list passed

drvink commented 7 years ago

Do you mean having FSAC ask msbuild, or having FCS ask? There's already plenty of magic resolution in FCS (as outlined/ranted), but doing it there has the benefit of fixing it for FCS clients--though the value may be dubious, as it just adds yet more magic resolution stuff. I defer to you/others here, since you have a better picture of how this plays out for consumers of FCS besides just FSAC.

enricosada commented 7 years ago

Having FSAC ask.. too difficult change FCS for me atm. the way inside FCS is just guess based on directories and filenames. or try to emulate msbuild resolution (faulty). that;'s fine. and doesnt depends on having msbuild installed. but ihmo is a just an approssimation and doesnt always work.

I'd like to use msbuild instead, but dont want to add to FCS until is sure it works ok. so for now is in FSAC.

Is not the FSharp.Compiler.Interactive.Service.dll, but is same flow:

Instead of magic resolution of fsi references inside FCS, what i want to do is:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />

  <ItemGroup>
    <Reference Include="System" />
    <Reference Include="System.Xml" />
    <Reference Include="System.Runtime.Remoting" />
    <Reference Include="System.Runtime.Serialization.Formatters.Soap" />
    <Reference Include="System.Data" />
    <Reference Include="System.Drawing" />
    <Reference Include="System.Core" />
          <!-- // These are the Portable-profile and .NET Standard 1.6 dependencies of FSharp.Core.dll.  These are needed
          // when an F# sript references an F# profile 7, 78, 259 or .NET Standard 1.6 component which in turn refers 
          // to FSharp.Core for profile 7, 78, 259 or .NET Standard. -->
    <Reference Include="System.Runtime" /> <!-- // lots of types -->
    <Reference Include="System.Linq" /> <!-- // System.Linq.Expressions.Expression<T>  -->
    <Reference Include="System.Reflection" /> <!--  // System.Reflection.ParameterInfo -->
    <Reference Include="System.Linq.Expressions" /> <!--  // System.Linq.IQueryable<T> -->
    <Reference Include="System.Threading.Tasks" /> <!--  // valuetype [System.Threading.Tasks]System.Threading.CancellationToken -->
    <Reference Include="System.IO" />  <!--  //  System.IO.TextWriter -->
          <!-- //yield "System.Console"  //  System.Console.Out etc. -->
    <Reference Include="System.Net.Requests" />  <!--  //  System.Net.WebResponse etc. -->
    <Reference Include="System.Collections" /> <!--  // System.Collections.Generic.List<T> -->
    <Reference Include="System.Runtime.Numerics" /> <!--  // BigInteger -->
    <Reference Include="System.Threading" />  <!--  // OperationCanceledException -->
          <!-- // always include a default reference to System.ValueTuple.dll in scripts and out-of-project sources
          match GetDefaultSystemValueTupleReference() with 
          | None -> ()
          | Some v -> yield v -->

    <Reference Include="System.Web" />
    <Reference Include="System.Web.Services" />
    <Reference Include="System.Windows.Forms" />
    <Reference Include="System.Numerics" />

    <!-- "-r:C:\Windows\Microsoft.NET\assembly\GAC_MSIL\FSharp.Compiler.Interactive.Settings\v4.0_4.4.0.0__b03f5f7f11d50a3a\FSharp.Compiler.Interactive.Settings.dll"|]; -->

    <Reference Include="FSharp.Core" />
  </ItemGroup>

  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />

  <Target Name="_GetFsxScriptReferences" DependsOnTargets="ResolveAssemblyReferences">
    <Message Text="V: %(ReferencePath.Identity)" />

    <WriteLinesToFile
            Condition=" '$(_GetFsxScriptReferences_OutFile)' != '' "
            File="$(_GetFsxScriptReferences_OutFile)"
            Lines="@(ReferencePath -> '%(Identity)')"
            Overwrite="true" 
            Encoding="UTF-8"/>
    <!-- WriteLinesToFile doesnt create the file if @(ReferencePath) is empty -->
    <Touch
        Condition=" '$(_GetFsxScriptReferences_OutFile)' != '' "
        Files="$(_GetFsxScriptReferences_OutFile)"
        AlwaysCreate="True" />

  </Target>

</Project>

now you can run:

msbuild prova2.1.csproj /t:_GetFsxScriptReferences /p:_GetFsxScriptReferences_OutFile=prova.3.4.txt /p:TargetFrameworkVersion=v3.5

msbuild prova2.1.csproj /t:_GetFsxScriptReferences /p:_GetFsxScriptReferences_OutFile=prova.4.5.txt /p:TargetFrameworkVersion=v4.5

msbuild prova2.1.csproj /t:_GetFsxScriptReferences /p:_GetFsxScriptReferences_OutFile=prova.4.7.txt /p:TargetFrameworkVersion=v4.7

at will, to get the reference assemblies for these files, like prova.4.5.txt:

C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\PublicAssemblies\FSharp.Core.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\mscorlib.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Collections.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Core.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Data.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Drawing.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.IO.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Linq.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Linq.Expressions.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Net.Requests.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Numerics.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Reflection.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Runtime.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Runtime.Numerics.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Runtime.Remoting.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Runtime.Serialization.Formatters.Soap.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Threading.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\Facades\System.Threading.Tasks.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Web.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Web.Services.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Windows.Forms.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Xml.dll

so something similar to try find that settings.dll too

drvink commented 7 years ago

OK, that's what I thought. One problem with this, though, is that while we can expect that msbuild/xbuild exists somewhere, it won't necessarily be in the user's $PATH: consider, say, users of Emacs (there are probably better examples--Ionide?)--we're unlikely to find msbuild.exe unless the user runs Emacs from a developer command prompt or has otherwise set something up to set their $PATH appropriately. Could we assume the presence of some version of the MSBuild assemblies instead, and evaluate your project file (or something that gives us the info we want) some other way?

nosami commented 7 years ago

FWIW, I just took a local copy of FSharp.Compiler.Interactive.Service.dll and referenced it for the Interactive panel in VS for Mac.

Might make sense to just create a NuGet package for this and reference it that way.

drvink commented 7 years ago

That's an even better suggestion if it's tenable. It completely didn't occur to me that much of this stuff is shipped via NuGet nowadays. If someone can vouch that it isn't e.g. tied to specific F# versions (it's nice that FSAC has backwards compatibility), that sounds like the way to go--it could just be part of pushing FCS releases.

enricosada commented 7 years ago

@nosami yes this is a good idea

As @drvink said, if just need to be aligned with FCS, we can ship it with FSAC bundle and pass as additional reference (-r) to GetProjectOptionsFromScript and call it a day.

I'll check how do this. i am anyway playing around GetProjectOptionsFromScript for #224

enricosada commented 7 years ago

Happen also on windows, with fresh install because new VS 15.x doesnt put anymore that assembly in GAC -> same error.

ref https://github.com/ionide/ionide-vscode-fsharp/issues/590#issuecomment-335607170

drvink commented 7 years ago

I figured it had the ability to break elsewhere (which was why I gave the rundown of how it attempts to search), and I'm not surprised to hear it is, since there are so many ways it can appear to work on one person's system but not on another. We should try to get an answer re: whether that assembly can ship with FCS; it definitely seems the easiest solution, and it's definitely bad for this to appear totally broken to people who can't diagnose it themselves.

enricosada commented 7 years ago

Anthoer possibility is to add the assembly from fsi location.

as ref https://github.com/fsharp/fsharpbinding/blob/master/FSharp.CompilerBinding/CompilerLocationUtils.fs#L189

nosami commented 7 years ago

Anthoer possibility is to add the assembly from fsi location.

This seems wrong to me :)

I think you would be better off using the symlink at /Library/Frameworks/Mono.framework/Versions/Current to point to the current version on OSX - /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/fsharp/FSharp.Compiler.Interactive.Settings.dll.. or /usr/lib/mono/fsharp/FSharp.Compiler.Interactive.Settings.dll on linux.

drvink commented 7 years ago

This breaks on all platforms. We should strictly avoid, as much as possible, doing anything that gets into visualfsharp-vs.-fsharp, CLR vs. Mono, or other platform-specific details--the reason we are dealing with this bug at all is because of the fragility and needless complexity of all this code we have for probing where stuff might live. Can someone determine whether it's practical to distribute FSharp.Compiler.Interactive.Settings.dll with FCS and/or distribute it separately? We already know what we need (i.e. our dependency, i.e. the aforementioned assembly); we just have to determine the easiest way to ensure it can get bundled with FSAC somehow.

drvink commented 7 years ago

Wow. I should just have looked myself. Here is everything comprising FSharp.Compiler.Interactive.Settings.dll--it appears to be trivial and could easily be built standalone:

https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/FSInteractiveSettings.txt https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/FSharp.Compiler.Interactive.Settings/InternalsVisibleTo.fs https://github.com/Microsoft/visualfsharp/blob/master/src/assemblyinfo/assemblyinfo.FSharp.Compiler.Interactive.Settings.dll.fs https://github.com/Microsoft/visualfsharp/blob/master/src/utils/reshapedreflection.fs https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/fsiattrs.fs https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/fsiaux.fsi https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/fsiaux.fs

Who do we need to poke to get this tossed up on NuGet somewhere?

dsyme commented 7 years ago

It seems reasonable for someone to create a nuget of FSharp.Compiler.Interactive.Service.dll and distribute it with FSAC,

Any application such as FSAC built using FSharp.Compiler.Service.dll that wants to typecheck scripts needs a copy of that DLL, you may as well just distribute it with the application.

ghost commented 6 years ago

Any updates on this issue? I'm really new to .net / f# , coming from js/python but I like f# and i'm trying to learn it in my spare time. I'm going over the stuff in fsharpforfunandprofit and one of the first examples involves running this code from an fsx script.

match fsi.CommandLineArgs with
    | [| scriptName; url; targetfile |] ->
        printfn "running script: %s" scriptName
        downloadUriToFile url targetfile
    | _ ->
        printfn "USAGE: [url] [targetfile]"

I'm using vcode with ionide on Ubuntu 18 and the error appearing in the editor is

The value, namespace, type or module 'fsi' is not defined. Maybe you want one of the following: fst

Any details on how to fix this? I added the following reference to the file #r "FSharp.Compiler.Interactive.Settings.dll" but it didnt fix the error though.

Thanks...

sbacquet commented 5 years ago

And here is the 2019 reminder :)

Krzysztof-Cieslak commented 5 years ago

Solved as part of #466, yay!