Missing from spec: atom()

I’m trying to add typespecs to the functions created by phx.gen.auth. One of them is Accounts.User.validate_current_password/2 which is reproduced here for completeness, along with my attempt at a typespec:

  @spec validate_current_password(Ecto.Changeset.t(), String.t()) :: Ecto.Changeset.t()
  def validate_current_password(changeset, password) do
    if valid_password?(changeset.data, password) do
      changeset
    else
      add_error(changeset, :current_password, "is not valid")
    end
  end

When running mix dialyzer with missing_return enabled, I get the following error:

lib/leaf/accounts/user.ex:171:missing_range
The type specification is missing types returned by function.

Function:
Leaf.Accounts.User.validate_current_password/2

Type specification return types:
%Ecto.Changeset{
  :action => atom(),
  :changes => %{atom() => _},
  :constraints => [
    %{
      :constraint =>
        binary()
        | %Regex{
            :opts => binary() | [any()],
            :re_pattern => _,
            :re_version => _,
            :source => binary()
          },
      :error_message => binary(),
      :error_type => atom(),
      :field => atom(),
      :match => :exact | :prefix | :suffix,
      :type => :check | :exclusion | :foreign_key | :unique
    }
  ],
  :data => nil | map(),
  :empty_values => _,
  :errors => Keyword.t({binary(), Keyword.t()}),
  :filters => %{atom() => _},
  :params => nil | %{binary() => _},
  :prepare => [(_ -> any())],
  :repo => atom(),
  :repo_opts => Keyword.t(),
  :required => [atom()],
  :types =>
    nil
    | %{
        atom() =>
          atom()
          | {:array | :assoc | :embed | :in | :map | :maybe | :param, _}
          | {:parameterized, atom(), _}
      },
  :valid? => boolean(),
  :validations => Keyword.t()
}

Missing from spec:
atom()

My question is, where does that atom() come from, and how do I get rid of the error?

After fiddling around a bit, the error still happens if both branches of the if return changeset, but not if I remove the if completely and just have the function returning changeset directly, e.g. this produces the same error:

  @spec validate_current_password(Ecto.Changeset.t(), String.t()) :: Ecto.Changeset.t()
  def validate_current_password(changeset, password) do
    if valid_password?(changeset.data, password) do
      changeset
    else
      changeset
    end
  end

But this works:

  @spec validate_current_password(Ecto.Changeset.t(), String.t()) :: Ecto.Changeset.t()
  def validate_current_password(changeset, password) do
    changeset
  end

I suppose it’s because of the access usage, since this could also imply that changeset is a module (thus atom()) and data is its function. What’s the typespec of valid_password?/2?

Here is the definition and type spec for valid_password?/2:

  @spec valid_password?(__MODULE__.t(), String.t()) :: boolean()
  def valid_password?(%Leaf.Accounts.User{hashed_password: hashed_password}, password)
      when is_binary(hashed_password) and byte_size(password) > 0 do
    Bcrypt.verify_pass(password, hashed_password)
  end

  def valid_password?(_, _) do
    Bcrypt.no_user_verify()
    false
  end

I suppose it’s because of the access usage, since this could also imply that changeset is a module (thus atom()) and data is its function.

That’s possible, but still weird since I have given the type of changeset in the spec.

I’m seeing this too.

Produces Missing from spec: atom():

case conn.assigns.foo do
  _ -> conn
end

Doesn’t produce:

%Plug.Conn{assigns: %{foo: foo}} = conn
case foo do
  _ -> conn
end

I’m guessing this post should have had the tag dialyzer and/or dialyxir… I can’t seem to add it now.

As usual, dialyzer is always right :smile:

case x.y do
 _ -> x
end

gets expanded to something like

case (case x do
        %{y: var2} ->
          var2

        var2 when :erlang.is_atom(var2) and var2 !== nil and var2 !== true and var2 !== false ->
          var2.y()

        var2 ->
          :erlang.error({:badkey, :y, var2})
      end) do
  _ -> x
end

x.y is basically Map.fetch!(x, :y) if x is a map, but can also be x.y() if x is a module atom.

iex(1)> x = :math
:math
iex(2)> x.pi
3.141592653589793

So, there are two ways this function succeeds: if x is a map or if it is an atom ; so is the return type.

If you add an extra guard to make sure x is a map, like %{} = x, the error disappears.

2 Likes

Right, but at the risk of going in circles it’s super counter-intuitive because we have told dialyzer that x is a struct/map in the spec, so (incorrectly) expect it not to consider the case where x is a module/atom.

It seems that what dialyzer is saying here is “You said you expect a struct and return a struct, but actually you might also return an atom” and leaves the rest implicit: if somebody gives you a module instead of the struct you said you were expecting.

The example was helpful so here’s my own to be completely explicit:

defmodule Example do
  @spec demo(map()) :: map() # Missing from spec: atom()
  def demo(map) do
    case map.key_looks_like_a_function_call do
      _ -> map
    end
  end

  def key_looks_like_a_function_call, do: "It is!"
end

Using it:

iex(1)> Example.demo(%{key_looks_like_a_function_call: "It's not"})
%{key_looks_like_a_function_call: "It's not"}
iex(2)> Example.demo(Example)
Example # atom()

My inner voice then wonders “What’s even the point of the spec on the parameter if it’s ignored?”

It’s because although Dialyzer ignores the input spec to check the outputs, it still uses the input spec for other checks, including to make sure you are not writing code like in the above iex session. If we put that into a function, dialyzer will generate the following warning:

Example.demo(Example)

breaks the contract
(map()) :: map()

2 Likes

I agree it would ideally be able to track this, but I guess by construction it doesn’t trust the spec and checks all clauses of a match.
It seems to be inferring its success typing and then checks if it matches the spec.

  @spec is_map(map()) :: true
  def is_map(%{}), do: true
  def is_map(_), do: false
Type specification return types:
true

Missing from spec:
false

it’s super counter-intuitive

I don’t disagree, dialyzer is counter-intuitive in many regards :smile:

1 Like

Thank you, that was exactly it! I have liberally sprinkled is_map guards all over my code, and dialyzer is happy now :smiley:

1 Like