Elixir: Dialyzer only taking one path in `with` statement

Created on 5 Jan 2018  Â·  10Comments  Â·  Source: elixir-lang/elixir

Precheck

  • Do not use the issues tracker for help or support (try Elixir Forum, Stack Overflow, IRC, etc.)
  • For proposing a new feature, please start a discussion on the Elixir Core mailing list
  • For bugs, do a quick search and make sure the bug has not yet been reported
  • Finally, be nice and have fun!

Environment

  • Elixir & Erlang versions (elixir --version): Elixir 1.5.2 & 1.6.0-rc.0 & 1.7.0-dev with Erlang/OTP 20
  • Operating system: Linux 4.14.11-1-ARCH

Current behavior

The following function will incorrectly be assumed to always return {:error, _} by dialyzer:

@spec read(url :: String.t()) :: {:ok, [Downloader.Torrent.t()]} | {:error, String.t()}
def read(url) do
  with {:ok, %HTTPoison.Response{body: body}} <- HTTPoison.get(url),
       {:ok, %FeederEx.Feed{entries: entries}, _} <- FeederEx.parse(body) do
    {:ok, Enum.map(entries, &Downloader.Torrent.from_entry/1)}
  else
    {:error, %HTTPoison.Error{reason: reason}} ->
      {:error, reason}

    {:fatal_error, _, reason, _, _} ->
      {:error, reason}
  end
end

If one were to change the second else clause to return {:fatal_error, reason} it still will not recognize that there is one more possible return value, but will still insist that only {:error, _} can be returned.

When matching with the following pattern:

case Downloader.RSS.read(state.url) do
  {:ok, ts} ->
    ts

  {:error, reason} ->
    Logger.error("Error reading '#{state.url}': #{reason}")
    []
end

will give a dialyzer error stating: The pattern {'ok', Vts@1} can never match the type {'error',_}

Expected behavior

To have no dialyzer errors.

Elixir Needs more info

Most helpful comment

There is also #6738. This is a known bug due to how we compile with and it will be fixed in future releases once we contribute some patches to erlang to perform function inlining.

All 10 comments

Can you please provide a sample application that reproduces the error after a simple command, such as mix dialyzer? Or send a patch that adds a failing test case to our suite?

Thanks!

Here is an issue that may be related: #6738

Btw, is there any chance that one of the two with conditions will never match, meaning it will indeed always returns {:error, _}?

I will create an app to make this easily reproducable. If one of the errors wouldn't hit (according to spec it should be fine and it works in practice with malformed XML, so I know it can hit the :fatal_error one) I would still expect dialyzer to tell me {:ok, [Downloader.Torrent.t()]} is a possible return value, but it simply doesn't think so.

I've made a repo for this: https://github.com/GoNZooo/with_only_one_path

There's some additional info and repro steps in the README.

Thanks @GoNZooo. I was able to reproduce the issue although I cannot reproduce it if I replace the call to FeederEx by something else with a similar return type.

This Erlang file can also reproduce the error and there is nothing suspicious about it:

-module(sample).
-export([read/0,use_read/0]).

