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

Do not check macro injected code #102

Merged
merged 4 commits into from
Sep 18, 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
4 changes: 2 additions & 2 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
elixir 1.12.3
erlang 24.3.4
elixir 1.13.4-otp-24
erlang 24.1
30 changes: 25 additions & 5 deletions lib/gradient.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand All @@ -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}
Expand Down Expand Up @@ -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)

Expand Down
33 changes: 25 additions & 8 deletions lib/gradient/elixir_checker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ->
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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
16 changes: 16 additions & 0 deletions lib/gradient/tokens.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion lib/mix/tasks/gradient.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 21 additions & 0 deletions test/examples/spec_in_macro.ex
Original file line number Diff line number Diff line change
@@ -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
31 changes: 23 additions & 8 deletions test/gradient/elixir_checker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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