False positive with dialyzer?

Questions / Help
1

Background

I am reading Designing Elixir Systems with OTP and while coding the examples I am trying to add dialyzer specs.

Now, one recent issue I find, is that no matter what, Dialyzer will complain about a spec I have. I am very very convinced dialyzer is actually wrong, which shouldn’t be a thing.

Code

Following is the code. I define a type called quiz_info which is a fancy map. Then I have 2 private functions that start quizzes.

  @type quiz_info :: %{
    fields: map, 
    templates: [keyword], 
    start_at: DateTime.t, 
    end_at: DateTime.t
  }

  # Divides ready to start quizzes from not ready to start ones. Starts the quizzes that are ready
  # and returns the ones that are not.
  @spec start_quizzes([quiz_info], DateTime.t) :: [quiz_info]
  defp start_quizzes(quizzes, now) do
    {ready, not_ready} = Enum.split_while(quizzes, fn quiz -> 
      date_time_less_than_or_equal?(quiz.start_at, now)
    end)

    Enum.each(ready, &start_quiz(&1, now))
    not_ready
  end

  # starts a quizz. Sends a message to itself once a quizz time is up.
  @spec start_quiz(quiz_info, DateTime.t) :: reference
  defp start_quiz(quiz, now) do
    QuizManager.build_quiz(quiz.fields)
    Enum.each(quiz.templates, &add_template(quiz, &1))
  
    timeout = 
      quiz.end_at
      |> DateTime.diff(now, :millisecond)
      |> time_to_finish()

    Process.send_after(self(), {:end_quiz, quiz.fields.title}, timeout)
  end

So, my main function start_quizzes receives a list of quiz_info and a date, and returns a list of quiz_info.
Dialyzer doesn’t complain here. But it complains on the next one, start_quiz.

Error

The @spec for the function does not match the success typing of the function.

Function:
Mastery.Boundary.Proctor.start_quiz/2

Success typing:
@spec start_quiz(atom(), %DateTime{
  :calendar => atom(),
  :day => pos_integer(),
  :hour => non_neg_integer(),
  :microsecond => {char(), 0 | 1 | 2 | 3 | 4 | 5 | 6},
  :minute => non_neg_integer(),
  :month => pos_integer(),
  :second => non_neg_integer(),
  :std_offset => integer(),
  :time_zone => binary(),
  :utc_offset => integer(),
  :year => integer(),
  :zone_abbr => binary()
}) :: reference()

According to dialyzer, my start_quiz function which receives a quiz_info should be receiving a atom. I have no idea where this comes from.

Question

What am I missing?

2

What’s the type/spec of add_template/2?

3 
  @spec add_template(quiz_info, keyword) :: :ok
  defp add_template(quiz, template_fields), do:
    QuizManager.add_template(quiz.fields.title, template_fields)
4

Sorry, but I can’t find anything in the function that would dialyzer make thing that the first argument were an atom, sorry…

1 Like
5

It’s OK, I know the feeling. Quite frustrating, if only dialyzer was more user friendly!
Thanks for trying though!

6

Have you tried bisecting the function body?

If you comment everything out, and then comment back in, line by line, which one causes dialyzer to complain? And what lines before that can you comment out again to keep the warning?

1 Like
7

Could you show where the start_quizzes/2 is being called?

8

Simple and yet effective idea!

I have bisected the code of the function and found the line that causes dialyzer to error out:

  defp start_quiz(quiz, now) do
    Logger.info("starting quiz #{quiz.fields.title}...")
    
    ## This line causes dialyzer to think that `start_quiz` takes 
    ## an atom as input.
    # QuizManager.build_quiz(quiz.fields)
    
    Enum.each(quiz.templates, &add_template(quiz, &1))
  
    timeout = 
      quiz.end_at
      |> DateTime.diff(now, :millisecond)
      |> time_to_finish()

    Process.send_after(self(), {:end_quiz, quiz.fields.title}, timeout)
  end

So my question is, what is the spec of QuizManager.build_quiz/1 ? Lets see:

  @spec build_quiz(module, keyword) :: :ok
  def build_quiz(manager \\ __MODULE__, quiz_fields), do:
    GenServer.call(manager, {:build_quiz, quiz_fields})

We see that it actually takes 2 arguments. 1 optional, one mandatory. Since module() is actually equivalent to atom(), I wonder if this is the source of the error?

