Trying to decipher a "The pattern can never match the type" Dialyzer error message

(Edit: Apologies everyone. I had forgot to make the repository public before posting. The repository is public now, and so the links should now work correctly.)

I am implementing code from the book Functional Web Development with Elixir, OTP, and Phoenix by Lance Halvorsen, which is really good by the way. I am fully typespeccing the project, and I have come across a Dialyzer error that I just can’t figure out. Here is the repository.

The function that it is complaining about is:

def handle_call({:position_island, player_role, island_type, row, column}, _from, state) do
    board = player_board(state, player_role)

    with {:ok, rules} <- Rules.check(state.rules, {:position_islands, player_role}),
         {:ok, coordinate} <- Coordinate.new(row, column),
         {:ok, island} <- Island.new(island_type, coordinate),
         board <- Board.position_island(board, island_type, island) do
      state
      |> update_board(player_role, board)
      |> update_rules(rules)
      |> reply_success(:ok)
    else
      :error = check_error ->
        {:reply, check_error, state}

      {:error, :invalid_coordinate} = coordinate_new_error ->
        {:reply, coordinate_new_error, state}

      {:error, :invalid_island_type} = coordinate_or_island_new_error ->
        {:reply, coordinate_or_island_new_error, state}

      {:error, :overlapping_island} = position_island_error ->
        {:reply, position_island_error, state}
    end
  end

and the specific line is:

{:ok, island} <- Island.new(island_type, coordinate),

The error reported by Dialyzer is:

lib/islands_engine/game.ex:130:pattern_match
The pattern can never match the type.

Pattern:
{:ok, _island}

Type:
{:error, :invalid_coordinate | :invalid_island_type}

This error is a common one in Dialyzer, and a commonly confusing one. In this context, the error makes zero sense because the entire point of the with form pattern match clause is to not match the error types Dialyzer is complaining, which are additionally caught below in the else section.

In my searching to try and figure this out, the consensus seemed to be that the problem wasn’t necessarily here but somewhere else in the call stack. I have tried tracing things down and ensuring that the typespecs made sense, and I haven’t been able to find any issues. I have toggled some things to try and see a different error but with no luck.

This was previously additionally confusing because Dialyzer was complaining about the update_board function never being called, which was false. It is clearly called above, and this code works and is tested decently. However, that Dialyzer went away at some point. The error I have described above is the only Dialyzer error reported.

Can someone help me debug this or provide any pointers? The code has been developed on Elixir 1.13 and has no dependencies outside of Hex packages. This has me quite confused.

This link is a 404, perhaps it’s a private repo?

It seems to be suggesting that your typespec for Island.new/2 does not include {:ok, island} as a possible return value. Pasting the function definition w/ spec for that function (or a working link to the repo) would be very useful to help you debug!

1 Like

Oh, shoot. Sorry about that. I had meant to make that change before posting this. I have now properly changed the repository to public. The link should hopefully work now.

Yea, it’s confusing. It definitely returns an {:ok, Island.t()} tuple in the success case, which is also reflected in the typespec for Island.new/2. I thought maybe the typespec for Island.t() may be causing grief since it uses parameterized types. It was originally:

  @type t() :: %__MODULE__{
          coordinates: MapSet.t(Coordinate),
          hit_coordinates: MapSet.t(Coordinate)
        }

but then I tried changing it to:

  @type t() :: %__MODULE__{
          coordinates: MapSet.t(Coordinate),
          hit_coordinates: MapSet.t(Coordinate) | MapSet.t()
        }

to try and account for the fact that Island.new/2 does return MapSet.new() for the hit_coordinates key. But that didn’t seem to affect anything.

One issue might be the %MapSet{} = pattern match in Island.new/2, even if the warning is ignored it might be throwing Dialyzer off. I think what Dialyzer is saying is that Island.new/2 will never return {:ok, _} so that pattern will never match.

1 Like

