Elixir 1.17 should provide better feedback on dead code

[bugnano]

Elixir and Erlang/OTP versions

Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:6:6] [ds:6:6:10] [async-threads:1] [jit:ns]

Elixir 1.17.1 (compiled with Erlang/OTP 27)

Operating system

Ubuntu 24.04

Current behavior

In our codebase we have a guard defined like so:

  defguardp conn_scope(conn, target)
            when is_atom(target) and is_struct(conn, Conn) and
                   is_atom(conn.assigns.auth_profile.scope) and
                   conn.assigns.auth_profile == target

  defguardp session_scope(session, target)
            when is_atom(target) and is_struct(session, Session) and
                   is_atom(session.scope) and
                   session.scope == target

  defguard auth_scope(conn, target)
           when conn_scope(conn, target) or session_scope(conn, target)

so that we can use the guard auth_scope either with a parameter of type Conn or a parameter of type Session, and you can see that we use is_struct to restrict further guards to access the correct members of the appropriate struct.

With Elixir 1.17 the compiler gives the following warning:

     warning: unknown key .assigns in expression:

         session.assigns

     where "session" was given the type:

         # type: dynamic(%DataLayer.Session{
           acked_server_sequence: term(),
           app_id: term(),
           authenticated: term(),
           default_api_version: term(),
           expires_at: term(),
           expiry_timer: term(),
           external_user_id: term(),
           id: term(),
           permissions: term(),
           protocol_version: term(),
           scope: term(),
           sdk_name: term(),
           server_sequence: term(),
           socket_pid: term(),
           transport: term(),
           user_id: term()
         })
         # from: lib/backend_web/calls/conversation_calls.ex:110
         %DataLayer.Session{app_id: app_id} = session

     typing violation found at:
     │
 112 │       when auth_scope(session, :app) do
     │       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     │
     └─ lib/backend_web/calls/conversation_calls.ex:112: BackendWeb.ConversationCalls.delete/2

because it checks also for the guards of the other data structure.

Expected behavior

No warning being emitted, because is_struct narrows down subsequent guards to only 1 data type.

[josevalim]

Correct. the type system does not do narrowing (or occurrence typing) yet.

Can you provide more details about the code though? In the code snippet that you shared, there is no narrowing. You have simply specified it can be both using or.

[bugnano]

The code where the warning is is like so:

  def delete(
        %Call{params: %{"conversation_params" => conversation_param_or_id}} = call,
        %Session{app_id: app_id} = session
      )
      when auth_scope(session, :app) do

So there's a Session struct passed as a parameter to the function, and auth_scope is designed to work either with Session or with Conn.

Given that in the function definition we know that the parameter is a Session, the guards after is_struct(conn, Conn) should be ignored by the type checker, because is_struct(conn, Conn) is known to be false.

[josevalim]

Oh, I got it. We need to perform type refinement on the guard and we plan to support this in the next Elixir version. However I am afraid the type system will still warn you have dead code (i.e. part of your guard is always false). You would need to use session_scope if you only have a session.

[josevalim]

I changed the title to reflect that the error is correct but the message is wrong. It should rather say about dead code.

[bugnano]

I don't know if it's dead code.

I mean, on other parts of our codebase we use that guard with a Conn struct, so both parts of the guard are used, just not at the same time.

[josevalim]

The type system knows one of the sides of your or is always false because you have pattern matched on the struct. This is fine:

  def delete(
        %Call{params: %{"conversation_params" => conversation_param_or_id}} = call,
        session
      )
      when auth_scope(session, :app) do

But this means there is dead code inside the auth_scope guard since is_struct(session, Conn) is always false:

  def delete(
        %Call{params: %{"conversation_params" => conversation_param_or_id}} = call,
        %Session{app_id: app_id} = session
      )
      when auth_scope(session, :app) do

[bugnano]

Got it, thank you😁

[marcandre]

I changed the title to reflect that the error is correct but the message is wrong. It should rather say about dead code.

I'm trying to understand why the "error is correct"? IMO, there is no typing violation, there is no error in the code, dead code generated in a macro should not be an issue, and there should be no warning.

If I understand correctly, the LiveView fix is more of a hack and works because there is no real need to call __live__, otherwise the hack would fail if a module was passed as a variable instead of a literal module.

Maybe the name should be changed again? I did look for an open bug that mentionned "typing violation" and found none. The tags should also be changed as I believe it is not an enhancement but a bug.

[josevalim]

Generally speaking, we only ignore warnings from macros if they are tagged as generated: true. Which is supported for types as of #13727. In the absence of the annotation, it is treated as regular code, which is indeed “dead”.

[marcandre]

Got it, thanks.

[josevalim]

Some updates to the folks tracking this issue. Elixir v1.18 will perform inference of patterns but not of guards yet, which means a better error message for this case is scheduled for v1.19. I have also changed it from enhancements to bug, which @marcandre requested earlier on, and I forgot to do it.

Just a reminder, in this particular case, the warning will remain, since you could do a more precise check. However, the warning should say that is_struct(session, Conn) will always return false (if that is not possible, it will say that map_get(session, :__struct__) == Conn always returns false).

[josevalim]

Closing this in favor of #13227. In particular, the "type inference of guards" bit. :)