Skip to content

Commit

Permalink
FIX: #1210; split FSIExtraParameters in two
Browse files Browse the repository at this point in the history
`FSIExtraParameters` (old) becomes `FSIExtraInteractiveParameters` and
`FSIExtraSharedParameters`.

Previously, `FSIExtraParameters` was passed directly to the compiler for code
analysis. This is to ensure that the static analysis of a .fsx script file is congruent
with the code being evaluated in the interpreter. The FSI interpreter has
different parameters than the compiler, [documented
here](https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/fsharp-interactive-options).

When an interactive-only parameter was used, this led to compiler errors.

It is intended that FSAC will learn to manage the interactive interpreter in the
future, but it does not yet do so. Editor plugins manage the interpreter right
now. Some plugins (at least Ionide-vim) use the config option
`FSIExtraParameters` to configure that interpreter, and this makes sense.

To support the current state and the future state, we split the single
`FSIExtraParameters` into `FSIExtraInteractiveParameters` and
`FSIExtraSharedParameters` so that the interactive interpreter can be launched
with the union of these two and the compiler can receive only the shared
parameters that it knows how to deal with. This allows editors to use both
config options today to launch the interpreter. Today, FSAC only uses the shared
parameters.

In the future, when FSAC learns to manage the interactive interpreter, it can
union the parameters for the interactive session and continue to pass only the
shared parameters to the compiler. When this change occurs, if an editor plugin
still manages the `dotnet fsi` interpreter, that will continue to work. Editor
plugins can opt in to FSAC-managed interpreters at their will, and with the same
configuration options the users already have set up.

Additional discussion:
- #1210: proximate bug: #1210
- `dotnet fsi` readline bug, making this option very salient for editor plugins:
  dotnet/fsharp#17215
  • Loading branch information
greggyb committed May 23, 2024
1 parent b43fe3a commit e35615c
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 19 deletions.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,12 @@ Options that should be send as `initializationOptions` as part of `initialize` r
* `FSharp.ResolveNamespaces` - enables resolve namespace quick fix (add `open` if symbol is from not yet opened module/namespace), recommended default value: `true`
* `FSharp.EnableReferenceCodeLens` - enables reference count code lenses, recommended default value: `true` if `--background-service-enabled` is used by default, `false` otherwise
* `FSharp.dotNetRoot` - sets the root path for finding dotnet SDK references. Primarily used for FSX Scripts. Default value: operating-system dependent. On windows, `C:\Program Files\dotnet`; on Unix, `/usr/local/share/dotnet`
* `FSharp.fsiExtraParameters` - an array of additional runtime arguments that are passed to FSI. These are used when typechecking scripts to ensure that typechecking has the same context as your FSI instances. An example would be to set the following parameters to enable Preview features (like opening static classes) for typechecking.
* `FSharp.fsiExtraInteractiveParameters` - currently unused by FSAC, but available to editor plugins for interactive `dotnet fsi` parameters that are not shared by the compiler. Future intentions are to manage the interpreter from FSAC, at which point FSAC will utilize this parameter. [Check this reference for parameters that are interactive-only or shared with the compiler](https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/fsharp-interactive-options).
* `FSharp.fsiExtraParameters` - an array of additional runtime arguments that are passed to FSI; specifically parameters that are shared with the compiler. These are used when typechecking scripts to ensure that typechecking has the same context as your FSI instances. An example would be to set the following parameters to enable Preview features (like opening static classes) for typechecking. [Check this reference for parameters that are interactive-only or shared with the compiler](https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/fsharp-interactive-options).

```json
"FSharp.fsiExtraParameters": ["--langversion:preview"]
"FSharp.fsiExtraSharedParameters": ["--langversion:preview"]
"FSharp.fsiExtraInteractiveParameters": ["--readline-"]
```

