Skip to content
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
3 changes: 2 additions & 1 deletion lib/gradient.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ defmodule Gradient do
alias Gradient.ElixirFileUtils
alias Gradient.ElixirFmt
alias Gradient.AstSpecifier
alias Gradient.ElixirChecker

require Logger

Expand All @@ -25,7 +26,7 @@ defmodule Gradient do
|> put_code_path(opts)
|> AstSpecifier.specify()

case :gradualizer.type_check_forms(forms, opts) do
case ElixirChecker.check(forms, opts) ++ :gradualizer.type_check_forms(forms, opts) do
[] ->
:ok

Expand Down
89 changes: 89 additions & 0 deletions lib/gradient/elixir_checker.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
defmodule Gradient.ElixirChecker do
@moduledoc ~s"""
Provide checks specific to Elixir that complement type checking delivered by Gradient.

Options:
- {`ex_check`, boolean()}: whether to use checks specific only to Elixir.
"""

@spec check([:erl_parse.abstract_form()], keyword()) :: [{:file.filename(), any()}]
def check(forms, opts) do
if Keyword.get(opts, :ex_check, true) do
check_spec(forms)
else
[]
end
end

@doc ~s"""
Check if all specs are exactly before the function that they specify
and if there is only one spec per function clause.

Correct spec locations:
```
@spec convert(integer()) :: float()
def convert(int) when is_integer(int), do: int / 1

@spec convert(atom()) :: binary()
def convert(atom) when is_atom(atom), do: to_string(atom)
```

Incorrect spec locations:
- More than one spec above function clause.
```
@spec convert(integer()) :: float()
@spec convert(atom()) :: binary()
def convert(int) when is_integer(int), do: int / 1

def convert(atom) when is_atom(atom), do: to_string(atom)
```

- Spec name doesn't match the function name.
```
@spec last_two(atom()) :: atom()
def last_three(:ok) do
:ok
end
```
"""
@spec check_spec([:erl_parse.abstract_form()]) :: [{:file.filename(), any()}]
def check_spec([{:attribute, _, :file, {file, _}} | forms]) do
forms
|> Stream.filter(&is_fun_or_spec?/1)
|> Stream.map(&simplify_form/1)
|> Stream.concat()
|> Stream.filter(&has_line/1)
|> Enum.sort(&(elem(&1, 2) < elem(&2, 2)))
|> Enum.reduce({nil, []}, fn
{:fun, fna, _} = fun, {{:spec, {n, a} = sna, anno}, errors} when fna != sna ->
# Spec name doesn't match the function name
{fun, [{:spec_error, :wrong_spec_name, anno, n, a} | errors]}

{:spec, {n, a}, anno} = s1, {{:spec, _, _}, errors} ->
# Only one spec per function clause is allowed
{s1, [{:spec_error, :spec_after_spec, anno, n, a} | errors]}

x, {_, errors} ->
{x, errors}
end)
|> elem(1)
|> Enum.map(&{file, &1})
end

# Filter out __info__ generated function
def has_line(form), do: :erl_anno.line(elem(form, 2)) > 1

def is_fun_or_spec?({:attribute, _, :spec, _}), do: true
def is_fun_or_spec?({:function, _, _, _, _}), do: true
def is_fun_or_spec?(_), do: false

@spec simplify_form(:erl_parse.abstract_form()) ::
Enumerable.t({:spec | :fun, {atom(), integer()}, :erl_anno.anno()})
def simplify_form({:attribute, _, :spec, {{name, arity}, types}}) do
Stream.map(types, &{:spec, {name, arity}, elem(&1, 1)})
end

def simplify_form({:function, _, name, arity, clauses}) do
Stream.map(clauses, &{:fun, {name, arity}, elem(&1, 1)})
end
end
27 changes: 27 additions & 0 deletions lib/gradient/elixir_fmt.ex
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,33 @@ defmodule Gradient.ElixirFmt do
format_expr_type_error(expression, actual_type, expected_type, opts)
end

def format_type_error(
{:spec_error, :wrong_spec_name, anno, name, arity},
opts
) do
:io_lib.format(
"~sThe spec ~p/~p~s doesn't match the function name/arity~n",
[
format_location(anno, :brief, opts),
name,
arity,
format_location(anno, :verbose, opts)
]
)
end

def format_type_error({:spec_error, :spec_after_spec, anno, name, arity}, opts) do
:io_lib.format(
"~sThe spec ~p/~p~s follows another spec, but only one spec per function clause is allowed~n",
[
format_location(anno, :brief, opts),
name,
arity,
format_location(anno, :verbose, opts)
]
)
end