This is correct! I cloned this repo and tried it, and this fixes the error. If you want to keep everything spec’d and get rid of this error while matching on the success in the with statement, you can wrap the success return of add_coordinates/2 in an {:ok, success} tuple, which makes the API more consistent and idiomatic IMO, anyway.

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

  @spec add_coordinates([offset()], Coordinate.t()) ::
          {:ok, coordinates()} | {:error, :invalid_coordinate}
  defp add_coordinates(offsets, upper_left) do
    Enum.reduce_while(offsets, MapSet.new(), fn island_type, coordinates ->
      add_coordinate(coordinates, upper_left, island_type)
    end)
    |> case do
      {:error, reason} -> {:error, reason}
      coordinates -> {:ok, coordinates}
    end
  end

Another thing I noticed, which does not fix the dialyzer error but will make your specs more correct, your type for Island.t/0 has a mistake:

@type t() :: %__MODULE__{
        coordinates: MapSet.t(Coordinate),
        hit_coordinates: MapSet.t(Coordinate) | MapSet.t()
      }

This is saying that you’re typing it for a MapSet with elements of the type atom with value Coordinate. It should be:

@type t() :: %__MODULE__{
        coordinates: MapSet.t(Coordinate.t()),
        hit_coordinates: MapSet.t(Coordinate.t()) | MapSet.t()
      }
1 Like

Cat walked across the keyboard and caused me to submit an in-work response. Replaced it with this post since I don’t see a way to delete a comment.

Thank you to both @msimonborg and @Nicd! I checked this as well, and indeed, removing the %MapSet{} pattern match works. However, I’m not sure why.

It does seem that the way to read the Dialyzer message is to take

The pattern can never match the type.

Pattern:
{:ok, _island}

Type:
{:error, :invalid_coordinate | :invalid_island_type}

and read it as “Dialyzer thinks that Island.new/2 will never return {:ok, _} and thus {:ok, island} can never match what Dialyzer thinks the actual return of Island.new/2 is, which it thinks it is {:error, :invalid_coordinate | :invalid_island_type}”.

What I am confused by is:

  1. The Dialyzer message, as written, is wrong. Island.new/2 can return {:ok, Island.t()}, and if it doesn’t, the error cases will be cause by the with’s else branch.

  2. I’m not sure why the %MapSet{} pattern match was throwing things off in Island.new/2. Island.add_coordinates/2 does return a MapSet in the success case, as is indicated by both the implementation and its typespec.

I’m happy enough to move on since the Dialyzer error message is thankfully gone, but I am pretty confused as to what’s going on and would ideally like to.

If you want to keep everything spec’d and get rid of this error while matching on the success in the with statement, you can wrap the success return of add_coordinates/2 in an {:ok, success} tuple, which makes the API more consistent and idiomatic IMO, anyway.

That’s true that that is more idiomatic. The current way is as it is implemented in the book, but I think I’ll change add_coordinates/2 with a case on the return value of the Enum.reduce_while to return an {:ok, coordinates()}. However, the problem does return if I match on {:ok, %MapSet{} = coordinates} <- add_coordinates(offsets, upper_left). I don’t understand why the %MapSet{} is throwing things off. coordinates is a MapSet.

Another thing I noticed, which does not fix the dialyzer error but will make your specs more correct, your type for Island.t/0 has a mistake:

Thank you for the catch! I have corrected that now. What do you think about the typespec for Island.t() when it says hit_coordinates: MapSet.t(Coordinate.t()) | MapSet.t()? I added on the MapSet.t() because Island.new/2 does return an empty MapSet, i.e., doesn’t have elements of type Coordinate.t(), for hit_coordinates.

MapSet.t() is opaque, so Dialyzer doesn’t consider it to “match” anything that isn’t also that type.

Seems like MapSet is missing an “is this a MapSet” guard like other opaque types have (for instance, :queue.is_queue/1) - that would let you write this:

  def new(type, %Coordinate{} = upper_left) do
    with [{_, _} | _] = offsets <- offsets(type),
         coordinates when is_mapset(coordinates) <- add_coordinates(offsets, upper_left) do
      {:ok, %__MODULE__{coordinates: coordinates, hit_coordinates: MapSet.new()}}
    end
  end
1 Like