#### Debug Settings
Expand Down
17 changes: 12 additions & 5 deletions src/FsAutoComplete/LspHelpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,8 @@ type FSharpConfigDto =
LineLens: LineLensConfig option
UseSdkScripts: bool option
DotNetRoot: string option
FSIExtraParameters: string array option
FSIExtraInteractiveParameters: string array option
FSIExtraSharedParameters: string array option
FSICompilerToolLocations: string array option
TooltipMode: string option
GenerateBinlog: bool option
Expand Down Expand Up @@ -815,7 +816,10 @@ type FSharpConfig =
LineLens: LineLensConfig
UseSdkScripts: bool
DotNetRoot: string
FSIExtraParameters: string array
// for parameters only used in interactive FSI sessions; currently unused
FSIExtraInteractiveParameters: string array
// for parameters used both in the compiler and interactive FSI
FSIExtraSharedParameters: string array
FSICompilerToolLocations: string array
TooltipMode: string
GenerateBinlog: bool
Expand Down Expand Up @@ -864,7 +868,8 @@ type FSharpConfig =
LineLens = { Enabled = "never"; Prefix = "" }
UseSdkScripts = true
DotNetRoot = Environment.dotnetSDKRoot.Value.FullName
FSIExtraParameters = [||]
FSIExtraInteractiveParameters = [||]
FSIExtraSharedParameters = [||]
FSICompilerToolLocations = [||]
TooltipMode = "full"
GenerateBinlog = false
Expand Down Expand Up @@ -920,7 +925,8 @@ type FSharpConfig =
dto.DotNetRoot
|> Option.bind (fun s -> if String.IsNullOrEmpty s then None else Some s)
|> Option.defaultValue Environment.dotnetSDKRoot.Value.FullName
FSIExtraParameters = defaultArg dto.FSIExtraParameters FSharpConfig.Default.FSIExtraParameters
FSIExtraInteractiveParameters = defaultArg dto.FSIExtraInteractiveParameters FSharpConfig.Default.FSIExtraInteractiveParameters
FSIExtraSharedParameters = defaultArg dto.FSIExtraSharedParameters FSharpConfig.Default.FSIExtraSharedParameters
FSICompilerToolLocations = defaultArg dto.FSICompilerToolLocations FSharpConfig.Default.FSICompilerToolLocations
TooltipMode = defaultArg dto.TooltipMode "full"
GenerateBinlog = defaultArg dto.GenerateBinlog false
Expand Down Expand Up @@ -1029,7 +1035,8 @@ type FSharpConfig =
dto.DotNetRoot
|> Option.bind (fun s -> if String.IsNullOrEmpty s then None else Some s)
|> Option.defaultValue FSharpConfig.Default.DotNetRoot
FSIExtraParameters = defaultArg dto.FSIExtraParameters FSharpConfig.Default.FSIExtraParameters
FSIExtraInteractiveParameters = defaultArg dto.FSIExtraInteractiveParameters FSharpConfig.Default.FSIExtraInteractiveParameters
FSIExtraSharedParameters = defaultArg dto.FSIExtraSharedParameters FSharpConfig.Default.FSIExtraSharedParameters
FSICompilerToolLocations = defaultArg dto.FSICompilerToolLocations FSharpConfig.Default.FSICompilerToolLocations
TooltipMode = defaultArg dto.TooltipMode x.TooltipMode
GenerateBinlog = defaultArg dto.GenerateBinlog x.GenerateBinlog
Expand Down
6 changes: 4 additions & 2 deletions src/FsAutoComplete/LspHelpers.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,8 @@ type FSharpConfigDto =
LineLens: LineLensConfig option
UseSdkScripts: bool option
DotNetRoot: string option
FSIExtraParameters: string array option
FSIExtraInteractiveParameters: string array option
FSIExtraSharedParameters: string array option
FSICompilerToolLocations: string array option
TooltipMode: string option
GenerateBinlog: bool option
Expand Down Expand Up @@ -402,7 +403,8 @@ type FSharpConfig =
LineLens: LineLensConfig
UseSdkScripts: bool
DotNetRoot: string
FSIExtraParameters: string array
FSIExtraInteractiveParameters: string array
FSIExtraSharedParameters: string array
FSICompilerToolLocations: string array
TooltipMode: string
GenerateBinlog: bool
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete/LspServers/AdaptiveServerState.fs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ type AdaptiveState
|> Observable.subscribe (fun (config, checker, rootPath) ->
toggleTraceNotification config.Notifications.Trace config.Notifications.TraceNamespaces

setFSIArgs checker config.FSICompilerToolLocations config.FSIExtraParameters
setFSIArgs checker config.FSICompilerToolLocations config.FSIExtraSharedParameters

loadAnalyzers config rootPath

Expand Down
2 changes: 1 addition & 1 deletion test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2333,7 +2333,7 @@ let private removeUnnecessaryReturnOrYieldTests state =
let private removeUnusedBindingTests state =
let config =
{ defaultConfigDto with
FSIExtraParameters = Some [| "--warnon:1182" |] }
FSIExtraSharedParameters = Some [| "--warnon:1182" |] }

