Skip to content

Commit

Permalink
fix(language-server): support LSP clients that only support `workspac…
Browse files Browse the repository at this point in the history
…e/configuration`.

See OmniSharp/csharp-language-server-protocol#1101 for the upstream bug that necessitates this.  We can revert to unscoped configuration when that is resolved.
  • Loading branch information
chrissimon-au committed Nov 13, 2023
1 parent 62fa42f commit 9beb586
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ let completionTests =
$"Given {fileName} contextive, in document {text} at position {position} respond with expected completion list " {
let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]

use! client = TestClient(config) |> init

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let definitionsTests =
[ testAsync "Can receive configuration value" {
let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathOptionsBuilder "one.yml" ]
ConfigurationSection.contextivePathBuilder "one.yml" ]

use! client = TestClient(config) |> init

Expand All @@ -29,17 +29,32 @@ let definitionsTests =

let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathLoaderOptionsBuilder pathLoader ]
ConfigurationSection.contextivePathLoaderBuilder pathLoader ]

use! client = TestClient(config) |> init

path <- "two.yml"
ConfigurationSection.didChange client path
ConfigurationSection.didChangePath client path

let! labels = Completion.getCompletionLabels client

test <@ (labels, Fixtures.Two.expectedCompletionLabels) ||> Seq.compareWith compare = 0 @>

}

testAsync "Can handle client that doesn't support didChangeConfiguration" {
let mutable path = "one.yml"

let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathBuilder path ]

use! client = TestClient(config) |> init

let! labels = Completion.getCompletionLabels client

test <@ (labels, Fixtures.One.expectedCompletionLabels) ||> Seq.compareWith compare = 0 @>

}

]
Original file line number Diff line number Diff line change
Expand Up @@ -163,21 +163,21 @@ let definitionsTests =

let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathLoaderOptionsBuilder pathLoader ]
ConfigurationSection.contextivePathLoaderBuilder pathLoader ]

use! client = TestClient(config) |> init

let! termsWhenValidAtStart = Completion.getCompletionLabels client
test <@ (termsWhenValidAtStart, Fixtures.One.expectedCompletionLabels) ||> compareList = 0 @>

path <- $"{fileName}.yml"
ConfigurationSection.didChange client path
ConfigurationSection.didChangePath client path

let! termsWhenInvalid = Completion.getCompletionLabels client
test <@ Seq.length termsWhenInvalid = 0 @>

path <- validPath
ConfigurationSection.didChange client path
ConfigurationSection.didChangePath client path

let! termsWhenValidAtEnd = Completion.getCompletionLabels client
test <@ (termsWhenValidAtEnd, Fixtures.One.expectedCompletionLabels) ||> compareList = 0 @>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ open OmniSharp.Extensions.LanguageServer.Protocol.Client
open OmniSharp.Extensions.LanguageServer.Protocol.Workspace
open System.Threading.Tasks

let jTokenFromMap values =
let private jTokenFromMap values =
let configValue = JObject()

values
Expand Down Expand Up @@ -38,27 +38,50 @@ let private createHandler section configValuesLoader =
else
Task.FromResult(configSectionResultFromMap <| Map [])

let optionsBuilder section configValuesLoader (b: LanguageClientOptions) =
let configurationHandlerBuilder section configValuesLoader (b: LanguageClientOptions) =
let handler = createHandler section configValuesLoader

b.OnConfiguration(handler) |> ignore
b
.WithCapability(Capabilities.DidChangeConfigurationCapability(DynamicRegistration = true))
.OnConfiguration(handler)

let private didChangeConfigurationBuilder (b: LanguageClientOptions) =
b.WithCapability(Capabilities.DidChangeConfigurationCapability(DynamicRegistration = true))
|> ignore

b

type PathLoader = unit -> obj

let mapLoader (pathLoader: PathLoader) () = Map[("path", pathLoader ())]

let contextivePathLoaderOptionsBuilder (pathLoader: PathLoader) =
optionsBuilder "contextive" <| mapLoader pathLoader

let contextivePathOptionsBuilder path =
contextivePathLoaderOptionsBuilder (fun () -> path)

let didChange (client: ILanguageClient) path =
type ValueLoader = unit -> obj

