Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix numerous cases of incorrect utf16 positions returned and passed into elixir_sense #677

Merged
merged 2 commits into from
Feb 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions apps/language_server/lib/language_server/build.ex
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ defmodule ElixirLS.LanguageServer.Build do
defp range(position, nil) when is_integer(position) do
line = position - 1

# we don't care about utf16 positions here as we send 0
%{
"start" => %{"line" => line, "character" => 0},
"end" => %{"line" => line, "character" => 0}
Expand All @@ -290,23 +291,24 @@ defmodule ElixirLS.LanguageServer.Build do
defp range(position, source_file) when is_integer(position) do
line = position - 1
text = Enum.at(SourceFile.lines(source_file), line) || ""
start_idx = String.length(text) - String.length(String.trim_leading(text))
length = Enum.max([String.length(String.trim(text)), 1])

%{
"start" => %{"line" => line, "character" => start_idx},
"end" => %{"line" => line, "character" => start_idx + length}
}
end
start_idx = String.length(text) - String.length(String.trim_leading(text)) + 1
length = max(String.length(String.trim(text)), 1)

defp range({start_line, start_col, end_line, end_col}, _) do
%{
"start" => %{"line" => start_line - 1, "character" => start_col},
"end" => %{"line" => end_line - 1, "character" => end_col}
"start" => %{
"line" => line,
"character" => SourceFile.elixir_character_to_lsp(text, start_idx)
},
"end" => %{
"line" => line,
"character" => SourceFile.elixir_character_to_lsp(text, start_idx + length)
}
}
end

defp range(_, nil) do
# we don't care about utf16 positions here as we send 0
%{"start" => %{"line" => 0, "character" => 0}, "end" => %{"line" => 0, "character" => 0}}
end

Expand Down
26 changes: 15 additions & 11 deletions apps/language_server/lib/language_server/protocol/location.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,30 @@ defmodule ElixirLS.LanguageServer.Protocol.Location do
defstruct [:uri, :range]

alias ElixirLS.LanguageServer.SourceFile
alias ElixirLS.LanguageServer.Protocol
require ElixirLS.LanguageServer.Protocol, as: Protocol

def new(%ElixirSense.Location{file: file, line: line, column: column}, uri) do
def new(
%ElixirSense.Location{file: file, line: line, column: column},
current_file_uri,
current_file_text
) do
uri =
case file do
nil -> uri
nil -> current_file_uri
_ -> SourceFile.path_to_uri(file)
end

# LSP messages are 0 indexed whilst elixir/erlang is 1 indexed.
# Guard against malformed line or column values.
line = max(line - 1, 0)
column = max(column - 1, 0)
text =
case file do
nil -> current_file_text
file -> File.read!(file)
end

{line, column} = SourceFile.elixir_position_to_lsp(text, {line, column})

%Protocol.Location{
uri: uri,
range: %{
"start" => %{"line" => line, "character" => column},
"end" => %{"line" => line, "character" => column}
}
range: Protocol.range(line, column, line, column)
}
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens do

def build_code_lens(line, title, command, argument) do
%{
# we don't care about utf16 positions here as we send 0
"range" => range(line - 1, 0, line - 1, 0),
"command" => %{
"title" => title,
Expand Down
14 changes: 9 additions & 5 deletions apps/language_server/lib/language_server/providers/completion.ex
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,20 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
|> SourceFile.lines()
|> Enum.at(line)

text_before_cursor = String.slice(line_text, 0, character)
text_after_cursor = String.slice(line_text, character..-1)
# convert to 1 based utf8 position
line = line + 1
character = SourceFile.lsp_character_to_elixir(line_text, character)

text_before_cursor = String.slice(line_text, 0, character - 1)
text_after_cursor = String.slice(line_text, (character - 1)..-1)

prefix = get_prefix(text_before_cursor)

# TODO: Don't call into here directly
# Can we use ElixirSense.Providers.Suggestion? ElixirSense.suggestions/3
env =
ElixirSense.Core.Parser.parse_string(text, true, true, line + 1)
|> ElixirSense.Core.Metadata.get_env(line + 1)
ElixirSense.Core.Parser.parse_string(text, true, true, line)
|> ElixirSense.Core.Metadata.get_env(line)

scope =
case env.scope do
Expand Down Expand Up @@ -135,7 +139,7 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
}

items =
ElixirSense.suggestions(text, line + 1, character + 1)
ElixirSense.suggestions(text, line, character)
|> maybe_reject_derived_functions(context, options)
|> Enum.map(&from_completion_item(&1, context, options))
|> maybe_add_do(context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@ defmodule ElixirLS.LanguageServer.Providers.Definition do
Go-to-definition provider utilizing Elixir Sense
"""

alias ElixirLS.LanguageServer.Protocol
alias ElixirLS.LanguageServer.{Protocol, SourceFile}

def definition(uri, text, line, character) do
{line, character} = SourceFile.lsp_position_to_elixr(text, {line, character})

result =
case ElixirSense.definition(text, line + 1, character + 1) do
case ElixirSense.definition(text, line, character) do
nil ->
nil

%ElixirSense.Location{} = location ->
Protocol.Location.new(location, uri)
Protocol.Location.new(location, uri, text)
end

{:ok, result}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
"""

alias ElixirLS.LanguageServer.Providers.SymbolUtils
alias ElixirLS.LanguageServer.Protocol
alias ElixirLS.LanguageServer.SourceFile
require ElixirLS.LanguageServer.Protocol, as: Protocol

defmodule Info do
defstruct [:type, :name, :location, :children]
Expand All @@ -25,22 +26,22 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
def symbols(uri, text, hierarchical) do
case list_symbols(text) do
{:ok, symbols} ->
{:ok, build_symbols(symbols, uri, hierarchical)}
{:ok, build_symbols(symbols, uri, text, hierarchical)}

{:error, :compilation_error} ->
{:error, :server_error, "[DocumentSymbols] Compilation error while parsing source file"}
end
end

defp build_symbols(symbols, uri, hierarchical)
defp build_symbols(symbols, uri, text, hierarchical)

defp build_symbols(symbols, uri, true) do
Enum.map(symbols, &build_symbol_information_hierarchical(uri, &1))
defp build_symbols(symbols, uri, text, true) do
Enum.map(symbols, &build_symbol_information_hierarchical(uri, text, &1))
end

defp build_symbols(symbols, uri, false) do
defp build_symbols(symbols, uri, text, false) do
symbols
|> Enum.map(&build_symbol_information_flat(uri, &1))
|> Enum.map(&build_symbol_information_flat(uri, text, &1))
|> List.flatten()
end

Expand Down Expand Up @@ -283,25 +284,27 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do

defp extract_symbol(_, _), do: nil

defp build_symbol_information_hierarchical(uri, info) when is_list(info),
do: Enum.map(info, &build_symbol_information_hierarchical(uri, &1))
defp build_symbol_information_hierarchical(uri, text, info) when is_list(info),
do: Enum.map(info, &build_symbol_information_hierarchical(uri, text, &1))

defp build_symbol_information_hierarchical(uri, text, %Info{} = info) do
range = location_to_range(info.location, text)

defp build_symbol_information_hierarchical(uri, %Info{} = info) do
%Protocol.DocumentSymbol{
name: info.name,
kind: SymbolUtils.symbol_kind_to_code(info.type),
range: location_to_range(info.location),
selectionRange: location_to_range(info.location),
children: build_symbol_information_hierarchical(uri, info.children)
range: range,
selectionRange: range,
children: build_symbol_information_hierarchical(uri, text, info.children)
}
end

defp build_symbol_information_flat(uri, info, parent_name \\ nil)
defp build_symbol_information_flat(uri, text, info, parent_name \\ nil)

defp build_symbol_information_flat(uri, info, parent_name) when is_list(info),
do: Enum.map(info, &build_symbol_information_flat(uri, &1, parent_name))
defp build_symbol_information_flat(uri, text, info, parent_name) when is_list(info),
do: Enum.map(info, &build_symbol_information_flat(uri, text, &1, parent_name))

defp build_symbol_information_flat(uri, %Info{} = info, parent_name) do
defp build_symbol_information_flat(uri, text, %Info{} = info, parent_name) do
case info.children do
[_ | _] ->
[
Expand All @@ -310,11 +313,11 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
kind: SymbolUtils.symbol_kind_to_code(info.type),
location: %{
uri: uri,
range: location_to_range(info.location)
range: location_to_range(info.location, text)
},
containerName: parent_name
}
| Enum.map(info.children, &build_symbol_information_flat(uri, &1, info.name))
| Enum.map(info.children, &build_symbol_information_flat(uri, text, &1, info.name))
]

_ ->
Expand All @@ -323,18 +326,18 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
kind: SymbolUtils.symbol_kind_to_code(info.type),
location: %{
uri: uri,
range: location_to_range(info.location)
range: location_to_range(info.location, text)
},
containerName: parent_name
}
end
end

defp location_to_range(location) do
%{
start: %{line: location[:line] - 1, character: location[:column] - 1},
end: %{line: location[:line] - 1, character: location[:column] - 1}
}
defp location_to_range(location, text) do
{line, character} =
SourceFile.elixir_position_to_lsp(text, {location[:line], location[:column]})

Protocol.range(line, character, line, character)
end

defp extract_module_name(protocol: protocol, implementations: implementations) do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ApplySpec do
"label" => "Add @spec to #{mod}.#{fun}/#{arity}",
"edit" => %{
"changes" => %{
# we don't care about utf16 positions here as we send 0
uri => [%{"range" => range(line - 1, 0, line - 1, 0), "newText" => formatted}]
}
}
Expand Down
19 changes: 12 additions & 7 deletions apps/language_server/lib/language_server/providers/hover.ex
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
defmodule ElixirLS.LanguageServer.Providers.Hover do
alias ElixirLS.LanguageServer.SourceFile
import ElixirLS.LanguageServer.Protocol

@moduledoc """
Hover provider utilizing Elixir Sense
Expand All @@ -17,14 +18,16 @@ defmodule ElixirLS.LanguageServer.Providers.Hover do
|> Enum.map(fn x -> "lib/#{x}/lib" end)

def hover(text, line, character, project_dir) do
{line, character} = SourceFile.lsp_position_to_elixr(text, {line, character})

response =
case ElixirSense.docs(text, line + 1, character + 1) do
case ElixirSense.docs(text, line, character) do
%{subject: ""} ->
nil

%{subject: subject, docs: docs} ->
line_text = Enum.at(SourceFile.lines(text), line)
range = highlight_range(line_text, line, character, subject)
line_text = Enum.at(SourceFile.lines(text), line - 1)
range = highlight_range(line_text, line - 1, character - 1, subject)

%{"contents" => contents(docs, subject, project_dir), "range" => range}
end
Expand All @@ -45,10 +48,12 @@ defmodule ElixirLS.LanguageServer.Providers.Hover do

Enum.find_value(regex_ranges, fn
[{start, length}] when start <= character and character <= start + length ->
%{
"start" => %{"line" => line, "character" => start},
"end" => %{"line" => line, "character" => start + length}
}
range(
line,
SourceFile.elixir_character_to_lsp(line_text, start + 1),
line,
SourceFile.elixir_character_to_lsp(line_text, start + 1 + length)
)

_ ->
nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ defmodule ElixirLS.LanguageServer.Providers.Implementation do
Go-to-implementation provider utilizing Elixir Sense
"""

alias ElixirLS.LanguageServer.Protocol
alias ElixirLS.LanguageServer.{Protocol, SourceFile}

def implementation(uri, text, line, character) do
locations = ElixirSense.implementations(text, line + 1, character + 1)
results = for location <- locations, do: Protocol.Location.new(location, uri)
{line, character} = SourceFile.lsp_position_to_elixr(text, {line, character})
locations = ElixirSense.implementations(text, line, character)
results = for location <- locations, do: Protocol.Location.new(location, uri, text)

{:ok, results}
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ defmodule ElixirLS.LanguageServer.Providers.OnTypeFormatting do
alias ElixirLS.LanguageServer.SourceFile
import ElixirLS.LanguageServer.Protocol

def format(%SourceFile{} = source_file, line, character, "\n", _options) do
def format(%SourceFile{} = source_file, line, character, "\n", _options) when line >= 1 do
# we don't care about utf16 positions here as we only pass character back to client
lines = SourceFile.lines(source_file)
prev_line = Enum.at(lines, line - 1)

Expand Down Expand Up @@ -69,6 +70,7 @@ defmodule ElixirLS.LanguageServer.Providers.OnTypeFormatting do
# In VS Code, currently, the cursor jumps strangely if the current line is blank and we try to
# insert a newline at the current position, so unfortunately, we have to check for that.
defp insert_end_edit(indentation, line, character, insert_on_next_line?) do
# we don't care about utf16 positions here as we either use 0 or what the client sent
if insert_on_next_line? do
{range(line + 1, 0, line + 1, 0), "#{indentation}end\n"}
else
Expand Down
Loading