From 6fd420abdb8715817d3678741f0a0b462666c6c8 Mon Sep 17 00:00:00 2001 From: Zach Allaun Date: Tue, 23 Jul 2024 16:47:34 -0400 Subject: [PATCH 1/4] Strip top-level calls from .exs files before compilation --- .../build/document/compilers/elixir.ex | 5 ++- .../build/document/compilers/quoted.ex | 42 +++++++++++++++++++ .../lexical/remote_control/build_test.exs | 2 +- .../lexical_shared/lib/lexical/document.ex | 9 +++- 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/elixir.ex b/apps/remote_control/lib/lexical/remote_control/build/document/compilers/elixir.ex index 36a62d074..e53ab498c 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/elixir.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/document/compilers/elixir.ex @@ -11,8 +11,9 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.Elixir do @behaviour Build.Document.Compiler @impl true - def recognizes?(%Document{language_id: "elixir"}), do: true - def recognizes?(_), do: false + def recognizes?(%Document{} = doc) do + doc.language_id in ["elixir", "elixir-script"] + end @impl true def enabled?, do: true diff --git a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex b/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex index ae416c681..7e7a63529 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex @@ -9,6 +9,13 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.Quoted do def compile(%Document{} = document, quoted_ast, compiler_name) do prepare_compile(document.path) + quoted_ast = + if document.language_id == "elixir-script" do + wrap_top_level_forms(quoted_ast) + else + quoted_ast + end + {status, diagnostics} = if Features.with_diagnostics?() do do_compile(quoted_ast, document) @@ -130,4 +137,39 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.Quoted do defp replace_source(result, source) do Map.put(result, :source, source) end + + defp wrap_top_level_forms({:__block__, meta, nodes}) do + chunks = + nodes + |> Enum.chunk_by(&should_wrap?/1) + |> Enum.with_index(fn [node | _] = nodes, i -> + if should_wrap?(node) do + wrap_nodes(nodes, i) + else + nodes + end + end) + + {:__block__, meta, chunks} + end + + defp wrap_top_level_forms(ast) do + wrap_top_level_forms({:__block__, [], [ast]}) + end + + defp wrap_nodes(nodes, i) do + module_name = :"lexical_wrapper_#{i}" + + quote do + defmodule unquote(module_name) do + def __lexical_wrapper__ do + (unquote_splicing(nodes)) + end + end + end + end + + @allowed_top_level [:defmodule, :alias, :import, :require, :use] + defp should_wrap?({allowed, _, _}) when allowed in @allowed_top_level, do: false + defp should_wrap?(_), do: true end diff --git a/apps/remote_control/test/lexical/remote_control/build_test.exs b/apps/remote_control/test/lexical/remote_control/build_test.exs index bdca695b5..98c01fc51 100644 --- a/apps/remote_control/test/lexical/remote_control/build_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build_test.exs @@ -24,7 +24,7 @@ defmodule Lexical.BuildTest do project |> Project.root_path() |> Path.join(to_string(sequence)) - |> Path.join("file.exs") + |> Path.join("file.ex") |> Document.Path.to_uri() end diff --git a/projects/lexical_shared/lib/lexical/document.ex b/projects/lexical_shared/lib/lexical/document.ex index c3afc64ab..3d5abe94c 100644 --- a/projects/lexical_shared/lib/lexical/document.ex +++ b/projects/lexical_shared/lib/lexical/document.ex @@ -48,7 +48,12 @@ defmodule Lexical.Document do uri = DocumentPath.ensure_uri(maybe_uri) path = DocumentPath.from_uri(uri) - language_id = language_id || language_id_from_path(path) + language_id = + if String.ends_with?(path, ".exs") do + "elixir-script" + else + language_id || language_id_from_path(path) + end %__MODULE__{ uri: uri, @@ -233,7 +238,7 @@ defmodule Lexical.Document do "elixir" ".exs" -> - "elixir" + "elixir-script" ".eex" -> "eex" From 9c3a3c6dd0aabaef37c47c0fb5ad84c479d7f2c2 Mon Sep 17 00:00:00 2001 From: Zach Allaun Date: Thu, 25 Jul 2024 09:58:52 -0400 Subject: [PATCH 2/4] Add test case --- .../lexical/remote_control/build_test.exs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/apps/remote_control/test/lexical/remote_control/build_test.exs b/apps/remote_control/test/lexical/remote_control/build_test.exs index 98c01fc51..5e35c7d22 100644 --- a/apps/remote_control/test/lexical/remote_control/build_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build_test.exs @@ -647,6 +647,30 @@ defmodule Lexical.BuildTest do end end + describe ".exs files" do + setup do + start_supervised!(RemoteControl.Dispatch) + start_supervised!(RemoteControl.ModuleMappings) + start_supervised!(Build.CaptureServer) + :ok + end + + test "should not run top-level forms" do + source = ~S[ + IO.puts("fail") + ] + + doc = Document.new("file:///file.exs", source, 0) + + captured = + ExUnit.CaptureIO.capture_io(fn -> + Build.Document.compile(doc) + end) + + refute captured =~ "fail" + end + end + if Features.after_verify?() do describe "exceptions during compilation" do test "compiling a project with callback errors" do From bc080e19bb8f857b5aed5f57789472ac09181c38 Mon Sep 17 00:00:00 2001 From: Zach Allaun Date: Thu, 25 Jul 2024 12:09:58 -0400 Subject: [PATCH 3/4] Fix incorrect unused/undefined var warnings and errors in .exs files --- apps/common/lib/lexical/ast.ex | 60 ++++++++ .../build/document/compilers/quoted.ex | 133 +++++++++++++++-- .../build/document/compilers/quoted_test.exs | 137 ++++++++++++++++++ 3 files changed, 317 insertions(+), 13 deletions(-) create mode 100644 apps/remote_control/test/lexical/remote_control/build/document/compilers/quoted_test.exs diff --git a/apps/common/lib/lexical/ast.ex b/apps/common/lib/lexical/ast.ex index 46f19d606..08bbf5ad1 100644 --- a/apps/common/lib/lexical/ast.ex +++ b/apps/common/lib/lexical/ast.ex @@ -414,6 +414,66 @@ defmodule Lexical.Ast do {:ok, List.wrap(current_module)} end + @doc """ + Walks a quoted expression with an accumulator, applying `fun` to each + var or pinned var. + + Returns a tuple where the first element is the potentially modified + expression and the second is the accumulator. + """ + # Adapted from `ExUnit.Assertions.collect_vars_from_pattern/1`, + # licensed under Apache License 2.0: + # https://github.com/elixir-lang/elixir/blob/1e914b04b46125b3b9b251b64ee04380e523afc4/lib/ex_unit/lib/ex_unit/assertions.ex#L657 + @spec prewalk_vars(Macro.t(), acc, (Macro.t(), acc -> {Macro.t(), acc})) :: {Macro.t(), acc} + when acc: term() + def prewalk_vars(ast, acc, fun) do + {ast, {acc, _}} = + Macro.prewalk(ast, {acc, false}, fn + {:"::", meta, [left, right]}, {acc, _prev_pinned?} -> + {right, acc} = prewalk_vars_in_binary(right, acc, fun) + {{:"::", meta, [left, right]}, {acc, false}} + + {skip, _, [_]} = node, {acc, _prev_pinned?} when skip in [:@, :quote] -> + {node, {acc, false}} + + {:_, _, context} = node, {acc, _prev_pinned?} when is_atom(context) -> + {node, {acc, false}} + + {:^, _, [{name, _, context}]} = pinned, {acc, _prev_pinned?} + when is_atom(name) and is_atom(context) -> + {pinned, acc} = fun.(pinned, acc) + {pinned, {acc, true}} + + {name, _, context} = var, {acc, false} when is_atom(name) and is_atom(context) -> + {var, acc} = fun.(var, acc) + {var, {acc, false}} + + node, {acc, _prev_pinned?} -> + {node, {acc, false}} + end) + + {ast, acc} + end + + defp prewalk_vars_in_binary(right, acc, fun) do + Macro.prewalk(right, acc, fn + {mode, mode_meta, [{name, _, context} = var]}, acc + when is_atom(mode) and is_atom(name) and is_atom(context) -> + {var, acc} = fun.(var, acc) + {{mode, mode_meta, [var]}, acc} + + node, acc -> + {node, acc} + end) + end + + @doc """ + Returns whether a var with `name` and `context` is in `vars`. + """ + def has_var?(vars, name, context) do + Enum.any?(vars, &match?({^name, _, ^context}, &1)) + end + # private defp do_string_to_quoted(string) when is_binary(string) do diff --git a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex b/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex index 7e7a63529..94551b504 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex @@ -1,5 +1,6 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.Quoted do alias Elixir.Features + alias Lexical.Ast alias Lexical.Document alias Lexical.RemoteControl.Build alias Lexical.RemoteControl.ModuleMappings @@ -138,38 +139,144 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.Quoted do Map.put(result, :source, source) end - defp wrap_top_level_forms({:__block__, meta, nodes}) do - chunks = + @doc false + def wrap_top_level_forms({:__block__, meta, nodes}) do + {chunks, _vars} = nodes |> Enum.chunk_by(&should_wrap?/1) - |> Enum.with_index(fn [node | _] = nodes, i -> + |> Enum.with_index() + |> Enum.flat_map_reduce([], fn {[node | _] = nodes, i}, vars -> if should_wrap?(node) do - wrap_nodes(nodes, i) + {wrapped, vars} = wrap_nodes(nodes, vars, i) + {[wrapped], vars} else - nodes + {nodes, vars} end end) {:__block__, meta, chunks} end - defp wrap_top_level_forms(ast) do + def wrap_top_level_forms(ast) do wrap_top_level_forms({:__block__, [], [ast]}) end - defp wrap_nodes(nodes, i) do + defp wrap_nodes(nodes, vars, i) do module_name = :"lexical_wrapper_#{i}" - - quote do - defmodule unquote(module_name) do - def __lexical_wrapper__ do - (unquote_splicing(nodes)) + {nodes, new_vars} = suppress_and_extract_vars(nodes) + + quoted = + quote do + defmodule unquote(module_name) do + def __lexical_wrapper__([unquote_splicing(vars)]) do + (unquote_splicing(nodes)) + end end end - end + + {quoted, new_vars ++ vars} end @allowed_top_level [:defmodule, :alias, :import, :require, :use] defp should_wrap?({allowed, _, _}) when allowed in @allowed_top_level, do: false defp should_wrap?(_), do: true + + @doc false + # This function replaces all unused variables with `_` in order + # to suppress warnings while accumulating those vars. The approach + # here is bottom-up, starting from the last expression and working + # back to the beginning: + # + # - If the expression is an assignment, collect vars from the LHS, + # replacing them with `_` if they haven't been referenced, then + # collect references from the RHS. + # - If the expression isn't an assignment, just collect references. + # - Note that pinned vars on the LHS of an assignment are references. + # + def suppress_and_extract_vars(quoted) + + def suppress_and_extract_vars(list) when is_list(list) do + list + |> Enum.reverse() + |> do_suppress_and_extract_vars() + end + + def suppress_and_extract_vars({:__block__, meta, nodes}) do + {nodes, vars} = suppress_and_extract_vars(nodes) + {{:__block__, meta, nodes}, vars} + end + + def suppress_and_extract_vars(expr) do + {[expr], vars} = suppress_and_extract_vars([expr]) + {expr, vars} + end + + defp do_suppress_and_extract_vars(list, acc \\ [], references \\ [], vars \\ []) + + defp do_suppress_and_extract_vars([expr | rest], acc, references, vars) do + {expr, new_vars} = suppress_and_extract_vars_from_expr(expr, references) + new_references = extract_references_from_expr(expr) + + do_suppress_and_extract_vars( + rest, + [expr | acc], + new_references ++ references, + new_vars ++ vars + ) + end + + defp do_suppress_and_extract_vars([], acc, _references, vars) do + {acc, vars} + end + + defp suppress_and_extract_vars_from_expr({:=, meta, [left, right]}, references) do + {left, left_vars} = + Ast.prewalk_vars(left, [], fn + {:^, _, _} = pinned, acc -> + {pinned, acc} + + {name, meta, context} = var, acc -> + if Ast.has_var?(references, name, context) do + {var, [{name, [], context} | acc]} + else + {{:_, meta, nil}, [var | acc]} + end + end) + + {right, right_vars} = suppress_and_extract_vars_from_expr(right, references) + + {{:=, meta, [left, right]}, left_vars ++ right_vars} + end + + defp suppress_and_extract_vars_from_expr(other, _references) do + {other, []} + end + + defp extract_references_from_expr({:=, _, [left, right]}) do + {_, left_references} = + Ast.prewalk_vars(left, [], fn + {:^, _, [referenced_var]}, acc -> + {:ok, [referenced_var | acc]} + + node, acc -> + {node, acc} + end) + + right_references = extract_references_from_expr(right) + + left_references ++ right_references + end + + defp extract_references_from_expr(expr) do + {_, references} = + Ast.prewalk_vars(expr, [], fn + {:^, _, _}, acc -> + {:ok, acc} + + var, acc -> + {:ok, [var | acc]} + end) + + references + end end diff --git a/apps/remote_control/test/lexical/remote_control/build/document/compilers/quoted_test.exs b/apps/remote_control/test/lexical/remote_control/build/document/compilers/quoted_test.exs new file mode 100644 index 000000000..b11c2c6f9 --- /dev/null +++ b/apps/remote_control/test/lexical/remote_control/build/document/compilers/quoted_test.exs @@ -0,0 +1,137 @@ +defmodule Lexical.RemoteControl.Build.Document.Compilers.QuotedTest do + alias Lexical.RemoteControl.Build.Document.Compilers.Quoted + + import Lexical.Test.CodeSigil + + use ExUnit.Case, async: true + + defp parse!(code) do + Code.string_to_quoted!(code, columns: true, token_metadata: true) + end + + describe "wrap_top_level_forms/1" do + test "chunks and wraps unsafe top-level forms" do + quoted = + ~q[ + foo = 1 + bar = foo + 1 + + import Something + + defmodule MyModule do + :ok + end + + baz = bar + foo + ] + |> parse!() + + assert quoted |> Quoted.wrap_top_level_forms() |> Macro.to_string() == """ + defmodule :lexical_wrapper_0 do + def __lexical_wrapper__([]) do + foo = 1 + _ = foo + 1 + end + end + + import Something + + defmodule MyModule do + :ok + end + + defmodule :lexical_wrapper_2 do + def __lexical_wrapper__([foo, bar]) do + _ = bar + foo + end + end\ + """ + end + end + + describe "suppress_and_extract_vars/1" do + test "suppresses and extracts unused vars" do + quoted = + ~q[ + foo = 1 + bar = 2 + ] + |> parse!() + + assert {suppressed, [{:foo, _, nil}, {:bar, _, nil}]} = + Quoted.suppress_and_extract_vars(quoted) + + assert Macro.to_string(suppressed) == """ + _ = 1 + _ = 2\ + """ + end + + test "suppresses and extracts unused vars in nested assignments" do + quoted = + ~q[ + foo = bar = 1 + baz = qux = 2 + ] + |> parse!() + + assert {suppressed, [{:foo, _, nil}, {:bar, _, nil}, {:baz, _, nil}, {:qux, _, nil}]} = + Quoted.suppress_and_extract_vars(quoted) + + assert Macro.to_string(suppressed) == """ + _ = _ = 1 + _ = _ = 2\ + """ + end + + test "suppresses vars only referenced in RHS" do + quoted = ~q[foo = foo + 1] |> parse!() + + assert {suppressed, [{:foo, _, nil}]} = Quoted.suppress_and_extract_vars(quoted) + + assert Macro.to_string(suppressed) == "_ = foo + 1" + end + + test "suppresses deeply nested vars" do + quoted = ~q[{foo, {bar, %{baz: baz}}} = call()] |> parse!() + + assert {suppressed, [{:baz, _, nil}, {:bar, _, nil}, {:foo, _, nil}]} = + Quoted.suppress_and_extract_vars(quoted) + + assert Macro.to_string(suppressed) == "{_, {_, %{baz: _}}} = call()" + end + + test "does not suppress vars referenced in a later expression" do + quoted = + ~q[ + foo = 1 + bar = foo + 1 + ] + |> parse!() + + assert {suppressed, [{:foo, _, nil}, {:bar, _, nil}]} = + Quoted.suppress_and_extract_vars(quoted) + + assert Macro.to_string(suppressed) == """ + foo = 1 + _ = foo + 1\ + """ + end + + test "does not suppress vars referenced with pin operator in a later assignment" do + quoted = + ~q[ + foo = 1 + %{^foo => 2} = call() + ] + |> parse!() + + assert {suppressed, [{:foo, _, nil}]} = Quoted.suppress_and_extract_vars(quoted) + + assert Macro.to_string(suppressed) == """ + foo = 1 + %{^foo => 2} = call()\ + """ + end + end +end From 7852effe5392e5749b007ca60fa1ebc54983b7d1 Mon Sep 17 00:00:00 2001 From: Zach Allaun Date: Sun, 28 Jul 2024 12:17:04 -0400 Subject: [PATCH 4/4] Add comment for each branch in AST pattern match --- apps/common/lib/lexical/ast.ex | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/common/lib/lexical/ast.ex b/apps/common/lib/lexical/ast.ex index 08bbf5ad1..cd1db8e57 100644 --- a/apps/common/lib/lexical/ast.ex +++ b/apps/common/lib/lexical/ast.ex @@ -429,25 +429,34 @@ defmodule Lexical.Ast do def prewalk_vars(ast, acc, fun) do {ast, {acc, _}} = Macro.prewalk(ast, {acc, false}, fn + # <> + # ^^^^^^^^^^^ {:"::", meta, [left, right]}, {acc, _prev_pinned?} -> {right, acc} = prewalk_vars_in_binary(right, acc, fun) {{:"::", meta, [left, right]}, {acc, false}} + # skip vars inside quote or @ {skip, _, [_]} = node, {acc, _prev_pinned?} when skip in [:@, :quote] -> {node, {acc, false}} + # skip _ {:_, _, context} = node, {acc, _prev_pinned?} when is_atom(context) -> {node, {acc, false}} + # ^pinned + # emit the pinned var and set prev_pinned? so the var isn't omitted + # immediately after {:^, _, [{name, _, context}]} = pinned, {acc, _prev_pinned?} when is_atom(name) and is_atom(context) -> {pinned, acc} = fun.(pinned, acc) {pinned, {acc, true}} + # var {name, _, context} = var, {acc, false} when is_atom(name) and is_atom(context) -> {var, acc} = fun.(var, acc) {var, {acc, false}} + # skip everything else node, {acc, _prev_pinned?} -> {node, {acc, false}} end)