let private mapLoader (key: string) (valueLoader: ValueLoader) () = Map[(key, valueLoader ())]

let private mapPathLoader = mapLoader "path"

let private contextivePathConfigurationHandlerBuilder (pathLoader: ValueLoader) =
configurationHandlerBuilder "contextive" <| mapPathLoader pathLoader

/// <summary>
/// <para>Use when you have a setting value that needs to change during the test.</para>
/// <para>The test client will support `workspace/configuration` AND `workspace/didChangeConfiguration`.</para>
/// <para>When a `workspace/configuration` request is received, the `pathLoader` function will be invoked to get the current value.</para>
/// <para>To trigger a `workspace/didChangeConfiguration` notification, use <see cref="didChangePath">didChangePath</see>.</para>
/// </summary>
let contextivePathLoaderBuilder (pathLoader: ValueLoader) =
contextivePathConfigurationHandlerBuilder pathLoader
>> didChangeConfigurationBuilder

/// <summary>
/// <para>Use when you have a fixed setting value.</para>
/// <para>The test client will ONLY support `workspace/configuration` and will always supply this fixed value</para>
/// <para>Using <see cref="didChangePath">didChangePath</see> will have no impact as the server will not support the `workspace/didChangeConfiguration` notification</para>
/// </summary>
let contextivePathBuilder path =
contextivePathConfigurationHandlerBuilder (fun () -> path)

/// <summary>
/// <para>Trigger a `workspace/didChangeConfiguration` notification with the new value.</para>
/// <para>This will only work if the TestClient was created with the <see cref="contextivePathLoaderBuilder">contextivePathLoaderBuilder</see>.</para>
/// <para>Ensure this method is invoked with the same path value as will be returned from the `pathLoader` registered with the TestClient.</para>
/// </summary>
let didChangePath (client: ILanguageClient) path =
let setting = jTokenFromMap <| Map[("path", path)]
let configSection = jTokenFromMap <| Map[("contextive", setting)]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ let hoverTests =
$"Given definitions file '{fileName}' and file contents '{multiLineToSingleLine text}', server responds to hover request at Position {position} with '{expectedTerm}'" {
let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]

use! client = TestClient(config) |> init

Expand Down Expand Up @@ -89,7 +89,7 @@ let hoverTests =

let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]

use! client = TestClient(config) |> init

Expand Down Expand Up @@ -129,7 +129,7 @@ let hoverTests =

let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]

use! client = TestClient(config) |> init

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,38 +28,34 @@ let initializationTests =

let config =
[ Workspace.optionsBuilder ""
ConfigurationSection.contextivePathOptionsBuilder pathValue ]
ConfigurationSection.contextivePathBuilder pathValue ]

let! (client, reply) = TestClient(config) |> initAndWaitForReply

test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @>
test <@ client.ClientSettings.Capabilities.Workspace.DidChangeConfiguration.IsSupported @>

test <@ (defaultArg reply "").Contains(pathValue) @>
}

testAsync "Server loads contextive file from absolute location without workspace" {
let pathValue = Guid.NewGuid().ToString()

let config =
[ ConfigurationSection.contextivePathOptionsBuilder $"/tmp/{pathValue}" ]
let config = [ ConfigurationSection.contextivePathBuilder $"/tmp/{pathValue}" ]

let! (client, reply) = TestClient(config) |> initAndWaitForReply

test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @>
test <@ client.ClientSettings.Capabilities.Workspace.DidChangeConfiguration.IsSupported @>

test <@ (defaultArg reply "").Contains(pathValue) @>
}

testAsync "Server does NOT load contextive file from relative location without workspace" {
let pathValue = Guid.NewGuid().ToString()
let config = [ ConfigurationSection.contextivePathOptionsBuilder pathValue ]
let config = [ ConfigurationSection.contextivePathBuilder pathValue ]

let! (client, reply) = TestClientWithCustomInitWait(config, Some pathValue) |> initAndWaitForReply

test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @>
test <@ client.ClientSettings.Capabilities.Workspace.DidChangeConfiguration.IsSupported @>

test
<@
Expand All @@ -71,7 +67,7 @@ let initializationTests =
testAsync "Server loads contextive file from default location when no configuration supplied" {
let config =
[ Workspace.optionsBuilder ""
ConfigurationSection.optionsBuilder "dummySection" (fun () -> Map []) ]
ConfigurationSection.configurationHandlerBuilder "dummySection" (fun () -> Map []) ]

let defaultPath = ".contextive/definitions.yml"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ let initializationTests =
let config =
[ showMessageRequestHandlerBuilder <| handler messageAwaiter null
Workspace.optionsBuilder ""
ConfigurationSection.contextivePathOptionsBuilder pathValue ]
ConfigurationSection.contextivePathBuilder pathValue ]

