From f68d7e1dd88ed302f73e6773201c8e5f484f247b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Wed, 15 Jun 2022 01:32:23 +0200 Subject: [PATCH 1/4] Store module information in env --- lib/gradient.ex | 31 ++++++++++++++++++++++++------- lib/gradient/tokens.ex | 16 ++++++++++++++++ lib/mix/tasks/gradient.ex | 4 +++- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/lib/gradient.ex b/lib/gradient.ex index 1d129f21..a88cb363 100644 --- a/lib/gradient.ex +++ b/lib/gradient.ex @@ -25,6 +25,8 @@ defmodule Gradient do no_specify: boolean() ] + @type env() :: %{tokens_present: boolean(), macro_lines: [integer()]} + @doc """ Type-checks file in `path` with provided `opts`, and prints the result. """ @@ -37,16 +39,19 @@ defmodule Gradient do {:ok, first_ast} <- get_first_forms(asts), {:elixir, _} <- wrap_language_name(first_ast) do asts - |> Enum.map(fn module_forms -> - single_module_forms = maybe_specify_forms(module_forms, opts) - - case maybe_gradient_check(single_module_forms, opts) ++ - maybe_gradualizer_check(single_module_forms, opts) do + |> Enum.map(fn ast -> + ast = put_source_path(ast, opts) + tokens = maybe_use_tokens(ast, opts) + ast = maybe_specify_forms(ast, tokens, opts) + opts = [{:env, build_env(tokens)} | opts] + + case maybe_gradient_check(ast, opts) ++ + maybe_gradualizer_check(ast, opts) do [] -> :ok errors -> - opts = Keyword.put(opts, :forms, single_module_forms) + opts = Keyword.put(opts, :forms, ast) ElixirFmt.print_errors(errors, opts) {:error, errors} @@ -74,6 +79,18 @@ defmodule Gradient do end end + def build_env(tokens) do + %{tokens_present: tokens != [], macro_lines: Gradient.Tokens.find_macro_lines(tokens)} + end + + defp maybe_use_tokens(forms, opts) do + unless opts[:no_tokens] do + Gradient.ElixirFileUtils.load_tokens(forms) + else + [] + end + end + defp maybe_gradualizer_check(forms, opts) do opts = Keyword.put(opts, :return_errors, true) @@ -98,7 +115,7 @@ defmodule Gradient do end end - defp maybe_specify_forms(forms, opts) do + defp maybe_specify_forms(forms, tokens, opts) do unless opts[:no_specify] do forms |> put_source_path(opts) diff --git a/lib/gradient/tokens.ex b/lib/gradient/tokens.ex index ac6c7b22..1be72890 100644 --- a/lib/gradient/tokens.ex +++ b/lib/gradient/tokens.ex @@ -60,6 +60,22 @@ defmodule Gradient.Tokens do end end + def find_macro_lines([ + {:identifier, {l, _, _}, :use}, + {:alias, {l, _, _}, _}, + {:eol, {l, _, _}} | t + ]) do + [l | find_macro_lines(t)] + end + + def find_macro_lines([_ | t]) do + find_macro_lines(t) + end + + def find_macro_lines([]) do + [] + end + @doc """ Drop tokens to the first tuple occurrence. Returns type of the encountered list and the following tokens. diff --git a/lib/mix/tasks/gradient.ex b/lib/mix/tasks/gradient.ex index a6400b97..0f7f6d24 100644 --- a/lib/mix/tasks/gradient.ex +++ b/lib/mix/tasks/gradient.ex @@ -12,7 +12,8 @@ defmodule Mix.Tasks.Gradient do * `--no-gradualizer-check` - do not perform the Gradualizer checks * `--no-specify` - do not specify missing lines in AST what can result in less precise error messages - * `--source-path` - provide a path to the .ex file containing code for analyzed .beam + * `--source-path` - provide a path to the .ex file containing code for analyzed .beam + * `--no-tokens` - do not use tokens to increase the precision of typechecking * `--no-deps` - do not import dependencies to the Gradualizer * `--stop_on_first_error` - stop type checking at the first error @@ -44,6 +45,7 @@ defmodule Mix.Tasks.Gradient do no_specify: :boolean, # checker options source_path: :string, + no_tokens: :boolean, no_deps: :boolean, stop_on_first_error: :boolean, infer: :boolean, From 063bd8ec210159a510674030c41ea57cd566777b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Wed, 15 Jun 2022 01:34:52 +0200 Subject: [PATCH 2/4] Do not check spec position for forms injected by a macro --- lib/gradient/elixir_checker.ex | 33 ++++++++++++++++++++------- test/examples/spec_in_macro.ex | 21 +++++++++++++++++ test/gradient/elixir_checker_test.exs | 31 ++++++++++++++++++------- 3 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 test/examples/spec_in_macro.ex diff --git a/lib/gradient/elixir_checker.ex b/lib/gradient/elixir_checker.ex index ec8aad2a..62c9abad 100644 --- a/lib/gradient/elixir_checker.ex +++ b/lib/gradient/elixir_checker.ex @@ -6,12 +6,14 @@ defmodule Gradient.ElixirChecker do - {`ex_check`, boolean()}: whether to use checks specific only to Elixir. """ + @type env() :: Gradient.env() + @type opts :: [env: env(), ex_check: boolean()] @type simplified_form :: {:spec | :fun, {atom(), integer()}, :erl_anno.anno()} - @spec check([:erl_parse.abstract_form()], keyword()) :: [{:file.filename(), any()}] + @spec check([:erl_parse.abstract_form()], opts()) :: [{:file.filename(), any()}] def check(forms, opts) do if Keyword.get(opts, :ex_check, true) do - check_spec(forms) + check_spec(forms, opts[:env]) else [] end @@ -48,13 +50,16 @@ defmodule Gradient.ElixirChecker do end ``` """ - @spec check_spec([:erl_parse.abstract_form()]) :: [{:file.filename(), any()}] - def check_spec([{:attribute, _, :file, {file, _}} | forms]) do + @spec check_spec([:erl_parse.abstract_form()], map()) :: [{:file.filename(), any()}] + def check_spec([{:attribute, _, :file, {file, _}} | forms], env) do + %{tokens_present: tokens_present, macro_lines: macro_lines} = env + forms - |> Stream.filter(&is_fun_or_spec?/1) + |> Stream.filter(&is_fun_or_spec?(&1, macro_lines)) |> Stream.map(&simplify_form/1) |> Stream.concat() |> Stream.filter(&is_not_generated?/1) + |> remove_injected_forms(not tokens_present) |> Enum.sort(&(elem(&1, 2) < elem(&2, 2))) |> Enum.reduce({nil, []}, fn {:fun, {n, :def}, _}, {{:spec, {sn, _}, _}, _} = acc when n == sn -> @@ -83,9 +88,10 @@ defmodule Gradient.ElixirChecker do not (String.starts_with?(name_str, "__") and String.ends_with?(name_str, "__")) end - def is_fun_or_spec?({:attribute, _, :spec, _}), do: true - def is_fun_or_spec?({:function, _, _, _, _}), do: true - def is_fun_or_spec?(_), do: false + # The forms injected by `__using__` macro inherit the line from `use` keyword. + def is_fun_or_spec?({:attribute, anno, :spec, _}, ml), do: :erl_anno.line(anno) not in ml + def is_fun_or_spec?({:function, anno, _, _, _}, ml), do: :erl_anno.line(anno) not in ml + def is_fun_or_spec?(_, _), do: false @doc """ Returns a stream of simplified forms in the format defined by type `simplified_form/1` @@ -115,4 +121,15 @@ defmodule Gradient.ElixirChecker do _ -> false end) end + + # When tokens were not present to detect macro_lines, the forms without unique + # lines can be removed. + def remove_injected_forms(forms, true) do + forms + |> Enum.group_by(fn {_, _, line} -> line end) + |> Enum.filter(fn {_, fs2} -> length(fs2) == 1 end) + |> Enum.flat_map(fn {_, fs2} -> fs2 end) + end + + def remove_injected_forms(forms, false), do: forms end diff --git a/test/examples/spec_in_macro.ex b/test/examples/spec_in_macro.ex new file mode 100644 index 00000000..a29a8ced --- /dev/null +++ b/test/examples/spec_in_macro.ex @@ -0,0 +1,21 @@ +defmodule NewMod do + defmacro __using__(_) do + quote do + @spec new(attrs :: map()) :: atom() + def new(_attrs), do: :ok + + @spec a(attrs :: map()) :: atom() + def a(_attrs), do: :ok + + @spec b(attrs :: map()) :: atom() + def b(_attrs), do: :ok + end + end +end + +defmodule SpecInMacro do + use NewMod + + @spec c(attrs :: map()) :: atom() + def c(_attrs), do: :ok +end diff --git a/test/gradient/elixir_checker_test.exs b/test/gradient/elixir_checker_test.exs index e85d9665..b60b576a 100644 --- a/test/gradient/elixir_checker_test.exs +++ b/test/gradient/elixir_checker_test.exs @@ -9,27 +9,26 @@ defmodule Gradient.ElixirCheckerTest do test "checker options" do ast = load("Elixir.SpecWrongName.beam") - assert [] = ElixirChecker.check(ast, ex_check: false) - assert [] != ElixirChecker.check(ast, ex_check: true) + assert [] = ElixirChecker.check(ast, env([], ex_check: false)) + assert [] != ElixirChecker.check(ast, env([], ex_check: true)) end test "all specs are correct" do ast = load("Elixir.CorrectSpec.beam") - assert [] = ElixirChecker.check(ast, ex_check: true) + assert [] = ElixirChecker.check(ast, env()) end test "specs over default args are correct" do ast = load("Elixir.SpecDefaultArgs.beam") - assert [] = ElixirChecker.check(ast, ex_check: true) + assert [] = ElixirChecker.check(ast, env()) end test "spec arity doesn't match the function arity" do ast = load("Elixir.SpecWrongArgsArity.beam") - assert [{_, {:spec_error, :wrong_spec_name, 2, :foo, 3}}] = - ElixirChecker.check(ast, ex_check: true) + assert [{_, {:spec_error, :wrong_spec_name, 2, :foo, 3}}] = ElixirChecker.check(ast, env()) end test "spec name doesn't match the function name" do @@ -38,7 +37,7 @@ defmodule Gradient.ElixirCheckerTest do assert [ {_, {:spec_error, :wrong_spec_name, 5, :convert, 1}}, {_, {:spec_error, :wrong_spec_name, 11, :last_two, 1}} - ] = ElixirChecker.check(ast, []) + ] = ElixirChecker.check(ast, env()) end test "mixing specs names is not allowed" do @@ -47,6 +46,22 @@ defmodule Gradient.ElixirCheckerTest do assert [ {_, {:spec_error, :mixed_specs, 3, :encode, 1}}, {_, {:spec_error, :wrong_spec_name, 3, :encode, 1}} - ] = ElixirChecker.check(ast, []) + ] = ElixirChecker.check(ast, env()) + end + + test "spec defined in a __using__ macro with tokens" do + {tokens, ast} = load("Elixir.SpecInMacro.beam", "spec_in_macro.ex") + + assert [] = ElixirChecker.check(ast, env(tokens)) + end + + test "spec defined in a __using__ macro without tokens" do + ast = load("Elixir.SpecInMacro.beam") + + assert [] = ElixirChecker.check(ast, env()) + end + + defp env(tokens \\ [], opts \\ []) do + [{:env, Gradient.build_env(tokens)} | opts] end end From 6c5f0e567196fd0146430f5ee3db2c41d36d01f7 Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Sun, 18 Sep 2022 20:13:21 +0200 Subject: [PATCH 3/4] Update .tool-versions --- .tool-versions | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.tool-versions b/.tool-versions index 4ac68ed6..44348225 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ -elixir 1.12.3 -erlang 24.3.4 +elixir 1.13.4-otp-24 +erlang 24.1 From 163834b12b6d437da59f45820cda13fc42edeff1 Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Sun, 18 Sep 2022 20:58:42 +0200 Subject: [PATCH 4/4] Drop unused tokens var --- lib/gradient.ex | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/gradient.ex b/lib/gradient.ex index a88cb363..e596a3aa 100644 --- a/lib/gradient.ex +++ b/lib/gradient.ex @@ -40,9 +40,12 @@ defmodule Gradient do {:elixir, _} <- wrap_language_name(first_ast) do asts |> Enum.map(fn ast -> - ast = put_source_path(ast, opts) + ast = + ast + |> put_source_path(opts) + |> maybe_specify_forms(opts) + tokens = maybe_use_tokens(ast, opts) - ast = maybe_specify_forms(ast, tokens, opts) opts = [{:env, build_env(tokens)} | opts] case maybe_gradient_check(ast, opts) ++ @@ -115,7 +118,7 @@ defmodule Gradient do end end - defp maybe_specify_forms(forms, tokens, opts) do + defp maybe_specify_forms(forms, opts) do unless opts[:no_specify] do forms |> put_source_path(opts)