Merge pull request #102 from esl/do-not-check-macro-injected-code

Do not check macro injected code

Author: erszcz

.tool-versions

@@ -1,2 +1,2 @@
-elixir 1.12.3
-erlang 24.3.4
+elixir 1.13.4-otp-24
+erlang 24.1

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,22 @@ 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)
+      |> Enum.map(fn ast ->
+        ast =
+          ast
+          |> put_source_path(opts)
+          |> maybe_specify_forms(opts)
+
+        tokens = maybe_use_tokens(ast, opts)
+        opts = [{:env, build_env(tokens)} | opts]
 
-        case maybe_gradient_check(single_module_forms, opts) ++
-               maybe_gradualizer_check(single_module_forms, opts) do
+        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 +82,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)
 

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

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.

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,

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

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