elixir-cldr / cldr_units

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

Wrong Dialyzer Typing #15

Closed maennchen closed 3 years ago

maennchen commented 3 years ago

Working Code

@task.co2_savings_kg |> Decimal.to_float |> Cldr.Unit.to_string!(unit: :kilogram)

Dialyzer Error

lib/acme/templates/challenge/task/show.html.eex:32:call
The function call will not succeed.

Acme.Cldr.Unit.to_string!(float(), [{:unit, :kilogram}])

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

and the contract is
(Cldr.Unit.t() | [Cldr.Unit.t(), ...], Keyword.t()) :: String.t() | no_return()
kipcole9 commented 3 years ago

Hmm, surprising and disappointing that I still have a dialyzer error since I check it on each release.

I won’t be able to do anything about it for 48 hour I’m afraid but will fix it right away after that.

As always, thanks for the report.

Sent from my iPhone

On 12 Aug 2020, at 17:23, Jonatan Männchen notifications@github.com wrote:

 Working Code

@task.co2_savings_kg |> Decimal.to_float |> Cldr.Unit.to_string!(unit: :kilogram) Dialyzer Error

lib/acme/templates/challenge/task/show.html.eex:32:call The function call will not succeed.

Acme.Cldr.Unit.to_string!(float(), [{:unit, :kilogram}])

will never return since the success typing is: ( %Cldr.Unit{ :base_conversion => [any()] | {[map(), ...], [map(), ...]} | %{ :base_unit => [atom(), ...], :factor => number() | %Ratio{:denominator => pos_integer(), :numerator => integer()}, :offset => number() }, :formatoptions => [], :unit => atom() | binary(), :usage => atom(), :value => number() | %{ :struct => Decimal | Ratio, :coef => , :denominator => posinteger(), :exp => , :numerator => integer(), :sign => _ } }, Keyword.t() ) :: binary()

and the contract is (Cldr.Unit.t() | [Cldr.Unit.t(), ...], Keyword.t()) :: String.t() | no_return() — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

maennchen commented 3 years ago

I think there's different ways to call it, maybe it was just never called this way...?

I'm not in a hurry with that problem, so take your time :)

kipcole9 commented 3 years ago

It’s now a matter of pride. I want one release that passes the Jonathan männchen dialyzer test 😀

Sent from my iPhone

On 12 Aug 2020, at 18:58, Jonatan Männchen notifications@github.com wrote:

 I think there's different ways to call it, maybe it was just never called this way...?

I'm not in a hurry with that problem, so take your time :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

maennchen commented 3 years ago

It's not only cldr, Phoenix Router also always generates warnings :P

kipcole9 commented 3 years ago

Fixed (I believe - I added a call of this type to a function that is checked by dialyzer in development). Also fixed the bug whereby Decimal types weren't supported on to_string/3. Therefore you should now be able to:

# Where @task.co2_savings_kg is a Decimal
Cldr.Unit.to_string!(@task.co2_savings_kg, unit: :kilogram)

Published to hex as Cldr Units version 3.1.2. Standing by to any updates if the warning persists......