Compiler warning on app cldr module since 2.8.0

[lucacorti]

On 2.8.0 a compiler warning is generated when compiling the app Cldr module:

warning: MyApp.Cldr.List.wrap/1 is undefined or private

This wouldn't be a problem but is generated when compiling the app, not ex_cldr_territories and if you have a strict no warnings policy on your code (e.g. mix compile --warnings-as-errors) like we do it fails in CI.

[Schultzer]

Thanks for this report, can you tell me what other cldr modules you are using in your backend, and if you happen to alias any of them.

[lucacorti]

This is the dependency list for ex_cldr related modules:

ex_cldr_currencies   2.15.0   2.15.0  Up-to-date
ex_cldr_dates_times  2.13.3   2.13.3  Up-to-date
ex_cldr_lists        2.10.0   2.10.0  Up-to-date
ex_cldr_messages     0.14.1   0.14.1  Up-to-date
ex_cldr_plugs        1.3.0    1.3.0   Up-to-date
ex_cldr_territories  2.7.0    2.8.0   Update not possible
ex_cldr_units        3.16.2   3.16.2  Up-to-date
ex_money             5.13.0   5.13.0  Up-to-date

If you mean aliases in the app module doing use Cldr, then no no aliases there, hust the use macro invocation.

[Schultzer]

@kipcole9 I believe that due to the nested module structure and that Cldr.List might be compiled before Cldr.Territory, then List ends up resolving MyApp.Cldr.List instead of Elixir.List when compiling Cldr.Territory.

https://github.com/elixir-cldr/cldr_lists/blob/master/lib/cldr/list/backend.ex

defmodule Cldr.List.Backend do
  def define_list_module(config) do
    module = inspect(__MODULE__)
    backend = config.backend
    config = Macro.escape(config)

    quote location: :keep, bind_quoted: [module: module, backend: backend, config: config] do
      defmodule List do
        @moduledoc false
      end
    end
  end
end

My first thought was to write out the fully qualified name as you have done here: Elixir.List.to_tuple(), but that doesn't feel right to me, do you know if there is any other way to solve this or is this a limitation on how the modules are compiled?

[lucacorti]

When generating code in other modules through macros, I usually fully specify module names in generated code exactly for this reason. It's easy to generate collisions and more importantly the macro user is unaware of the aliasing being done, which can have this kind of hard to pinpoint behaviour.

Or you could alias with a more specific name, like alias Cldr.List, as: CldrList if you don't want to use the full module name.

[Schultzer]

When generating code in other modules through macros, I usually fully specify module names in generated code exactly for this reason. It's easy to generate collisions and more importantly the macro user is unaware of the aliasing being done, which can have this kind of hard to pinpoint behaviour.

Or you could alias with a more specific name, like alias Cldr.List, as: CldrList if you don't want to use the full module name.

I agree with all this; FWIW, I'm not saying this is because of an explicit alias Cldr.List, but rather that the nesting of modules would automatically alias it since the module name is List.

[kipcole9]

The module nesting is deliberate for ex_cldr backends to ensure they are scoped to a given backend (ie MyApp.Cldr.*). But the side effect of automatic aliasing is tricky. Thankfully it (as far as I can tell) should only affect other backend modules and therefore ex_cldr lib implementers - not consumer code.

Therefore while not beautiful, either explicitly calling Elixir.List.wrap/1, or the alias suggestion, would be my proposal.

I'm still not clear on how the MyApp.Cldr.List is being aliased into MyApp.Cldr.Territory as part of the backend macro expansion process so I'm hesitate to make wholesale changes until I understand that clearly.

[Schultzer]

The module nesting is deliberate for ex_cldr backends to ensure they are scoped to a given backend (ie MyApp.Cldr.*). But the side effect of automatic aliasing is tricky. Thankfully it (as far as I can tell) should only affect other backend modules and therefore ex_cldr lib implementers - not consumer code.

Therefore while not beautiful, either explicitly calling Elixir.List.wrap/1, or the alias suggestion, would be my proposal.

I'm still not clear on how the MyApp.Cldr.List is being aliased into MyApp.Cldr.Territory as part of the backend macro expansion process so I'm hesitate to make wholesale changes until I understand that clearly.

I've pushed a failing test here: #36.

defmodule TestAllBackends.Cldr do
  require Cldr.Territory.Backend

  use Cldr,
    default_locale: "en-001",
    locales: ["en-001"],
    providers: [Cldr.Calendar, Cldr.DateTime, Cldr.Language, Cldr.List, Cldr.Message, Cldr.Number, Cldr.Territory, Cldr.Unit]
end

The above code produces a warning when compiled since this generated function is calling List.wrap/1: https://github.com/Schultzer/cldr_territories/blob/master/lib/cldr/backend.ex#L815

[lucacorti]

I guess explicitly referencing Elixir.List.wrap() is a good way to quickly solve this specific issue.

[Schultzer]

I guess explicitly referencing Elixir.List.wrap() is a good way to quickly solve this specific issue.

Thanks, I've pushed a new release. @kipcole9, let me know if you have any thoughts on what else we can do.