let! (client, reply) = TestClient(config) |> initAndWaitForReply

Expand All @@ -75,7 +75,7 @@ let initializationTests =
let config =
[ showMessageRequestHandlerBuilder <| handler messageAwaiter null
Workspace.optionsBuilder ""
ConfigurationSection.contextivePathOptionsBuilder pathValue ]
ConfigurationSection.contextivePathBuilder pathValue ]

let! (client, reply) = TestClient(config) |> initAndWaitForReply

Expand All @@ -102,7 +102,7 @@ let initializationTests =
[ showMessageRequestHandlerBuilder <| handler messageAwaiter response
showDocumentRequestHandlerBuilder <| handler showDocAwaiter showDocResponse
Workspace.optionsBuilder ""
ConfigurationSection.contextivePathOptionsBuilder pathValue ]
ConfigurationSection.contextivePathBuilder pathValue ]

let! (client, reply) = TestClient(config) |> initAndWaitForReply

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ let textDocumentTests =
testAsync "Server supports full sync" {
let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathOptionsBuilder $"one.yml" ]
ConfigurationSection.contextivePathBuilder $"one.yml" ]

use! client = TestClient(config) |> init
test <@ client.ServerSettings.Capabilities.TextDocumentSync.Options.Change = TextDocumentSyncKind.Full @>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ let watchedFileTests =

let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathOptionsBuilder $"one.yml"
ConfigurationSection.contextivePathBuilder $"one.yml"
WatchedFiles.optionsBuilder registrationAwaiter ]

let! client = TestClient(config) |> init
Expand All @@ -62,7 +62,7 @@ let watchedFileTests =

let config =
[ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
ConfigurationSection.contextivePathLoaderOptionsBuilder pathLoader
ConfigurationSection.contextivePathLoaderBuilder pathLoader
WatchedFiles.optionsBuilder registrationAwaiter ]

let! client = TestClient(config) |> init
Expand All @@ -72,7 +72,7 @@ let watchedFileTests =
ConditionAwaiter.clear registrationAwaiter 500

definitionsFile <- newDefinitionsFile
ConfigurationSection.didChange client definitionsFile
ConfigurationSection.didChangePath client definitionsFile

let! secondRegistrationMsg =
ConditionAwaiter.waitFor
Expand Down Expand Up @@ -113,7 +113,7 @@ let watchedFileTests =

let config =
[ Workspace.optionsBuilder relativePath
ConfigurationSection.contextivePathOptionsBuilder definitionsFile ]
ConfigurationSection.contextivePathBuilder definitionsFile ]

let! client = TestClient(config) |> init

Expand Down Expand Up @@ -153,7 +153,7 @@ let watchedFileTests =

let config =
[ Workspace.optionsBuilder relativePath
ConfigurationSection.contextivePathOptionsBuilder definitionsFile ]
ConfigurationSection.contextivePathBuilder definitionsFile ]

let! client = TestClient(config) |> init

Expand Down
5 changes: 4 additions & 1 deletion src/language-server/Contextive.LanguageServer/Server.fs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ let private getConfig (s: ILanguageServer) section key =
Log.Logger.Information $"Getting {section} {key} config..."

let! config =
s.Configuration.GetConfiguration(ConfigurationItem(Section = configSection))
// We need to use `GetScopedConfiguration` because of https://github.com/OmniSharp/csharp-language-server-protocol/issues/1101
// We can revert to `GetConfiguration` when that bug is fixed.
s.Configuration.GetScopedConfiguration("file:///", System.Threading.CancellationToken.None)
//s.Configuration.GetConfiguration(ConfigurationItem(Section = configSection))
|> Async.AwaitTask

let configValue = config.GetSection(section).Item(key)
Expand Down

0 comments on commit 9beb586

Please sign in to comment.