Sure, its called on Callbacks (GenServer ones):

  @impl GenServer
  def handle_call({:schedule_quiz, quiz }, _from, quizzes) do
    now = DateTime.utc_now
    ordered_quizzes = 
      [quiz | quizzes]
      |> start_quizzes(now)
      |> Enum.sort(fn a, b -> 
        date_time_less_than_or_equal?(a.start_at, b.start_at)
      end)

    build_reply_with_timeout({:reply, :ok}, ordered_quizzes, now)
  end

  @impl GenServer
  def handle_info(:timeout, quizzes) do
    now = DateTime.utc_now()
    remaining_quizzes = start_quizzes(quizzes, now)
    build_reply_with_timeout({:noreply}, remaining_quizzes, now)
  end

I personally don’t see any redflags here.

9

Perhaps something is indeed wrong with the generated specs…

Can you handroll the two functions and specs here?

@spec build_quiz(keyword) :: :ok
def build_quiz(fields), do: build_quiz(__MODULE__, fields)

# original spec and implementation but without the default value

And in another experiment swap the arguments, so making it build_quiz(fields, manager \\ __MODULE__) (adjust the spec accordingly). If neither of these results in the message you observe, then it’s probably a bug in elixirs code generation. Then please check if thats still causing issues in the latest release and perhaps file a bug.

10

I have changed the specs and code to this version (above) but I still get the same error in the same function. It is weird because atom comes from nowhere… Even without the module it still complains.

Here are the function’s code:

  @spec build_quiz(module, keyword) :: :ok
  def build_quiz(manager \\ __MODULE__, quiz_fields), do:
    GenServer.call(manager, {:build_quiz, quiz_fields})

  @impl GenServer
  def handle_call({:build_quiz, quiz_fields}, _from, quizzes) do
    quiz = Quiz.new(quiz_fields)

    # Using a title as UUID is a trade off in the name of simplicity
    # https://elixirforum.com/t/designing-elixir-systems-with-otp-pragprog/21626/25?u=fl4m3ph03n1x
    new_quizzes = Map.put(quizzes, quiz.title, quiz)
    {:reply, :ok, new_quizzes}
  end

I don’t see anything wrong with the specs.

The full code replicating the bug can be found here:

https://github.com/Fl4m3Ph03n1x/quizz_mastery/tree/bug_dialyzer

I am inclined towards believing this is a bug, but if I can’t find the exact cause I can’t file it.

These are my version of elixir:

elixir 1.9.1-otp-22
erlang 22.1.8

And I am using:


{:dialyxir,       "~> 1.0.0-rc.7",  only: [:dev],         runtime: false}
1 Like
11

Thank you for providing the full sources! I’ll try to experiment a bit during my spare time as well.

12

The spec for Quiz.build_quiz is incorrect. It states that fields is keyword, while in quiz_info the fields field is declared as a map. If we change build_quiz spec into @spec build_quiz(module, map) :: :ok, the dialyzer doesn’t complain anymore.

As often, the dialyzer message is sadly not very helpful, so it takes a bit of experimenting, commenting out lines, and looking through the callstack to find out the root cause.

6 Likes
13

This is rather confusing to me.

You are right in that the specs were incorrect. In reality fields is of type Enum.t. Sometimes the authors use a map, other times they use a keyword list. It is a side effect of having 2 people authoring the same book (I have read this in the book club thread). Please notice this is not a criticism of any kind, I still enjoy their work and recommend their book.

What really puts me against the wall here, is that the dialzyer error message quite misleading. The issue is not that the function start_quiz expects an atom, the issue is the type fileds in build_quiz has a conflicting type. Both things are very different and the message truly threw me in the wrong direction.

Is this considered a bug? Is it a feature? I have a hard time telling. (Should I even report this? What do you guys think?)

Thanks everyone for the help, I truly appreciate it.

1 Like
14

I think that part of the problem is Elixir’s convenient support for accessing map fields. Basically, foo.bar can be interpreted as either: fetch the key :bar from the map foo, or invoke the function bar from the module represented by the var foo. In the latter case, foo is of the atom type, and I presume that this is part of the reason why dialyzer ultimately concludes that the variable should be an atom.

I’m not familiar enough with dialyzer internals to comment whether something can be improved here. Either way, I agree that UX is terrible (it usually is, but this example is particularly nasty). While I use dialyzer regularly in projects I work on, and I’ve acquired enough practice to usually get to the root cause of the problem reasonably quickly, I’m far from happy with it. Unfortunately, AFAIK, it’s the best thing we’ve got at the moment :frowning:

7 Likes