Skip to content

Commit

Permalink
Do not check spec position for forms injected by a macro
Browse files Browse the repository at this point in the history
  • Loading branch information
Premwoik authored and erszcz committed Sep 18, 2022
1 parent f68d7e1 commit 063bd8e
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 16 deletions.
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
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

0 comments on commit 063bd8e

Please sign in to comment.