Skip to content

Commit 2ad5313

Browse files
lukaszsamsonlukaszsamson
committed
pull configuration on workspace/didChangeConfiguration
add tests
1 parent 70cf84e commit 2ad5313

File tree

7 files changed

+257
-203
lines changed

7 files changed

+257
-203
lines changed

‎apps/language_server/lib/language_server/server.ex

Lines changed: 55 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -294,32 +294,7 @@ defmodule ElixirLS.LanguageServer.Server do
294294
# according to https://github.com/microsoft/language-server-protocol/issues/567#issuecomment-1060605611
295295
# this is the best point to pull configuration
296296

297-
supports_get_configuration =
298-
get_in(state.client_capabilities, [
299-
"workspace",
300-
"configuration"
301-
])
302-
303-
state =
304-
if supports_get_configuration do
305-
response = JsonRpc.get_configuration_request(state.root_uri, "elixirLS")
306-
307-
case response do
308-
{:ok, [result]} when is_map(result) ->
309-
Logger.info(
310-
"Received client configuration via workspace/configuration\n#{inspect(result)}"
311-
)
312-
313-
set_settings(state, result)
314-
315-
other ->
316-
Logger.error("Cannot get client configuration: #{inspect(other)}")
317-
state
318-
end
319-
else
320-
Logger.info("Client does not support workspace/configuration request")
321-
state
322-
end
297+
state = pull_configuration(state)
323298

324299
unless state.settings do
325300
# We still don't have the configuration. Let's wait for workspace/didChangeConfiguration
@@ -372,25 +347,32 @@ defmodule ElixirLS.LanguageServer.Server do
372347
end
373348

374349
# We don't start performing builds until we receive settings from the client in case they've set
375-
# the `projectDir` or `mixEnv` settings. If the settings don't match the format expected, leave
376-
# settings unchanged or set default settings if this is the first request.
350+
# the `projectDir` or `mixEnv` settings.
351+
# note that clients supporting `workspace/configuration` will send nil and we need to pull
377352
defp handle_notification(did_change_configuration(changed_settings), state = %__MODULE__{}) do
378-
Logger.info(
379-
"Received client configuration via workspace/didChangeConfiguration:\n#{inspect(changed_settings)}"
380-
)
381-
353+
Logger.info("Received workspace/didChangeConfiguration")
382354
prev_settings = state.settings || %{}
383355

384-
new_settings =
385-
case changed_settings do
386-
%{"elixirLS" => settings} when is_map(settings) ->
387-
Map.merge(prev_settings, settings)
356+
case changed_settings do
357+
%{"elixirLS" => settings} when is_map(settings) ->
358+
Logger.info(
359+
"Received client configuration via workspace/didChangeConfiguration\n#{inspect(settings)}"
360+
)
388361

389-
_ ->
390-
prev_settings
391-
end
362+
# deprecated push based configuration - interesting config updated
363+
new_settings = Map.merge(prev_settings, settings)
364+
set_settings(state, new_settings)
365+
366+
nil ->
367+
# pull based configuration
368+
# on notification the server should pull current configuration
369+
# see https://github.com/microsoft/language-server-protocol/issues/1792
370+
pull_configuration(state)
392371

393-
set_settings(state, new_settings)
372+
_ ->
373+
# deprecated push based configuration - some other config updated
374+
set_settings(state, prev_settings)
375+
end
394376
end
395377

396378
defp handle_notification(notification("exit"), state = %__MODULE__{}) do
@@ -1244,14 +1226,14 @@ defmodule ElixirLS.LanguageServer.Server do
12441226
end
12451227

12461228
defp listen_for_configuration_changes(server_instance_id) do
1247-
# the schema is not documented in official LSP docs
1248-
# using https://github.com/microsoft/vscode-languageserver-node/blob/7792b0b21c994cc9bebc3117eeb652a22e2d9e1f/protocol/src/common/protocol.ts#L1504C18-L1504C59
1229+
# the schema is not documented but uses https://github.com/microsoft/vscode-languageserver-node/blob/7792b0b21c994cc9bebc3117eeb652a22e2d9e1f/protocol/src/common/protocol.ts#L1504C18-L1504C59
1230+
# passing empty map without `section` results in notification with `{"settings": null}` config but the server should
1231+
# simply pull for configuration instead of depending on data sent in notification
1232+
# see https://github.com/microsoft/language-server-protocol/issues/1792
12491233
case JsonRpc.register_capability_request(
12501234
server_instance_id,
12511235
"workspace/didChangeConfiguration",
1252-
%{
1253-
"section" => "elixirLS"
1254-
}
1236+
%{}
12551237
) do
12561238
{:ok, nil} ->
12571239
Logger.info("client/registerCapability succeeded")
@@ -1440,4 +1422,32 @@ defmodule ElixirLS.LanguageServer.Server do
14401422
end
14411423
end
14421424
end
1425+
1426+
defp pull_configuration(state) do
1427+
supports_get_configuration =
1428+
get_in(state.client_capabilities, [
1429+
"workspace",
1430+
"configuration"
1431+
])
1432+
1433+
if supports_get_configuration do
1434+
response = JsonRpc.get_configuration_request(state.root_uri, "elixirLS")
1435+
1436+
case response do
1437+
{:ok, [result]} when is_map(result) ->
1438+
Logger.info(
1439+
"Received client configuration via workspace/configuration\n#{inspect(result)}"
1440+
)
1441+
1442+
set_settings(state, result)
1443+
1444+
other ->
1445+
Logger.error("Cannot get client configuration: #{inspect(other)}")
1446+
state
1447+
end
1448+
else
1449+
Logger.info("Client does not support workspace/configuration request")
1450+
state
1451+
end
1452+
end
14431453
end

