elixir-cldr / cldr_units

Unit formatting (volume, area, length, ...) functions for the Common Locale Data Repository (CLDR)
Other
16 stars 13 forks source link

Dialyzer warning on `Cldr.Unit.display_name/2` with atom as first argument #43

Closed ryotsu closed 3 months ago

ryotsu commented 3 months ago

The documentation for the function Cldr.Unit.display_name/2 says that the first argument can be any unit name returned by Cldr.Unit.known_units/0, which returns a list of atoms. But the spec for Cldr.Unit.display_name/2 only specifies Cldr.Unit.value() | Cldr.Unit.t() as the first argument which causes a dialyzer warning when using an atom as the first argument.

The function call will not succeed.

MyRepo.Cldr.Unit.display_name(_unit :: atom(), [{:style, :narrow}])

will never return since the success typing is:
(
  number()
  | %{
      :__struct__ => Cldr.Unit | Decimal,
      :backend => _,
      :base_conversion => [{atom() | [any()], map()}] | {[{_, _}], [{_, _}]},
      :coef => _,
      :exp => _,
      :format_options => Keyword.t(),
      :sign => _,
      :unit => atom() | binary() | [atom()],
      :usage => atom(),
      :value => number() | %Decimal{:coef => _, :exp => _, :sign => _}
    },
  Keyword.t()
) :: binary() | {:error, {atom(), binary()}}

and the contract is
(Cldr.Unit.value() | Cldr.Unit.t(), Keyword.t()) ::
  String.t() | {:error, {module(), binary()}}
kipcole9 commented 3 months ago

I've published ex_cldr_units version 3.16.5 with the following changelog entry:

Bug Fixes

Thanks much for the report.