Yep, this is the reason. Dialyzer deduces that the %MapSet{} match will always fail because it’s an opaque type and you’re not supposed to know that it is that struct. Thus the function will never return the success case. This is a sad deficiency in Elixir when using MapSet due to technical reasons surrounding Dialyzer and opaque types.

2 Likes

Ah, I think I understand now. This isn’t an Elixir pattern match issue but rather an issue where Dialyzer can’t “see into” opaque types, such as MapSet. Correct?

I knew it was an opaque type (at some point when I researched about the empty MapSet pattern matching) but was not considering this, and I apparently was confused by the fact that the pattern match on an empty MapSet does indeed work.

iex(1)> a = MapSet.new()                                                                                                                                           
#MapSet<[]>
iex(2)> %MapSet{} = b = a
#MapSet<[]>

So, matching on %MapSet{} is perfectly fine, unless you want to typespec things?

I might consider looking into contributing that.

interesting insight, TIL

Can’t the following work in lieu of something built in?

(disclaimer, untested code)


coordinates when is_struct(coordinates, MapSet) <- add_coordinates(offsets, upper_left)

1 Like

is_struct’s implementation might hit the same opaqueness behavior - for instance, :erlang.is_map_key/2’s second argument is typed as map() which won’t match MapSet.t():

It would likely work inside the MapSet module, though.

2 Likes

This is correct, there’s even one instance of this pattern in Elixir source code, and I tried to convince the Elixir team to change it, but I didn’t succeed. The Elixir team doesn’t respect opaqueness semantics dictated by Dialyzer in this case. The type is currently:

@opaque t(value) :: %{__struct__: MapSet, map: %{optional(value) => []}}

and what they mean is closer to (let’s invent a map type extension syntax):

@type t(value) :: %{__struct__: MapSet, ...internal(value)}
@opaque internal(value) :: %{map: %{optional(value) => []}}

but that can’t be achieved with current type syntax and Dialyzer. An easy fix would be to make the map field value an opaque type but then the name would need to be kept map even if internal implementation changes to something else. (Maybe the field could be renamed if it’s not a breaking change, I don’t know)

So maybe something like this can be proposed:

@type t(value) :: %{__struct__: MapSet, map: internal(value)}
@opaque internal(value) :: %{optional(value) => []}

I requested a related change to Erlang/OTP which would maybe make this (and other aspects of opaque types being annoying) less of a problem, it will be considered for Erlang/OTP 26: Dialyzer: Report opaqueness violation, but don't assume no return · Issue #5503 · erlang/otp · GitHub

2 Likes

While I agree it will be a valuable addition, I don’t think it will be possible to use it as a guard:

iex(1)> defmodule Hey do
...(1)> def f(x) when :queue.is_queue(x), do: x
...(1)> end
** (CompileError) iex:2: cannot invoke remote function :queue.is_queue/1 inside guards

So it would provide a way to check if something is a MapSet that doesn’t cause Dialyzer to complain, but using it would be a bit more verbose than in pattern matches. It should cover @bmitc 's use case, though, since it’s inside a function body.

It’s an unfortunate limitation, I think. As far as I can tell, it prevents any use of pattern matching against %MapSet{} in a Dialyzer compliant codebase. I think it may be beneficial to update the documentation here to mention this.

And it turns out that I’m an idiot. I didn’t notice my own note regarding ignoring the opaque warning:

  # Ignores the warning "Attempted to pattern match against the internal structure of an opaque term."
  # when pattern matching against `%MapSet{}` in `new/2`
  # @dialyzer {:no_opaque, new: 2}

which was there all along just above the definition of Island.new/2. The Dialyzer warning being ignored is:

lib/islands_engine/island.ex:48:opaque_match
Attempted to pattern match against the internal structure of an opaque term.

Type:
MapSet.t(_)

Pattern:
%MapSet{}

This breaks the opaqueness of the term.

I suppose this also teaches me to not ignore Dialyzer warnings unless I absolutely must and know that they will not affect things upstream. But I feel I’ve certainly gained some insight from this thread. Thanks to everyone contributing.

I did try implementing this in the MapSet module with:

  @spec is_mapset(term) :: boolean
  defguard is_mapset(term) when is_struct(term, __MODULE__)

