[osazemeu]

Closes #5509.

#5509 was closed without a fix. On it, @chrismccord wrote: "I'd happily review a PR that fixes this, but I'm not a dialyzer user myself so it's not something I'll be able to prioritize."

Problem

Phoenix.Channel.Server.join/4 is specced as taking a Phoenix.Socket.t(). Sockets built by Phoenix.ChannelTest did not satisfy that type in two ways:

1. socket/3 and connect/2 set transport: {Phoenix.ChannelTest, sup}, while Phoenix.Socket.t types transport as atom. The test supervisor was carried in the tuple so that join/4 could start channels under it.
2. Phoenix.Socket.t types topic as String.t() and channel_pid as pid, but both fields are nil until a channel is joined. A socket built by socket/3 has both set to nil. So does a socket built by Phoenix.Socket at connect time, so production code passes Server.join/4 sockets that do not match its spec either.

Dialyzer concludes that the join can never succeed, so the documented test pattern is reported as having no return. A minimal project that depends on Phoenix by path and calls socket/3 |> subscribe_and_join/3 in one function and connect/2 followed by subscribe_and_join/3 in another reports on main:

lib/repro.ex:25:7:no_return
Function join_lobby/0 has no local return.
________________________________________________________________________________
lib/repro.ex:34:8:unused_fun
Function topic_of/1 will never be called.
________________________________________________________________________________
lib/repro_connect.ex:6:7:no_return
Function connect_and_join_lobby/0 has no local return.
________________________________________________________________________________
lib/repro_connect.ex:12:8:unused_fun
Function topic_of/1 will never be called.
________________________________________________________________________________
done (warnings were emitted)

With this branch:

Total errors: 0, Skipped: 0, Unnecessary Skips: 0
done (passed successfully)

The repro code:

defmodule Repro.Channel do
  use Phoenix.Channel

  @impl true
  def join("room:" <> _id, _payload, socket), do: {:ok, socket}
end

defmodule Repro.Socket do
  use Phoenix.Socket

  channel "room:*", Repro.Channel

  @impl true
  def connect(_params, socket, _connect_info), do: {:ok, socket}

  @impl true
  def id(_socket), do: nil
end

defmodule Repro.ChannelHelpers do
  import Phoenix.ChannelTest

  @endpoint Repro.Endpoint

  def join_lobby do
    {:ok, _reply, socket} =
      Repro.Socket
      |> socket("user_id", %{})
      |> subscribe_and_join(Repro.Channel, "room:lobby")

    topic_of(socket)
  end

  defp topic_of(socket), do: socket.topic
end

defmodule Repro.ConnectHelpers do
  import Phoenix.ChannelTest

  @endpoint Repro.Endpoint

  def connect_and_join_lobby do
    {:ok, socket} = connect(Repro.Socket, %{})
    {:ok, _reply, socket} = subscribe_and_join(socket, Repro.Channel, "room:lobby")
    topic_of(socket)
  end

  defp topic_of(socket), do: socket.topic
end

Phoenix's CI does not run Dialyzer, so this repro is the evidence for the fix. I checked that each change on its own leaves the socket/3 case failing: with only the ChannelTest change, Dialyzer still rejects the nil topic and channel pid, and with only the type change, it still rejects the tuple transport.

Changes

Two commits:

1. Type topic and channel_pid as nilable in Phoenix.Socket.t. The type now matches what the struct holds before a join. transport stays atom.
2. Keep the ChannelTest supervisor in socket.private. transport is now Phoenix.ChannelTest, and the supervisor is stored under :phoenix_test_supervisor in socket.private, the field documented for library use. connect/2 adds it after the handler connects, because Phoenix.Socket builds a fresh socket that does not copy private. Channel.Server.join/4 keeps the socket's private and the channel process merges it with the channel's own, so the socket returned from join/4 can still be used to join again, including from another process. The supervisor is still resolved when the socket is built, so the :test_process option behaves as before.

I kept the supervisor in private instead of widening transport to accept a tuple. A tuple there would only be needed by test sockets, and every consumer that reads transport as an identifier would have to handle it.

Side effect

For ChannelTest sockets, the [:phoenix, :socket_connected] telemetry metadata and the connect log line now report Transport: Phoenix.ChannelTest instead of {Phoenix.ChannelTest, #PID<...>}.

Tests

In test/phoenix/test/channel_test.exs:

• The join/3 test now asserts socket.transport == Phoenix.ChannelTest instead of matching the tuple.
• The same join/3 test asserts that the joined socket can join again.
• The connect/2 test asserts socket.transport == Phoenix.ChannelTest.

The existing tests that join from another process and re-join from a joined socket pass unchanged. mix test --warnings-as-errors passes locally.

I also checked phoenix_live_view: it only calls the public connect/2, subscribe_and_join/3 and push/3 functions and never reads the transport tuple.

lib/phoenix/test/channel_test.ex and test/phoenix/test/channel_test.exs already fail mix format --check-formatted on main. This PR does not add any new formatting differences, and I left the existing ones alone to keep the diff focused.

The CHANGELOG has a new "Bug fixes" section in the v1.9 entry.

[SteffenDE]

Thanks!