Dialyzer has an incorrect report

Questions / Help
1

Background

I am reading “ Functional Web Development with Elixr, OTP, and Phoenix ” and there is a code sample that is setting of an error with dialyzer, an error which I believe is incorrect (should not be an error). This is rather confusing, as I have read that if dyalizer tells you something is wrong, then something is wrong for sure.

  @impl GenServer
  def handle_call({:position_island, player, key, row, col}, _from, state) do
    board = player_board(state, player)
    reply_error = create_error_reply(state)

    with  {:ok, rules}  <- Rules.check(state.rules, {:position_islands, player}),
          {:ok, coord}  <- Coordinate.new(row, col),
          {:ok, island} <- Island.new(key, coord),
          %{} = board   <- Board.position_island(board, key, island)
    do
      state
      |> update_board(player, board)
      |> update_rules(rules)
      |> reply_success(:ok)
    else
      :error  ->
        reply_error.(:error)
      {:error, :invalid_coordinate}   ->
        reply_error.({:error, :invalid_coordinate})
      {:error, :invalid_island_type}  ->
        reply_error.({:error, :invalid_island_type})
      {:error, :overlapping_island}   ->
        reply_error.({:error, :overlappign_island})
    end
  end

Error

It points to the line:

{:ok, island} <- Island.new(key, coord),

and tells me that I am not watching out for the errors {'error','invalid_coordinate' | 'invalid_island_type'}:

lib/islands_engine/game.ex:88: The pattern {‘ok’, _island@1} can never match the type {‘error’,‘invalid_coordinate’ | ‘invalid_island_type’}

Which is not true, as you can see I have an else clause that checks for this.

Why am is dialyzer screaming about this?

2

Depending on the version of elixir and erlang you use, there are a lot of known issues around dialyzer and with.

PS: Also look closely on the error message, it does not tell you, that you are not covering the error returns, but it tells you, that Island.new/2 will never return something of type {:error, :invalid_coordinate | :invalid_island_type}.

3

Versions:

elixir 1.8.1-otp-21
erlang 21.2.6

If dialyzer really is that broken, what alternatives would you recommend? I only know of Gradualizer, but it’s in such an early stage I don’t really want to try it yet.

PS: Also look closely on the error message, it does not tell you, that you are not covering the error returns, but it tells you, that Island.new/2 will never return something of type {:error, :invalid_coordinate | :invalid_island_type} .

Code wise, I’m pretty sure this can happen:

@spec new(any(), %Coordinate{}) :: {:ok, %Island{}} | {:error, any}
  def new(type, %Coordinate{} = upper_left) do
    with  [_|_] <- offsets = offsets(type),
          %MapSet{} <- coords = add_coordinates(offsets, upper_left)
    do
      {:ok, %Island{coordinates: coords, hit_coordinates: MapSet.new()}}
    end
  end

  @spec offsets(shape) :: [tuple] | {:error, :invalid_island_type}
  defp offsets(:square), do: [{0, 0}, {0, 1}, {1, 0}, {1, 1}]
  defp offsets(_), do: {:error, :invalid_island_type}

  @spec add_coordinates([tuple], %Coordinate{}) :: MapSet.t | {:error, :invalid_coordinate}
  defp add_coordinates(offsets, upper_left) do
    Enum.reduce_while(offsets, MapSet.new(), fn(offset, acc) ->
      add_coordinate(acc, upper_left, offset)
    end)
  end
4

I believe that the dialyzer message means that Island.new can only return {:error, :invalid_coordinate} or {:error, :invalid_island_type}. I presume that the code is in fact working correctly, but somewhere in the stack of Island.new there is some other error which leads dialyzer to believe that the function can’t return {:ok, island}. Did you get some other errors reported too? If so, then by fixing some of these other errors you might be able to fix this one too.

1 Like
5

Did you get some other errors reported too? If so, then by fixing some of these other errors you might be able to fix this one too.

Correct ! I had another issue:

Which was causing dialyzer to produce this output. What are your opinions on dialzyer? You didn’t use it in the book, is it because you don’t like it or because you couldn’t afford to put it?

6

I have mixed opinions.

My personal sentiment is that type specifications should be used in production code, because that greatly assists the reading experience. Dialyzer is the best (afaik the only) tool which can detect some problems related to typespecs, so I advise using it.

That said, there are a bunch of issues with it, such as hard to understand errors, the fact that dialyzer doesn’t detect all the problems, and the general slowness of the tool. We use it at Aircloak regularly, and run it in our CI pipeline, but I can’t say we’re very happy with it. That said, I’d still rather use dialyzer than not use it :slight_smile:

I didn’t use it in the book, to avoid covering too many topics and confusing readers. That said, for the next edition I’m considering adding type specifications to the book.

2 Likes
7

There is also Gradualizer:

2 Likes