serverTestList (nameof RemoveUnusedBinding) state config None (fun server ->
[ let selectRemoveUnusedBinding = CodeFix.withTitle RemoveUnusedBinding.titleBinding
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module private FsAutoComplete.Tests.CodeFixTests.ToInterpolatedStringTests
module private FsAutoComplete.Tests.CodeFixTests.ToInterpolatedStringTests

open Expecto
open Helpers
Expand All @@ -9,7 +9,7 @@ open FsAutoComplete.FCSPatches

let langVersion60Config =
{ defaultConfigDto with
FSIExtraParameters = Some [| "--langversion:6.0" |] }
FSIExtraSharedParameters = Some [| "--langversion:6.0" |] }

let tests state =
serverTestList (nameof ToInterpolatedString) state langVersion60Config None (fun server ->
Expand Down Expand Up @@ -356,7 +356,7 @@ let tests state =

let langVersion47Config =
{ defaultConfigDto with
FSIExtraParameters = Some [| "--langversion:4.7" |] }
FSIExtraSharedParameters = Some [| "--langversion:4.7" |] }

let unavailableTests state =
serverTestList $"unavailable {(nameof ToInterpolatedString)}" state langVersion47Config None (fun server ->
Expand Down
3 changes: 2 additions & 1 deletion test/FsAutoComplete.Tests.Lsp/Helpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ let defaultConfigDto: FSharpConfigDto =
LineLens = None
UseSdkScripts = Some true
DotNetRoot = None
FSIExtraParameters = None
FSIExtraInteractiveParameters = None
FSIExtraSharedParameters = None
FSICompilerToolLocations = None
TooltipMode = None
GenerateBinlog = Some true
Expand Down
8 changes: 4 additions & 4 deletions test/FsAutoComplete.Tests.Lsp/ScriptTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ let scriptPreviewTests state =
serverInitialize
path
{ defaultConfigDto with
FSIExtraParameters = Some [| "--langversion:preview" |] }
FSIExtraSharedParameters = Some [| "--langversion:preview" |] }
state

do! waitForWorkspaceFinishedParsing events
Expand Down Expand Up @@ -81,7 +81,7 @@ let scriptEvictionTests state =
{ FSharp =
Some
{ defaultConfigDto with
FSIExtraParameters = Some [| "--nowarn:760" |] } }
FSIExtraSharedParameters = Some [| "--nowarn:760" |] } }

{ Settings = Server.serialize config }

Expand Down Expand Up @@ -115,7 +115,7 @@ let dependencyManagerTests state =

let dependencyManagerEnabledConfig =
{ defaultConfigDto with
FSIExtraParameters = Some [| "--langversion:preview" |] }
FSIExtraSharedParameters = Some [| "--langversion:preview" |] }

let! (server, events) = serverInitialize workingDir dependencyManagerEnabledConfig state
do! waitForWorkspaceFinishedParsing events
Expand Down Expand Up @@ -165,7 +165,7 @@ let scriptProjectOptionsCacheTests state =

let previewEnabledConfig =
{ defaultConfigDto with
FSIExtraParameters = Some [| "--langversion:preview" |] }
FSIExtraSharedParameters = Some [| "--langversion:preview" |] }

let! (server, events) = serverInitialize workingDir previewEnabledConfig state
let options = ResizeArray()
Expand Down

0 comments on commit e35615c

Please sign in to comment.