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

Bump sourceror dependency to v0.12.2 #841

Closed
wants to merge 3 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,21 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.Ast do
| {any(), any()}
| {atom() | {any(), [any()], atom() | [any()]}, Keyword.t(), atom() | [any()]}

@spec from(source) :: t
def from(%SourceFile{} = source_file) do
def from(source_file, opts \\ [])

@spec from(source, keyword()) :: t
def from(%SourceFile{} = source_file, opts) do
source_file
|> SourceFile.to_string()
|> from()
|> from(opts)
end

def from(s) when is_binary(s) do
ElixirSense.string_to_quoted(s, 1, 6, token_metadata: true)
def from(s, opts) when is_binary(s) do
if opts[:include_comments] do
Sourceror.parse_string(s)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does sourceror fix errors like ElixirSense does?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. I'm not familar with how ElixirSense fixes errors. Do you have examples or tests that check for this? We could extend those to test Sourceror's behaviour.

else
ElixirSense.string_to_quoted(s, 1, 6, token_metadata: true)
end
end

@spec to_string(t()) :: String.t()
Expand Down
3 changes: 2 additions & 1 deletion apps/language_server/mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ defmodule ElixirLS.LanguageServer.Mixfile do
{:elixir_ls_utils, in_umbrella: true},
{:elixir_sense, github: "elixir-lsp/elixir_sense"},
{:erl2ex, github: "dazuma/erl2ex"},
{:sourceror, "0.11.2"},
# use tag to ensure sourceror/lib_vendored is included
{:sourceror, github: "doorgan/sourceror", tag: "v0.12.2"},
{:dialyxir_vendored, github: "elixir-lsp/dialyxir", branch: "vendored", runtime: false},
{:jason_vendored, github: "elixir-lsp/jason", branch: "vendored"},
{:stream_data, "~> 0.5", only: [:dev, :test], runtime: false},
Expand Down
86 changes: 86 additions & 0 deletions apps/language_server/test/experimental/code_mod/ast_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
defmodule ElixirLS.LanguageServer.Experimental.CodeMod.AstTest do
alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast

use ExUnit.Case

describe "ast" do
test "from\2 / to_string\1" do
string = """
defmodule Bar do
def foo(baz) do
# TODO
end
end
"""

{:ok, ast} = Ast.from(string, include_comments: true)
assert string == Ast.to_string(ast) <> "\n"

{:ok, ast_missing_comments} = Ast.from(string)

assert """
defmodule Bar do
def foo(baz) do
end
end
""" == Ast.to_string(ast_missing_comments) <> "\n"
end

test "from\1" do
assert {:ok,
{:def, [do: [line: 1, column: 16], end: [line: 3, column: 3], line: 1, column: 3],
[
{:foo, [closing: [line: 1, column: 14], line: 1, column: 7],
[{:baz, [line: 1, column: 11], nil}]},
[do: {:__block__, [], []}]
]}} =
Ast.from("""
def foo(baz) do
# TODO
end
""")
Comment on lines +29 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these assertions are pretty hard core, and are really testing the calls to elixir sense / sourceror

Perhaps instead, just ensure we're calling the two libs correctly in each case by using Patch?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sourceror github page says to expect "frequent breaking changes".

Perhaps we leave the assertion of the returned AST structure -- at least for the Ast.from(_, include_comments: true) call -- to catch any breaking changes?

I depend on the AST returned by sourceror for the extract function refactoring code I'm writing in a separate branch.

end

test "from\2" do
assert {:ok,
{:def,
[
trailing_comments: [
%{column: 5, line: 2, next_eol_count: 1, previous_eol_count: 1, text: "# TODO"}
],
leading_comments: [],
do: [line: 1, column: 16],
end: [line: 3, column: 3],
line: 1,
column: 3
],
[
{:foo,
[
trailing_comments: [],
leading_comments: [],
closing: [line: 1, column: 14],
line: 1,
column: 7
],
[
{:baz, [trailing_comments: [], leading_comments: [], line: 1, column: 11],
nil}
]},
[
{{:__block__,
[trailing_comments: [], leading_comments: [], line: 1, column: 16], [:do]},
{:__block__, [trailing_comments: [], leading_comments: []], []}}
]
]}} =
Ast.from(
"""
def foo(baz) do
# TODO
end
""",
include_comments: true
)
end
end
end
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ defmodule ElixirLS.Mixfile do

defp aliases do
[
test: "cmd mix test"
test: "cmd mix test --color"
]
end
end
2 changes: 1 addition & 1 deletion mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"nimble_parsec": {:hex, :nimble_parsec, "1.1.0", "3a6fca1550363552e54c216debb6a9e95bd8d32348938e13de5eda962c0d7f89", [:mix], [], "hexpm", "08eb32d66b706e913ff748f11694b17981c0b04a33ef470e33e11b3d3ac8f54b"},
"patch": {:hex, :patch, "0.12.0", "2da8967d382bade20344a3e89d618bfba563b12d4ac93955468e830777f816b0", [:mix], [], "hexpm", "ffd0e9a7f2ad5054f37af84067ee88b1ad337308a1cb227e181e3967127b0235"},
"path_glob_vendored": {:git, "https://github.com/elixir-lsp/path_glob.git", "965350dc41def7be4a70a23904195c733a2ecc84", [branch: "vendored"]},
"sourceror": {:hex, :sourceror, "0.11.2", "549ce48be666421ac60cfb7f59c8752e0d393baa0b14d06271d3f6a8c1b027ab", [:mix], [], "hexpm", "9ab659118896a36be6eec68ff7b0674cba372fc8e210b1e9dc8cf2b55bb70dfb"},
"sourceror": {:git, "https://github.com/doorgan/sourceror.git", "c3fa0c4a6e2915fdd7a9ea9983ca762a7d196b21", [tag: "v0.12.2"]},
"statistex": {:hex, :statistex, "1.0.0", "f3dc93f3c0c6c92e5f291704cf62b99b553253d7969e9a5fa713e5481cd858a5", [:mix], [], "hexpm", "ff9d8bee7035028ab4742ff52fc80a2aa35cece833cf5319009b52f1b5a86c27"},
"stream_data": {:hex, :stream_data, "0.5.0", "b27641e58941685c75b353577dc602c9d2c12292dd84babf506c2033cd97893e", [:mix], [], "hexpm", "012bd2eec069ada4db3411f9115ccafa38540a3c78c4c0349f151fc761b9e271"},
}