Possible incorrect warning on dynamic module calls

[wkirschbaum]

Elixir and Erlang/OTP versions

Elixir 1.18.0-dev (4328b09) (compiled with Erlang/OTP 27)

Operating system

Linux

Current behavior

With the following code snippet:

defmodule Foo do
  def something(input) do
    case module(input) do
      :error -> ""
      module -> module.zar()
    end
  end

  def zar() do
  end

  defp module(input) do
    if input == "bar" do
      :error
    else
      Foo
    end
  end
end

I get the following warning:

    warning: :error.zar/0 is undefined (module :error is not available or is yet to be defined)
    │
  5 │       module -> module.zar()
    │                        ~
    │
    └─ foo.exs:5:24: Foo.something/1

Expected behavior

The handles the :error case, so the warning is a false positive.

[josevalim]

You are correct! This should be fixed on Elixir v1.19, you can track this issue: #13227

Once we understand guards, we will be able to know the second clause can only be Foo (or atom() and not :error). Meanwhile you can tag the argument. I will close this as duplicate but feel free to ask if you have questions!

[wkirschbaum]

What do you mean by "tag the argument"?

I see this error popping up on some libraries we use and they build with --warnings-as-errors, so will fail on 1.18 builds.

[wojtekmach]

--warnings-as-errors would not fail on warnings from dependencies. (Strictly speaking it is a flag recognized by mix compile.elixir but ignored by mix deps.compile.)

[wkirschbaum]

@wojtekmach yes, correct. I just meant there are libraries out there with this issue and adding 1.18 to the ci-runs would mean they either need to remove the flag or modify the code ( which might not be trivial )... something to keep in mind.

[josevalim]

@wkirschbaum make it return {:ok, mod} or :error instead of overlapping modules with the atom :error. After all, you could define a module name :error. The tagging will remove any ambiguity.

[wkirschbaum]

thanks, it does make more sense. quick uncomplicated solution. 👍🏼

[dlobo]

Similar issue, and wondering how we can rewrite the code to make it more elixir-y (or how can we do it in a better way without triggering the warning). Details as follows:

In module A

  @spec submit_for_approval(map()) :: {:ok, SessionTemplate.t()} | {:error, String.t()}
  defp submit_for_approval(attrs) do
    Logger.info("Submitting template for approval with attrs as #{inspect(attrs)}")
    bsp_module = Provider.bsp_module(attrs.organization_id, :template)
    bsp_module.submit_for_approval(attrs)
  end

In module Provider:

def bsp_module(org_id, :template) do
    organization = Glific.Partners.organization(org_id)

    organization.bsp.shortcode
    |> case do
      "gupshup" -> Glific.Providers.Gupshup.Template
      "gupshup_enterprise" -> Glific.Providers.GupshupEnterprise.Template
      _ -> raise("#{organization.bsp.shortcode} Provider Not found.")
    end
  end

Happy to clean up our code base to improve it, I tried searching, and this seemed the closest issue. The warning I am getting is:

warning: Glific.Providers.GupshupEnterprise.submit_for_approval/1 is undefined (module Glific.Providers.GupshupEnterprise is not available or is yet to be defined)
     │
 270 │     bsp_module.submit_for_approval(attrs)
     │                ~
     │
     └─ lib/glific/templates.ex:270:16: Glific.Templates.submit_for_approval/1

     warning: Glific.Providers.Gupshup.import_templates/2 is undefined (module Glific.Providers.Gupshup is not available or is yet to be defined)
     │
 278 │     Provider.bsp_module(org_id, :template).import_templates(org_id, data)
     │                                            ~
     │
     └─ lib/glific/templates.ex:278:44: Glific.Templates.import_templates/2

Elixir 1.18.1

[josevalim]

I think in your case the warning is correct. Otherwise how can you be sure that Provider.bsp_module(attrs.organization_id, :template) will never return Glific.Providers.GupshupEnterprise which does not have such function? It seems a viable result to have depending on the runtime values.

If it is still a false positive, there are a few options:

1. Provide more specific bsp_module_for_submit_approval that only return valid modules

2. Remove the optional callback/function and implement them for all templates. If they are not meant to be invoked, make the callback raise instead, saying that code path should never have been executed

[madclaws]

@josevalim What about using Protocols for preventing this warning. Would it be an overkill?.

[josevalim]

You can use protocols but we want to solve behaviours at some point too.