Skip to content

Commit

Permalink
Fix how we assign FSI options
Browse files Browse the repository at this point in the history
For netcore/netfx we should always explicitly provide the targetprofile,
because the impacts how reference resolution works (ie which primary
assembly we look for and key off of), and we should also provide any
static additional arguments as the 'otherFlags' argument.

In addition, we need to explicitly not provide an mscorlib reference
when targeting netstandard, as that re-enables some of the legacy paths.

Finally, there was a bug in the handling of the 'replace refs' function
that was overwriting all of the OtherOptions instead of modifying just
the framework refs.
  • Loading branch information
baronfel committed Nov 13, 2019
1 parent 2d5679b commit db8baa1
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 19 deletions.
33 changes: 15 additions & 18 deletions src/FsAutoComplete.Core/CompilerServiceInterface.fs
Original file line number Diff line number Diff line change
Expand Up @@ -511,15 +511,14 @@ type FSharpCompilerServiceChecker(backgroundServiceEnabled) =

/// replace any BCL/FSharp.Core/FSI refs that FCS gives us with our own set, which is more probe-able
let replaceFrameworkRefs (projOptions: FSharpProjectOptions) =
let refs, otherOptions = projOptions.OtherOptions |> Array.partition (fun r -> r.StartsWith "-r:")

let fcsAndScriptReferences =
projOptions.OtherOptions
|> Array.choose (fun r ->
if r.StartsWith("-r")
then
refs
|> Array.map (fun r ->
let path = r.[3..]
let assemblyName = Path.GetFileNameWithoutExtension path
Some(assemblyName, path)
else None
assemblyName, path
)
|> Map.ofArray

Expand All @@ -531,10 +530,7 @@ type FSharpCompilerServiceChecker(backgroundServiceEnabled) =
|> Seq.map (fun r -> "-r:" + r)
|> Array.ofSeq

{ projOptions with OtherOptions = mergedRefs }

let applyFSIAdditionalArgs (projOptions: FSharpProjectOptions) =
{ projOptions with OtherOptions = Array.append fsiAdditionalArguments projOptions.OtherOptions }
{ projOptions with OtherOptions = Array.append otherOptions mergedRefs }

member __.CreateFCSBinder(netFwInfo: Dotnet.ProjInfo.Workspace.NetFWInfo, loader: Dotnet.ProjInfo.Workspace.Loader) =
Dotnet.ProjInfo.Workspace.FCS.FCSBinder(netFwInfo, loader, checker)
Expand All @@ -555,15 +551,16 @@ type FSharpCompilerServiceChecker(backgroundServiceEnabled) =

member private __.GetNetFxScriptOptions(file, source) = async {
logDebug "[Opts] Getting NetFX options for script file %s" file
let! (opts, errors) = checker.GetProjectOptionsFromScript(file, SourceText.ofString source, assumeDotNetFramework = true, useFsiAuxLib = true)
let allModifications = applyFSIAdditionalArgs
return allModifications opts, errors
let allFlags = Array.append [| "--targetprofile:mscorlib" |] fsiAdditionalArguments
let! (opts, errors) = checker.GetProjectOptionsFromScript(file, SourceText.ofString source, assumeDotNetFramework = true, useFsiAuxLib = true, otherFlags = allFlags, userOpName = "getNetFrameworkScriptOptions")
return opts, errors
}

member private __.GetNetCoreScriptOptions(file, source) = async {
logDebug "[Opts] Getting NetCore options for script file %s" file
let! (opts, errors) = checker.GetProjectOptionsFromScript(file, SourceText.ofString source, assumeDotNetFramework = false, useSdkRefs = true, useFsiAuxLib = true)
let allModifications = replaceFrameworkRefs >> applyFSIAdditionalArgs
let allFlags = Array.append [| "--targetprofile:netstandard" |] fsiAdditionalArguments
let! (opts, errors) = checker.GetProjectOptionsFromScript(file, SourceText.ofString source, assumeDotNetFramework = false, useSdkRefs = true, useFsiAuxLib = true, otherFlags = allFlags, userOpName = "getNetCoreScriptOptions")
let allModifications = replaceFrameworkRefs
return allModifications opts, errors
}

Expand All @@ -577,9 +574,9 @@ type FSharpCompilerServiceChecker(backgroundServiceEnabled) =

match errors with
| [] ->
let refs = projOptions.OtherOptions |> Array.filter (fun o -> o.StartsWith("-r")) |> String.concat "\n"
logDebug "[Opts] Resolved options - %A" projOptions
logDebug "[Opts] Resolved references:\n%s" refs
let refs, otherOpts = projOptions.OtherOptions |> Array.partition (fun o -> o.StartsWith("-r"))
logDebug "[Opts] Resolved references:\n%s" (refs |> String.concat "\n")
logDebug "[Opts] Resolved other options:\n%s" (otherOpts |> String.concat "\n")
| errs ->
logDebug "[Opts] Resolved options with errors\n%A\n%A" projOptions errs

Expand Down
8 changes: 7 additions & 1 deletion src/FsAutoComplete.Core/FSIRefs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,13 @@ let runtimeDir dotnetRoot runtimeVersion =
let findRuntimeRefs packDir runtimeDir =
match packDir, runtimeDir with
| Some refDir, _
| _, Some refDir -> Directory.EnumerateFiles(refDir, "*.dll") |> Seq.toArray
| _, Some refDir ->
Directory.EnumerateFiles(refDir, "*.dll")
// SUPER IMPORTANT: netstandard/netcore assembly resolution _must not_ contain mscorlib or else
// its presence triggers old netfx fallbacks, which end up bringing assemblies that aren't part
// of netcore.
|> Seq.filter (fun r -> not (Path.GetFileNameWithoutExtension(r) = "mscorlib"))
|> Seq.toArray
| None, None -> [||]

/// given the compiler root dir and if to include FSI refs, returns the set of compiler assemblies to references if that dir exists.
Expand Down

0 comments on commit db8baa1

Please sign in to comment.