‎apps/language_server/test/dialyzer_test.exs

Lines changed: 19 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
33

44
alias ElixirLS.LanguageServer.{Dialyzer, Server, Protocol, SourceFile, JsonRpc, Tracer, Build}
55
import ExUnit.CaptureLog
6-
alias ElixirLS.LanguageServer.Test.ServerTestHelpers
6+
import ElixirLS.LanguageServer.Test.ServerTestHelpers
77
use ElixirLS.Utils.MixTest.Case, async: false
88
use Protocol
99

@@ -22,7 +22,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
2222
end
2323

2424
setup do
25-
server = ServerTestHelpers.start_server()
25+
server = start_server()
2626
{:ok, _tracer} = start_supervised(Tracer)
2727

2828
{:ok, %{server: server}}
@@ -34,15 +34,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
3434
file_a = SourceFile.Path.to_uri(Path.absname("lib/a.ex"))
3535

3636
capture_log(fn ->
37-
root_uri = SourceFile.Path.to_uri(File.cwd!())
38-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
39-
40-
Server.receive_packet(
41-
server,
42-
did_change_configuration(%{
43-
"elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"}
44-
})
45-
)
37+
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"})
4638

4739
message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
4840

@@ -107,15 +99,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
10799
file_c = SourceFile.Path.to_uri(Path.absname("lib/c.ex"))
108100

109101
capture_log(fn ->
110-
root_uri = SourceFile.Path.to_uri(File.cwd!())
111-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
112-
113-
Server.receive_packet(
114-
server,
115-
did_change_configuration(%{
116-
"elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"}
117-
})
118-
)
102+
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"})
119103

120104
assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20_000
121105

@@ -168,15 +152,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
168152
file_a = SourceFile.Path.to_uri(Path.absname("lib/a.ex"))
169153

170154
capture_log(fn ->
171-
root_uri = SourceFile.Path.to_uri(File.cwd!())
172-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
173-
174-
Server.receive_packet(
175-
server,
176-
did_change_configuration(%{
177-
"elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"}
178-
})
179-
)
155+
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"})
180156

181157
message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
182158

@@ -222,15 +198,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
222198
file_a = SourceFile.Path.to_uri(Path.absname("lib/a.ex"))
223199

224200
capture_log(fn ->
225-
root_uri = SourceFile.Path.to_uri(File.cwd!())
226-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
227-
228-
Server.receive_packet(
229-
server,
230-
did_change_configuration(%{
231-
"elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_short"}
232-
})
233-
)
201+
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_short"})
234202

235203
message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
236204

@@ -262,20 +230,12 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
262230
end
263231

