gradient icon indicating copy to clipboard operation
gradient copied to clipboard

Incorrect :badarg when using comprehensions

Open Fl4m3Ph03n1x opened this issue 3 years ago • 0 comments

Background

I have an Option type that implements a couple of Protocols. Used alone, gradient likes the Option type. A problem however arises when I use it in elixir comprehensions.

The tool simply flips and raises :badarg errors for the functions using comprehensions.

Code

For completeness, here is the option.ex module:

defmodule Option do
  @type t(elem) :: __MODULE__.Some.t(elem) | __MODULE__.None.t()

  defmodule Some do
    @type t(elem) :: %__MODULE__{val: elem}

    defstruct [:val]

    defimpl Collectable do
      @impl Collectable
      def into(option), do: {option, fn acc, _command -> {:done, acc} end}
    end

    defimpl Enumerable do
      @impl Enumerable
      def count(_some), do: {:ok, 1}

      @impl Enumerable
      def member?(some, element), do: {:ok, some.val == element}

      @impl Enumerable
      def reduce(some, acc, fun)

      def reduce(_some, {:halt, acc}, _fun), do: {:halted, acc}
      def reduce(some, {:suspend, acc}, fun), do: {:suspended, acc, &reduce(some, &1, fun)}
      def reduce([], {:cont, acc}, _fun), do: {:done, acc}

      def reduce(%Option.Some{} = some, {:cont, acc}, fun),
        do: reduce([], fun.(some.val, acc), fun)

      @impl Enumerable
      @spec slice(%Option.Some{}) :: {:error, Enumerable.Option.Some}
      def slice(_option), do: {:error, __MODULE__}
    end
  end

  defmodule None do
    @type t :: %__MODULE__{}

    defstruct []

    defimpl Collectable do
      @impl Collectable
      def into(option) do
        {option,
         fn
           _acc, {:cont, val} ->
             %Option.Some{val: val}

           acc, :done ->
             acc

           _acc, :halt ->
             :ok
         end}
      end
    end

    defimpl Enumerable do
      @impl Enumerable
      def count(_none), do: {:error, __MODULE__}

      @impl Enumerable
      def member?(_none, _element), do: {:error, __MODULE__}

      @impl Enumerable
      def reduce(none, acc, fun)

      def reduce(_none, {:cont, acc}, _fun), do: {:done, acc}
      def reduce(_none, {:halt, acc}, _fun), do: {:halted, acc}
      def reduce(none, {:suspend, acc}, fun), do: {:suspended, acc, &reduce(none, &1, fun)}

      @impl Enumerable
      def slice(_option), do: {:error, __MODULE__}
    end
  end

  @spec new(any) :: __MODULE__.Some.t(any)
  def new(val), do: %__MODULE__.Some{val: val}

  @spec new :: __MODULE__.None.t()
  def new, do: %__MODULE__.None{}

  @spec none :: __MODULE__.None.t()
  def none, do: Option.new()

  @spec some(any) :: __MODULE__.Some.t(any)
  def some(val), do: Option.new(val)

  @spec or_else(Option.t(any), Option.t(any)) :: Option.t(any)
  def or_else(option_1, option_2) do
    case option_1 do
      %__MODULE__.Some{} -> option_1
      _ -> option_2
    end
  end
end

You can directly copy paste it. It will work out of the box.

Now, for the test snippet test.ex:

defmodule Test do
  alias Option

  @spec parse_show(String.t()) :: Option.t(String.t())
  def parse_show(raw_show) do
    for name <- extract_name(raw_show), into: Option.new() do
      name
    end
  end

  @spec extract_name(String.t()) :: Option.t(String.t())
  def extract_name(raw_show) do
    bracket_open_index = index_of(raw_show, "(")

    if bracket_open_index != -1 and bracket_open_index > 0 do
      raw_show
      |> String.slice(0..(bracket_open_index - 1))
      |> String.trim()
      |> Option.some()
    else
      Option.none()
    end
  end

  # Returns the index of the given char in the given String,
  # or -1 if the char is not found.
  @spec index_of(String.t(), String.t()) :: integer
  defp index_of(str, elem),
    do:
      str
      |> String.codepoints()
      |> list_index_of(elem)

  # Returns the index of the given element in the given list,
  # or -1 if the element is not found.
  @spec list_index_of([any], any) :: integer
  defp list_index_of(list, elem), do: search(list, elem, 0)

  @spec search([any], any, integer) :: integer
  defp search([], _elem, _index), do: -1

  defp search([head | tail], elem, index) do
    if head == elem do
      index
    else
      search(tail, elem, index + 1)
    end
  end
end

This module has a function parse_show, that given a String.t will return %Option.Some{val: "show name here"} if successful, or %Option.None{} otherwise.

The code works wonderfully:

iex(1)> Test.parse_show("Breaking Bad (2019)")
%Option.Some{val: "Breaking Bad"}
iex(2)> Test.parse_show("Breaking Bad")       
%Option.None{}
iex(3)> Test.parse_show("")            
%Option.None{}

Error

However, gradient seems to think otherwise:

Compiling 1 file (.ex)
Typechecking files...
lib/option.ex: The clause on line 44 cannot be reached

lib/option.ex: The clause on line 9 cannot be reached

lib/option.ex: The clause on line 61 cannot be reached

lib/option.ex: The clause on line 14 cannot be reached

lib/option.ex: The pattern %Option.Some{} on line 28 doesn't have the type any()

lib/test.ex: The function call on line 6 is expected to have type Option.t(String.t()) but it has type :badarg
4   @spec parse_show(String.t()) :: Option.t(String.t())
5   def parse_show(raw_show) do
6     for name <- extract_name(raw_show), into: Option.new() do
7       name
8     end

To be fair, the cannot be reached error lines are reported in another issue so I don't want to focus on them now.

The lib/test.ex: The function call on line 6 is expected to have type Option.t(String.t()) but it has type :badarg is what I believe is incorrect.

The extract_name function itself uses Option and works without issues (and gradient agrees). However, when combined into a comprehension for .... do ... end, gradient goes absolutely ballistic.

I believe this behavior is incorrect. Please note this behavior may very well be a side effect of the previously mentioned issue, but since I have no way of knowing, I am reporting it here.

Fl4m3Ph03n1x avatar Feb 23 '22 10:02 Fl4m3Ph03n1x