and used like

coordinates when MapSet.is_mapset(coordinates) <- add_coordinates(offsets, upper_left)

but I got a new Dialyzer error that I also simply don’t understand:

lib/islands_engine/island.ex:50:guard_fail_pat
The clause guard cannot succeed. The pattern variable_coordinates was matched against the type {:error, :invalid_coordinate} | MapSet.t(_).

If anyone has any augmentations to the above or further suggestions about implementing an is_mapset either in Kernel or MapSet, let me know, and I can try them out. For now, I will simply remove the pattern matching from my code and deal with it. :slight_smile:

These are unfortunate errors. They are flat out wrong and are due to mismatches between how Dialyzer thinks about things versus how Elixir thinks about things.

My pull request was just merged, can you please try Elixir from the main branch and see if it resolves the issue (allows you to pattern match on MapSet without causing Dialyzer issues, and doesn’t introduce other Dialyzer issues)?

4 Likes

Thanks for the heads up. Luckily, I already had a Docker container setup for building Elixir for source from earlier, so I was able to test this quickly and easily. Unfortunately, it doesn’t seem to have made any difference. (Please see updates in comments below.)

I changed this line, which looks like:

{:ok, coordinates} <- add_coordinates(offsets, upper_left) do

to this:

{:ok, %MapSet{} = coordinates} <- add_coordinates(offsets, upper_left) do

The Dialyzer errors are:

lib/islands_engine/game.ex:130:pattern_match
The pattern can never match the type.

Pattern:
{:ok, _island}

Type:
{:error, :invalid_coordinate | :invalid_island_type}
________________________________________________________________________________
lib/islands_engine/island.ex:44:opaque_match
Attempted to pattern match against the internal structure of an opaque term.

Type:
MapSet.t(_)

Pattern:
{:ok, _coordinates = %MapSet{}}

This breaks the opaqueness of MapSet.t(_).

Note that I removed the ignore Dialyzer warning for the opaque pattern match, which is why it’s there now.

Just as a note, my code has been updated since the beginning of the post, but I did this check with the latest changes on my repository as well as the latest of Elixir, which I checked git log to double-check it had your change in there.

Just to make sure, this is after a clean compile (both Elixir and then your project) and with an updated Dialyzer PLT?

edit: I just tried on a fresh container with Erlang 24.3.4.1 and VSCode’s ElixirLS extension doesn’t report any issues in your repository after changing code to

{:ok, coordinates = %MapSet{}} <- add_coordinates(offsets, upper_left) do

So I think it should be good :tada:

2 Likes

It’s good to double-check! I re-did the experiment with:

  1. Delete the _build and deps directories from my Mix project
  2. In the Elixir repository, do a git pull --force and run git log which showed these as the most recent two commits:
    commit fa7b6a874a39fd5bcb443628bb491dc9490fd8bf (HEAD -> main, origin/main, origin/HEAD)
    Author: José Valim <jose.valim@dashbit.co>
    Date:   Sat Jun 11 09:32:28 2022 +0200
    
        Move custom formatting docs to Logger.Formatter
    
    commit 76febeabca1599994c206c7b803f6da6404ed53a
    Author: Michał Łępicki <michallepicki@users.noreply.github.com>
    Date:   Sat Jun 11 09:10:45 2022 +0200
    
        Change MapSet.t definition to make Dialyzer happy (#11917)
    
  3. Run make clean in the Elixir repository
  4. Run make compile with no issues
  5. Run make test and observe no failures
  6. Run mix deps.get in my repository
  7. Run mix dialyzer

This does indeed get rid of the Dialyzer errors (both the pattern_match and opaque_match errors). I thought I had gone through the proper steps before, but I was extra thorough this time. It might be good for you (or anyone else) to verify as well. You will have to make the change in my repository to add back in the %MapSet{} pattern match, since I’m going to leave that out until this makes it into an Elixir release.

Thanks for getting that change in!

(Edit: I see your update now. As a note, I am actually running Elixir main against Erlang/OTP 25.)

2 Likes