Skip to content

Commit

Permalink
Emit parser warnings and errors on type (#986)
Browse files Browse the repository at this point in the history
* remove not needed monitor

* emit diagnostics from parser on edit

* react to closing and opening

* only parse ex and exs

* handle eex files

* use with_diagnostics

* use common error handling

handle unexpected errors

* fix tests

* add tests

* fix paths on windows

* don't assert on error message

* eex extension

* don't assert warnings on elixir < 1.15.3

* remove addressed todos
  • Loading branch information
lukaszsamson committed Sep 25, 2023
1 parent 02d203d commit 7470e5d
Show file tree
Hide file tree
Showing 4 changed files with 434 additions and 42 deletions.
154 changes: 141 additions & 13 deletions apps/language_server/lib/language_server/server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ defmodule ElixirLS.LanguageServer.Server do
:root_uri,
:project_dir,
:settings,
parse_file_refs: %{},
parser_diagnostics: %{},
build_diagnostics: [],
dialyzer_diagnostics: [],
needs_build?: false,
Expand Down Expand Up @@ -270,6 +272,47 @@ defmodule ElixirLS.LanguageServer.Server do
{:noreply, state}
end

@impl GenServer
def handle_info(
{:parse_file, uri},
%__MODULE__{source_files: source_files} = state
) do
old_diagnostics =
state.build_diagnostics ++
state.dialyzer_diagnostics ++ List.flatten(Map.values(state.parser_diagnostics))

parser_diagnostics =
case source_files[uri] do
%SourceFile{} = source_file ->
file = SourceFile.Path.from_uri(uri)
case parse_file(source_file.text, file) do
[] ->
Map.delete(state.parser_diagnostics, uri)

diagnostics ->
Map.put(state.parser_diagnostics, uri, diagnostics)
end

nil ->
Map.delete(state.parser_diagnostics, uri)
end

state = %{
state
| parser_diagnostics: parser_diagnostics,
parse_file_refs: Map.delete(state.parse_file_refs, uri)
}

publish_diagnostics(
state.build_diagnostics ++
state.dialyzer_diagnostics ++ List.flatten(Map.values(state.parser_diagnostics)),
old_diagnostics,
state.source_files
)

{:noreply, state}
end

## Helpers

defp handle_notification(notification("initialized"), state = %__MODULE__{}) do
Expand Down Expand Up @@ -381,13 +424,9 @@ defmodule ElixirLS.LanguageServer.Server do
else
source_file = %SourceFile{text: text, version: version}

Diagnostics.publish_file_diagnostics(
uri,
state.build_diagnostics ++ state.dialyzer_diagnostics,
source_file
)

put_in(state.source_files[uri], source_file)
state = put_in(state.source_files[uri], source_file)
# parse handler will emit diagnostics for opened file
trigger_parse(state, uri, 0)
end
end

Expand All @@ -402,6 +441,9 @@ defmodule ElixirLS.LanguageServer.Server do
else
awaiting_contracts = reject_awaiting_contracts(state.awaiting_contracts, uri)

# parse handler will clean up diagnostics and refs for closed file
state = trigger_parse(state, uri, 0)

%{
state
| source_files: Map.delete(state.source_files, uri),
Expand All @@ -421,10 +463,14 @@ defmodule ElixirLS.LanguageServer.Server do

state
else
update_in(state.source_files[uri], fn source_file ->
%SourceFile{source_file | version: version, dirty?: true}
|> SourceFile.apply_content_changes(content_changes)
end)
state =
update_in(state.source_files[uri], fn source_file ->
%SourceFile{source_file | version: version, dirty?: true}
|> SourceFile.apply_content_changes(content_changes)
end)

# trigger parse with debounce
trigger_parse(state, uri, 300)
end
end

Expand Down Expand Up @@ -1039,7 +1085,10 @@ defmodule ElixirLS.LanguageServer.Server do
end

defp handle_build_result(status, diagnostics, state = %__MODULE__{}) do
old_diagnostics = state.build_diagnostics ++ state.dialyzer_diagnostics
old_diagnostics =
state.build_diagnostics ++
state.dialyzer_diagnostics ++ List.flatten(Map.values(state.parser_diagnostics))

state = put_in(state.build_diagnostics, diagnostics)

state =
Expand All @@ -1055,7 +1104,8 @@ defmodule ElixirLS.LanguageServer.Server do
end

publish_diagnostics(
state.build_diagnostics ++ state.dialyzer_diagnostics,
state.build_diagnostics ++
state.dialyzer_diagnostics ++ List.flatten(Map.values(state.parser_diagnostics)),
old_diagnostics,
state.source_files
)
Expand Down Expand Up @@ -1432,4 +1482,82 @@ defmodule ElixirLS.LanguageServer.Server do
state
end
end

defp trigger_parse(state, uri, debounce_timeout) do
if String.ends_with?(uri, [".ex", ".exs", ".eex"]) do
update_in(state.parse_file_refs[uri], fn old_ref ->
if old_ref do
Process.cancel_timer(old_ref, info: false)
end

Process.send_after(self(), {:parse_file, uri}, debounce_timeout)
end)
else
state
end
end

defp parse_file(text, file) do
{result, raw_diagnostics} =
Build.with_diagnostics([log: false], fn ->
try do
parser_options = [
file: file,
columns: true
]

if String.ends_with?(file, ".eex") do
EEx.compile_string(text,
file: file,
parser_options: parser_options
)
else
Code.string_to_quoted!(text, parser_options)
end

:ok
rescue
e in [EEx.SyntaxError, SyntaxError, TokenMissingError] ->
message = Exception.message(e)

diagnostic = %Mix.Task.Compiler.Diagnostic{
compiler_name: "ElixirLS",
file: file,
position: {e.line, e.column},
message: message,
severity: :error
}

{:error, diagnostic}

e ->
message = Exception.message(e)

diagnostic = %Mix.Task.Compiler.Diagnostic{
compiler_name: "ElixirLS",
file: file,
position: {1, 1},
message: message,
severity: :error
}

# e.g. https://github.com/elixir-lang/elixir/issues/12926
Logger.warning(
"Unexpected parser error, please report it to elixir project https://github.com/elixir-lang/elixir/issues\n" <>
Exception.format(:error, e, __STACKTRACE__)
)

{:error, diagnostic}
end
end)

warning_diagnostics =
raw_diagnostics
|> Enum.map(&Diagnostics.code_diagnostic/1)

case result do
:ok -> warning_diagnostics
{:error, diagnostic} -> [diagnostic | warning_diagnostics]
end
end
end
2 changes: 1 addition & 1 deletion apps/language_server/mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ defmodule ElixirLS.LanguageServer.Mixfile do
end

def application do
[mod: {ElixirLS.LanguageServer, []}, extra_applications: [:logger]]
[mod: {ElixirLS.LanguageServer, []}, extra_applications: [:logger, :eex]]
end

defp deps do
Expand Down
7 changes: 0 additions & 7 deletions apps/language_server/test/dialyzer_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
@tag slow: true, fixture: true
test "only analyzes the changed files", %{server: server} do
in_fixture(__DIR__, "dialyzer", fn ->
file_c = SourceFile.Path.to_uri(Path.absname("lib/c.ex"))

capture_log(fn ->
initialize(server, %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"})

Expand Down Expand Up @@ -149,8 +147,6 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
}),
3_000

assert_receive publish_diagnostics_notif(^file_c, []), 20_000

assert_receive notification("window/logMessage", %{
"message" => "[ElixirLS Dialyzer] Done writing manifest" <> _
}),
Expand Down Expand Up @@ -376,9 +372,6 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do

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

assert_receive notification("textDocument/publishDiagnostics", %{"diagnostics" => []}),
30000

Process.sleep(2000)

v2_text = """
Expand Down
Loading

0 comments on commit 7470e5d

Please sign in to comment.