264232
@tag slow: true, fixture: true
265-
test "reports dialyzer_formatted error", %{server: server} do
233+
test "reports dialyzer formatted error", %{server: server} do
266234
in_fixture(__DIR__, "dialyzer", fn ->
267235
file_a = SourceFile.Path.to_uri(Path.absname("lib/a.ex"))
268236

269237
capture_log(fn ->
270-
root_uri = SourceFile.Path.to_uri(File.cwd!())
271-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
272-
273-
Server.receive_packet(
274-
server,
275-
did_change_configuration(%{
276-
"elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyzer"}
277-
})
278-
)
238+
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyzer"})
279239

280240
message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
281241

@@ -313,15 +273,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
313273
file_a = SourceFile.Path.to_uri(Path.absname("apps/app1/lib/app1.ex"))
314274

315275
capture_log(fn ->
316-
root_uri = SourceFile.Path.to_uri(File.cwd!())
317-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
318-
319-
Server.receive_packet(
320-
server,
321-
did_change_configuration(%{
322-
"elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_short"}
323-
})
324-
)
276+
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_short"})
325277

326278
message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
327279

@@ -357,13 +309,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
357309
file_a = SourceFile.Path.to_uri(Path.absname("lib/a.ex"))
358310

359311
capture_log(fn ->
360-
root_uri = SourceFile.Path.to_uri(File.cwd!())
361-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
362-
363-
Server.receive_packet(
364-
server,
365-
did_change_configuration(%{"elixirLS" => %{"dialyzerEnabled" => true}})
366-
)
312+
initialize(server, %{"dialyzerEnabled" => true})
367313

368314
assert_receive publish_diagnostics_notif(^file_a, [_, _]), 20000
369315

@@ -381,16 +327,9 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
381327
@tag slow: true, fixture: true
382328
test "protocol rebuild does not trigger consolidation warnings", %{server: server} do
383329
in_fixture(__DIR__, "protocols", fn ->
384-
root_uri = SourceFile.Path.to_uri(File.cwd!())
385330
uri = SourceFile.Path.to_uri(Path.absname("lib/implementations.ex"))
386331

387-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
388-
Server.receive_packet(server, notification("initialized"))
389-
390-
Server.receive_packet(
391-
server,
392-
did_change_configuration(%{"elixirLS" => %{"dialyzerEnabled" => true}})
393-
)
332+
initialize(server, %{"dialyzerEnabled" => true})
394333

395334
assert_receive notification("window/logMessage", %{"message" => "Compile took" <> _}), 5000
396335

@@ -458,15 +397,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
458397
file_c = SourceFile.Path.to_uri(Path.absname("lib/c.ex"))
459398

460399
capture_log(fn ->
461-
root_uri = SourceFile.Path.to_uri(File.cwd!())
462-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
463-
464-
Server.receive_packet(
465-
server,
466-
did_change_configuration(%{
467-
"elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"}
468-
})
469-
)
400+
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"})
470401

471402
message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
472403

@@ -495,19 +426,11 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
495426
file_c = SourceFile.Path.to_uri(Path.absname("lib/c.ex"))
496427

497428
capture_log(fn ->
498-
root_uri = SourceFile.Path.to_uri(File.cwd!())
499-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
500-
501-
Server.receive_packet(
502-
server,
503-
did_change_configuration(%{
504-
"elixirLS" => %{
505-
"dialyzerEnabled" => true,
506-
"dialyzerFormat" => "dialyxir_long",
507-
"suggestSpecs" => true
508-
}
509-
})
510-
)
429+
initialize(server, %{
430+
"dialyzerEnabled" => true,
431+
"dialyzerFormat" => "dialyxir_long",
432+
"suggestSpecs" => true
433+
})
511434

512435
message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
513436

@@ -555,7 +478,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
555478
)
556479

557480
assert_receive(%{
558-
"id" => 1,
481+
"id" => id,
559482
"method" => "workspace/applyEdit",
560483
"params" => %{
561484
"edit" => %{
@@ -575,19 +498,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
575498
}
576499
})
577500

578-
# TODO something is broken in packet capture
579-
# using JsonRpc.receive_packet causes the packet to be delivered to LanguageServer
580-
# which crashes with no match error
581-
# JsonRpc.receive_packet(
582-
# server,
583-
# response(1, %{"applied" => true})
584-
# )
585-
# instead we fake a callback in JsonRpc server that forwards the response as needed
586-
JsonRpc.handle_call(
587-
{:packet, response(1, %{"applied" => true})},
588-
nil,
589-
:sys.get_state(JsonRpc)
590-
)
501+
JsonRpc.receive_packet(response(id, %{"applied" => true}))
591502

592503
assert_receive(%{"id" => 4, "result" => nil}, 5000)
593504
end)

‎apps/language_server/test/providers/execute_command/mix_clean_test.exs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,20 @@
11
defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.MixCleanTest do
22
alias ElixirLS.LanguageServer.{Server, Protocol, SourceFile, Tracer}
33
use ElixirLS.Utils.MixTest.Case, async: false
4+
import ElixirLS.LanguageServer.Test.ServerTestHelpers
45
use Protocol
56

67
setup do
78
{:ok, _} = start_supervised(Tracer)
8-
server = ElixirLS.LanguageServer.Test.ServerTestHelpers.start_server()
9+
server = start_server()
910

1011
{:ok, %{server: server}}
1112
end
1213

1314
@tag fixture: true
1415
test "mix clean", %{server: server} do
1516
in_fixture(Path.join(__DIR__, "../.."), "clean", fn ->
16-
root_uri = SourceFile.Path.to_uri(File.cwd!())
17-
Server.receive_packet(server, initialize_req(1, root_uri, %{}))
18-
19-
Server.receive_packet(
20-
server,
21-
did_change_configuration(%{
22-
"elixirLS" => %{"dialyzerEnabled" => false}
23-
})
24-
)
17+
initialize(server)
2518

2619
assert_receive %{
2720
"method" => "window/logMessage",

0 commit comments

Comments
 (0)