Dialyzer, giving weird warning

Questions / Help
, , , , ,
1 
def create(conn, params) do
    with(
      %{status: "A"} = user <- UserController.get_user_by(String.trim(params["username"])),
      ldap when not is_nil(ldap) <- SystemSettings.ldap_params(),
      {:ok, pid} <- Exldap.open(ldap.host, ldap.port, false),
      :ok <- Exldap.verify_credentials(pid, "#{user.username}@#{ldap.host}", params["password"]),
      true <- is_nil(get_session(conn, :current_user)),
      {:ok, _log} <- Activity.create_user_log(%{user_id: user.id, activity: "logged in"}),
      logon_dt = Timex.format!(Timex.local(), "%Y-%m-%d %H:%M:%S", :strftime),
      remote_ip = conn.remote_ip |> :inet.ntoa() |> to_string(),
      {:ok, user} <- Accounts.update_user(user, %{remote_ip: remote_ip, last_login: logon_dt})
    ) do
      conn
      |> put_session(:current_user, user.id)
      |> put_session(:session_timeout_at, session_timeout_at())
      |> redirect(to: Routes.user_path(conn, :dashboard))
    else
      error ->
        error = if(is_nil(error), do: "User/Ldap params not found", else: error)  #the warning is on this line"
        Logger.error("Login error: " <> inspect(error))
        conn
        |> put_flash(:error, "Username/password not match")
        |> put_layout(false)
        |> render("index.html")
    end
  end

The test false == nil can never evaluate to ā€˜trueā€™.

2

This seems similar:

3

You might learn something if you separate your else to have clauses for both false and nil as well as the existing code for a general catch. The dialyzer errors might point you to a more specific problem. Right now it seems to think that the only possible value for error is false, and I donā€™t see how that can be the case since your various database access should also be able to fail.

4

This is a Dialyzer warning that points to potentially ā€œunreachableā€ code. If youā€™re sure that this can be nil, then maybe Dialyzer is warning here either because SystemSettings.ldap_params() has a spec but doesnā€™t have nil in return type, or because it is being run in a mix environment in which nil can never be returned by that function (e.g. in test and thereā€™s always some value returned in that mix env). If this is the latter, then it can be probably safely ignored.

1 Like
5

That makes some sense, but I would expect different wording in the error if ldap_params couldnā€™t return nil, because all those other matches should still be capable of failing. Or there should be more errors reporting that for example The test {:error, :couldnt_open_ldap} == nil can never evaluate to true. There appear to be 4 matches that should return something other than false or nil, and those should still cause some kind of error. The reported error by itself doesnā€™t make sense.

Unless @VincentBlackdot was only mystified by this one error and didnā€™t tell us about other ones on the same line. In which case your explanation is entirely plausible.

1 Like