def format_type_error({:call_undef, anno, module, func, arity}, opts) do
:io_lib.format(
"~sCall to undefined function ~s~p/~p~s~n",
Expand Down
Binary file added test/examples/Elixir.CorrectSpec.beam
Binary file not shown.
Binary file added test/examples/Elixir.SpecAfterSpec.beam
Binary file not shown.
Binary file added test/examples/Elixir.SpecWrongName.beam
Binary file not shown.
6 changes: 6 additions & 0 deletions test/examples/spec_after_spec.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
defmodule SpecAfterSpec do
@spec convert(integer()) :: float()
@spec convert(atom()) :: binary()
def convert(int) when is_integer(int), do: int / 1
def convert(atom) when is_atom(atom), do: to_string(atom)
end
6 changes: 6 additions & 0 deletions test/examples/spec_correct.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
defmodule CorrectSpec do
@spec convert(integer()) :: float()
def convert(int) when is_integer(int), do: int / 1
@spec convert(atom()) :: binary()
def convert(atom) when is_atom(atom), do: to_string(atom)
end
15 changes: 15 additions & 0 deletions test/examples/spec_wrong_name.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
defmodule SpecWrongName do
@spec convert(integer()) :: float()
def convert(int) when is_integer(int), do: int / 1

@spec convert(atom()) :: binary()
def last_two(list) do
[last, penultimate | _tail] = Enum.reverse(list)
[penultimate, last]
end

@spec last_two(atom()) :: atom()
def last_three(:ok) do
:ok
end
end
2 changes: 2 additions & 0 deletions test/gradient/ast_specifier_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1585,8 +1585,10 @@ defmodule Gradient.AstSpecifierTest do

assert {:function, 3, :name, 0, [{:clause, 3, [], [], [{:atom, 4, :module_a}]}]} =
List.last(AstSpecifier.run_mappers(astA, tokensA))

assert {:function, 9, :name, 0, [{:clause, 9, [], [], [{:atom, 10, :module_b}]}]} =
List.last(AstSpecifier.run_mappers(astB, tokensB))

assert {:function, 14, :name, 0, [{:clause, 14, [], [], [{:atom, 15, :module}]}]} =
List.last(AstSpecifier.run_mappers(ast, tokens))
end
Expand Down
38 changes: 38 additions & 0 deletions test/gradient/elixir_checker_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
defmodule Gradient.ElixirCheckerTest do
use ExUnit.Case
doctest Gradient.ElixirChecker

alias Gradient.ElixirChecker

import Gradient.TestHelpers

test "checker options" do
ast = load("Elixir.SpecWrongName.beam")

assert [] = ElixirChecker.check(ast, ex_check: false)
assert [] != ElixirChecker.check(ast, ex_check: true)
end

test "all specs are correct" do
ast = load("Elixir.CorrectSpec.beam")

assert [] = ElixirChecker.check(ast, ex_check: true)
end

test "spec name doesn't match the function name" do
ast = load("Elixir.SpecWrongName.beam")

assert [
{_, {:spec_error, :wrong_spec_name, 11, :last_two, 1}},
{_, {:spec_error, :wrong_spec_name, 5, :convert, 1}}
] = ElixirChecker.check(ast, [])
end

test "more than one spec per function clause is not allowed" do
ast = load("Elixir.SpecAfterSpec.beam")

assert [
{_, {:spec_error, :spec_after_spec, 3, :convert_a, 1}}
] = ElixirChecker.check(ast, [])
end
end
21 changes: 21 additions & 0 deletions test/gradient/elixir_fmt_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,27 @@ defmodule Gradient.ElixirFmtTest do
end
end

describe "spec" do
test "name doesn't match the function name" do
msg =
{:spec_error, :wrong_spec_name, 3, :convert, 1}
|> ElixirFmt.format_error([])
|> :erlang.iolist_to_binary()

assert "The spec convert/1 on line 3 doesn't match the function name/arity\n" = msg
end

test "follows another spec" do
msg =
{:spec_error, :spec_after_spec, 3, :convert, 1}
|> ElixirFmt.format_error([])
|> :erlang.iolist_to_binary()

assert "The spec convert/1 on line 3 follows another spec, but only one spec per function clause is allowed\n" =
msg
end
end

@tag :skip
test "format_expr_type_error/4" do
opts = [forms: basic_erlang_forms()]
Expand Down
3 changes: 1 addition & 2 deletions test/gradient/tokens_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ defmodule Gradient.TokensTest do
end

test "unless" do
{tokens, _ast} =
load("conditional/Elixir.Conditional.Unless.beam", "conditional/unless.ex")
{tokens, _ast} = load("conditional/Elixir.Conditional.Unless.beam", "conditional/unless.ex")

tokens = Tokens.drop_tokens_to_line(tokens, 2)
opts = [end_line: -1]
Expand Down
10 changes: 10 additions & 0 deletions test/support/helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@ defmodule Gradient.TestHelpers do

@examples_path "test/examples/"

@spec load(String.t()) :: T.forms()
def load(beam_file) do
beam_file = String.to_charlist(@examples_path <> beam_file)

{:ok, {_, [abstract_code: {:raw_abstract_v1, ast}]}} =
:beam_lib.chunks(beam_file, [:abstract_code])

ast
end

@spec load(String.t(), String.t()) :: {T.tokens(), T.forms()}
def load(beam_file, ex_file) do
beam_file = String.to_charlist(@examples_path <> beam_file)
Expand Down