Skip to content

Commit

Permalink
Preserve old FSIExtraParameters; pick reasonable options
Browse files Browse the repository at this point in the history
Bring the old `FSIExtraParameters` option back to not break existing clients.
Implement logic to mimic old behavior if the old option is used, warning if the
old option is mixed with either of the new.

Introduce a new private helper method
`AdaptiveFSharpLspServer.oldOrNewExtraParams` for use in RPC calls that parse
config options: `Initialize` and `DidChangeConfiguration`. This helper ensures
that any clients using the old parameter get the expected behavior from before
this PR. Both RPC methods will send a warning to the client if they mix
parameters.

In the future, we can update warnings to support deprecation in that helper
function.

The signature of the helper is not the best, and to work with the CEs where the
RPC happens, we need to have our message sent at the top level of the CE, so we
can use `do!`. This means that we read the Dto, then break our pipeline so we
can consume the message, then have a second pipeline that consumes the fixed up
Dto. Given that this is in support of a to-be-deprecated option, and that upon
deprecation we can remove the helper, it seems okay to have this ugliness.
  • Loading branch information
greggyb committed May 23, 2024
1 parent 841b449 commit 27343a1
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 5 deletions.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,10 @@ 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.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).
* 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.fsiExtraSharedParameters": ["--langversion:preview"]
Expand Down
7 changes: 7 additions & 0 deletions src/FsAutoComplete/LspHelpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,7 @@ 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
Expand Down Expand Up @@ -816,6 +817,9 @@ 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
Expand Down Expand Up @@ -868,6 +872,7 @@ type FSharpConfig =
LineLens = { Enabled = "never"; Prefix = "" }
UseSdkScripts = true
DotNetRoot = Environment.dotnetSDKRoot.Value.FullName
FSIExtraParameters = [||]
FSIExtraInteractiveParameters = [||]
FSIExtraSharedParameters = [||]
FSICompilerToolLocations = [||]
Expand Down Expand Up @@ -925,6 +930,7 @@ 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
Expand Down Expand Up @@ -1036,6 +1042,7 @@ 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
Expand Down
2 changes: 2 additions & 0 deletions src/FsAutoComplete/LspHelpers.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ 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
Expand Down Expand Up @@ -403,6 +404,7 @@ type FSharpConfig =
LineLens: LineLensConfig
UseSdkScripts: bool
DotNetRoot: string
FSIExtraParameters: string array
FSIExtraInteractiveParameters: string array
FSIExtraSharedParameters: string array
FSICompilerToolLocations: string array
Expand Down
59 changes: 56 additions & 3 deletions src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/FsAutoComplete.Tests.Lsp/Helpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ let defaultConfigDto: FSharpConfigDto =
LineLens = None
UseSdkScripts = Some true
DotNetRoot = None
FSIExtraParameters = None
FSIExtraInteractiveParameters = None
FSIExtraSharedParameters = None
FSICompilerToolLocations = None
Expand Down

0 comments on commit 27343a1

Please sign in to comment.