read() ->
    case file:read_file(<<"example.com">>) of
        {ok,_} ->
            case 'Elixir.FeederEx':parse(<<"incorrect xml here">>) of
                {ok,#{'__struct__' := 'Elixir.FeederEx.Feed'}} ->
                    ok;
                _@1 ->
                    case _@1 of
                        {fatal_error,_,_,_,_} ->
                            feeder_error;
                        _@2 ->
                            error({with_clause,_@2})
                    end
            end;
        _@1 ->
            case _@1 of
                {error, _} ->
                    httpoison_error
            end
    end.

use_read() ->
    case read() of
        ok ->
            successful_read;
        httpoison_error ->
            httpoison_error;
        feeder_error ->
            feeder_error
    end.

So it is not an Elixir bug. It may be either a dialyzer bug or a bug in the feeder dependency.

Therefore I would like to ask you to reproduce the issue without the FeederEx dependency. The feeder library defines a very poor type for its stream/2 function and it is unclear if that is affecting the results. Maybe you can try calling xmerl_sax_parser directly. If we have a minimal case, we can report it to upstream to the OTP team.

Ok, so after testing a few versions of this it does seem pretty conclusively that feeder (by way of FeederEx) was causing it. Both xmerl_sax_parser and Scrape did fine.

Thanks for taking a serious look at it! If there's a conclusion I guess it's that underspecs can cause dialyzer to act pretty strangely?

Thanks for taking a serious look at it! If there's a conclusion I guess it's that underspecs can cause dialyzer to act pretty strangely?

I am not proficient enough in Dialyzer to say if that's the conclusion or if it is a bug. Maybe @fishcakez is though. :)

In any case, closing this as it is decidedly not an Elixir bug.

I don't think this is related to underspecs. I looked at the repo and this issue and FeederEx.parse(body) is supposed to be returning a 2-tuple in one and a 3-tuple in another but it may not matter here.

My assumption is that dialyzer believes that FeederEx.parse/1 is not going to return {:ok, %FeederEx.Feed{}, _}, {:ok, %FeederEx.Feed{}} or {:fatal_error, _, _, _, _}. Therefore the only way this function does not raise is if HTTPoison.get/1 returns an {:error, _} tuple.

I was considering opening a new issue, but I found this issue and feel like it describes my case as well.

I think the error in this case is related to the use of raise in the with/else branches.

Code

  def hello do
    with {:value1, :ok} <- {:value1, get_value()},
         {:value2, :ok} <- {:value2, get_value()} do
      :ok
    else
      {:value1, :error} ->
        raise "value1 error"

      {:value2, :error} ->
        raise "value2 error"
    end
  end

  defp get_value do
    if :rand.uniform() >= 0.5, do: :ok, else: :error
  end

Current behavior

mix dialyzer outputs the following:

➜  dialyzer_path mix dialyzer
Compiling 1 file (.ex)
Generated dialyzer_path app
Finding suitable PLTs
Checking PLT...
[:compiler, :dialyxir, :elixir, :kernel, :logger, :stdlib]
PLT is up to date!
Starting Dialyzer
[
  check_plt: false,
  init_plt: '/Users/percy/Code/dialyzer_path/_build/dev/dialyxir_erlang-20.3.4_elixir-1.7.2_deps-dev.plt',
  files_rec: ['/Users/percy/Code/dialyzer_path/_build/dev/lib/dialyzer_path/ebin'],
  warnings: [:unknown]
]
Total errors: 1, Skipped: 0
done in 0m1.21s
lib/dialyzer_path.ex:16:pattern_match
The pattern
{:value1, :error}

can never match the type
{:value2, :error}
________________________________________________________________________________
done (warnings were emitted)

Expected behavior

No errors from mix dialyzer.

Steps to reproduce

Additional data

Changing the raise lines to tuples removes the errors:

  def hello do
    with {:value1, :ok} <- {:value1, get_value()},
         {:value2, :ok} <- {:value2, get_value()} do
      :ok
    else
      {:value1, :error} ->
        {:error, :value1}

      {:value2, :error} ->
        {:error, :value2}
    end
  end

  defp get_value do
    if :rand.uniform() >= 0.5, do: :ok, else: :error
  end
➜  dialyzer_path mix dialyzer
Compiling 1 file (.ex)
Finding suitable PLTs
Checking PLT...
[:compiler, :dialyxir, :elixir, :kernel, :logger, :stdlib]
PLT is up to date!
Starting Dialyzer
[
  check_plt: false,
  init_plt: '/Users/percy/Code/dialyzer_path/_build/dev/dialyxir_erlang-20.3.4_elixir-1.7.2_deps-dev.plt',
  files_rec: ['/Users/percy/Code/dialyzer_path/_build/dev/lib/dialyzer_path/ebin'],
  warnings: [:unknown]
]
Total errors: 0, Skipped: 0
done in 0m1.15s
done (passed successfully)

There is also #6738. This is a known bug due to how we compile with and it will be fixed in future releases once we contribute some patches to erlang to perform function inlining.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Irio picture Irio  Â·  3Comments

lukaszsamson picture lukaszsamson  Â·  3Comments

eproxus picture eproxus  Â·  3Comments

ckampfe picture ckampfe  Â·  3Comments

LucianaMarques picture LucianaMarques  Â·  3Comments