Dialyzer, giving weird warning

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’.

This seems similar:

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.

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

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