Separate fsi extra parameters (#1299)

# FIX: #1210; split `FSIExtraParameters` in two
    
`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

README.md

@@ -199,10 +199,14 @@ 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.
+* Extra parameters for FSI: use only `FSharp.FSIExtraParameters` on its own *or* a combination of `FSharp.FSIExtraInteractiveParameters` and `FSharp.FSIExtraSharedParameters`. The former is expected to be deprecated in favor of the second two. See #1210 for more detail. FSAC will send a warning if you mix usage improperly.
+  * `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.FSIExtraSharedParameters` - 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

src/FsAutoComplete/LspHelpers.fs

@@ -674,6 +674,8 @@ type FSharpConfigDto =
     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
@@ -815,7 +817,13 @@ type FSharpConfig =
     LineLens: LineLensConfig
     UseSdkScripts: bool
     DotNetRoot: string
+    // old, combines both shared and interactive. To be deprecated. Either use
+    // only this one, or some combination of the new ones.
     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
@@ -865,6 +873,8 @@ type FSharpConfig =
       UseSdkScripts = true
       DotNetRoot = Environment.dotnetSDKRoot.Value.FullName
       FSIExtraParameters = [||]
+      FSIExtraInteractiveParameters = [||]
+      FSIExtraSharedParameters = [||]
       FSICompilerToolLocations = [||]
       TooltipMode = "full"
       GenerateBinlog = false
@@ -921,6 +931,9 @@ type FSharpConfig =
         |> 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
@@ -1030,6 +1043,9 @@ type FSharpConfig =
         |> 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

src/FsAutoComplete/LspHelpers.fsi

@@ -302,6 +302,8 @@ type FSharpConfigDto =
     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
@@ -403,6 +405,8 @@ type FSharpConfig =
     UseSdkScripts: bool
     DotNetRoot: string
     FSIExtraParameters: string array
+    FSIExtraInteractiveParameters: string array
+    FSIExtraSharedParameters: string array
     FSICompilerToolLocations: string array
     TooltipMode: string
     GenerateBinlog: bool

src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs

@@ -284,6 +284,41 @@ type AdaptiveFSharpLspServer
 
     Helpers.ignoreNotification
 
+  ///<summary>
+  /// Transform a config DTO to use (old FSIExtraParameters) xor (new FSIExtraInteractiveParameters and FSIExtraSharedParameters).
+  ///</summary>
+  ///<remarks>
+  /// We expect to eventually deprecate FSIExtraParameters in favor of the two
+  /// new, specialized versions. For now, mimic old behavior either silently
+  /// or loudly, depending on the client's combination of config options. This
+  /// method and the consumption of it can be removed after deprecation.
+  ///</remarks>
+  static member private oldOrNewExtraParams(dto: FSharpConfigDto) =
+    match dto.FSIExtraParameters, dto.FSIExtraInteractiveParameters, dto.FSIExtraSharedParameters with
+    // old-style, silent success; start warning when we plan to
+    // deprecate
+    | Some p, None, None ->
+      let c =
+        { dto with
+            FSIExtraSharedParameters = Some p }
+
+      None, c
+    // mix old and new, warn and mimic old behavior
+    | Some p, Some _, _
+    | Some p, _, Some _ ->
+      let m =
+        { Type = MessageType.Warning
+          Message =
+            "Do not mix usage of FSIExtraParameters and (FSIExtraInteractiveParameters or FSIExtraSharedParameters)." }
+
+      let c =
+        { dto with
+            FSIExtraSharedParameters = Some p }
+
+      Some m, c
+    // no old parameter, proceed happily
+    | None, _, _ -> None, dto
+
   interface IFSharpLspServer with
     override x.Shutdown() = (x :> System.IDisposable).Dispose() |> async.Return
 
@@ -295,10 +330,19 @@ type AdaptiveFSharpLspServer
         try
           logger.info (Log.setMessage "Initialize Request {p}" >> Log.addContextDestructured "p" p)
 
-          let c =
+          let configMessage =
             p.InitializationOptions
             |> Option.bind (fun options -> if options.HasValues then Some options else None)
             |> Option.map Server.deserialize<FSharpConfigDto>
+            |> Option.map AdaptiveFSharpLspServer.oldOrNewExtraParams
+
+          match Option.bind fst configMessage with
+          | Some message -> do! lspClient.WindowShowMessage message
+          | None -> ()
+
+          let c =
+            configMessage
+            |> Option.map snd
             |> Option.map FSharpConfig.FromDto
             |> Option.defaultValue FSharpConfig.Default
 
@@ -1684,9 +1728,18 @@ type AdaptiveFSharpLspServer
             >> Log.addContextDestructured "params" p
           )
 
-          let dto = p.Settings |> Server.deserialize<FSharpConfigRequest>
+          let dto =
+            p.Settings
+            |> Server.deserialize<FSharpConfigRequest>
+            |> (fun f -> f.FSharp)
+            |> Option.map AdaptiveFSharpLspServer.oldOrNewExtraParams
+
+          match Option.bind fst dto with
+          | Some message -> do! lspClient.WindowShowMessage message
+          | None -> ()
 
-          dto.FSharp
+          dto
+          |> Option.map snd
           |> Option.iter (fun fsharpConfig ->
             let c = state.Config
             let c = c.AddDto fsharpConfig

src/FsAutoComplete/LspServers/AdaptiveServerState.fs

@@ -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
 

test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs

@@ -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

test/FsAutoComplete.Tests.Lsp/CodeFixTests/ToInterpolatedStringTests.fs

@@ -1,4 +1,4 @@
-module private FsAutoComplete.Tests.CodeFixTests.ToInterpolatedStringTests
+module private FsAutoComplete.Tests.CodeFixTests.ToInterpolatedStringTests
 
 open Expecto
 open Helpers
@@ -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 ->
@@ -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 ->

test/FsAutoComplete.Tests.Lsp/Helpers.fs

@@ -264,6 +264,8 @@ let defaultConfigDto: FSharpConfigDto =
     UseSdkScripts = Some true
     DotNetRoot = None
     FSIExtraParameters = None
+    FSIExtraInteractiveParameters = None
+    FSIExtraSharedParameters = None
     FSICompilerToolLocations = None
     TooltipMode = None
     GenerateBinlog = Some true

test/FsAutoComplete.Tests.Lsp/ScriptTests.fs

@@ -24,7 +24,7 @@ let scriptPreviewTests state =
         serverInitialize
           path
           { defaultConfigDto with
-              FSIExtraParameters = Some [| "--langversion:preview" |] }
+              FSIExtraSharedParameters = Some [| "--langversion:preview" |] }
           state
 
       do! waitForWorkspaceFinishedParsing events
@@ -81,7 +81,7 @@ let scriptEvictionTests state =
                   { FSharp =
                       Some
                         { defaultConfigDto with
-                            FSIExtraParameters = Some [| "--nowarn:760" |] } }
+                            FSIExtraSharedParameters = Some [| "--nowarn:760" |] } }
 
                 { Settings = Server.serialize config }
 
@@ -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
@@ -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()