Function call without opaqueness type mismatch in OTP 28

[woylie]

After upgrading from OTP 27 to 28, I'm getting this dialyzer warning:

lib/my_app_web_web/gettext.ex:1:call_without_opaque
Function call without opaqueness type mismatch.

Call does not have expected term of type
  binary()
  | {_,
     integer()
     | %Expo.PluralForms{
         :nplurals => pos_integer(),
         :plural => Expo.PluralForms.plural_ast()
       }}
 (with opaque subterms) in the 1st position.

Gettext.Plural.plural({<<106, 97>>, %Expo.PluralForms{:nplurals => 1, :plural => 0}}, _ :: any())

Content of the MyAppWeb.Gettext:

defmodule MyAppWeb.Gettext do
  use Gettext.Backend, otp_app: :my_app
end

config.exs:

config :gettext, :default_locale, "ja"

I found related issues in the Elixir repo:

• Dialyzer errors after upgrading to 1.19.0 elixir-lang/elixir#14837
• Opaqueness warning on OTP28 elixir-lang/elixir#14750

This occurs in Gettext 1.0.2, Expo 1.1.1, Elixir 1.19.5, OTP 28.3.

[josevalim]

If it is caused by MapSet, which I think it is, because we don't use @opaque anywhere in gettext/expo, then it should be fixed on Elixir v1.20-rc.1. Did you try it?

[woylie]

I just tried it with 1.20.0-rc.1 on OTP 28.3. I'm still getting the same dialyzer warnings, including ones that I think also should be fixed already:

Call does not have expected term of type %URI{
  :authority => URI.authority(),
  :fragment => nil | binary(),
  :host => nil | binary(),
  :path => nil | binary(),
  :port => nil | char(),
  :query => nil | binary(),
  :scheme => nil | binary(),
  :userinfo => nil | binary()
} (with opaque subterms) in the 1st position.

URI.to_string(%URI{
  :authority => nil,
  :fragment => nil,
  :host => nil,
  :path => nil | binary(),
  :port => nil,
  :query => nil | binary(),
  :scheme => nil,
  :userinfo => nil
})

Call does not have expected term of type %Ecto.Multi{
  :names => %MapSet{:map => :sets.set(_)},
  :operations => [{_, {_, _} | {_, _, _} | {_, _, _, _}}]
} (with opaque subterms) in the 1st position.

Ecto.Multi.update(
  %Ecto.Multi{:names => %MapSet{:map => %{}}, :operations => []},
  :application,
  _changeset :: %Ecto.Changeset{
    :action => atom(),
    :changes => %{atom() => _},
    :constraints => [
      %{
        :constraint =>
          binary() | %Regex{:opts => [any()], :re_pattern => _, :source => binary()},
        :error_message => binary(),
        :error_type => atom(),
        :field => atom(),
        :match => :exact | :prefix | :suffix,
        :type => :check | :exclusion | :foreign_key | :unique
      }
    ],
    :data => nil | map(),
    :empty_values => _,
    :errors => Keyword.t({binary(), Keyword.t()}),
    :filters => %{atom() => _},
    :params => nil | %{binary() => _},
    :prepare => [(_ -> any())],
    :repo => atom(),
    :repo_opts => Keyword.t(),
    :required => [atom()],
    :types => %{
      atom() =>
        atom()
        | {:array | :assoc | :embed | :in | :map | :parameterized | :supertype | :try, _}
    },
    :valid? => boolean(),
    :validations => Keyword.t()
  }
)

[josevalim]

The authority still hapens, has to be fixed in Elixir. What about Ecto? I think we remove all opaques in more recent versions...

[woylie]

This was the output with ecto 3.13.5 and ecto_sql 3.13.4.

The type definition of MapSet.t/1 uses :sets.set/1:

https://github.com/elixir-lang/elixir/blob/3bb5f92fa91a7cd7cddb67618f5f6ac3f8015d2e/lib/elixir/lib/map_set.ex#L60

:sets.set/1 itself is opaque:

https://github.com/erlang/otp/blob/ab489c0b260c637a9487f8753909ea94dacbbec2/lib/stdlib/src/sets.erl#L164

Is that causing the issue?

[sabiwara]

The authority still hapens, has to be fixed in Elixir.

Oh I didn't notice this discussion until now, just found the same issue and submitted elixir-lang/elixir#15062.

@woylie MapSet has been fixed in elixir-lang/elixir@7c13e99.

[woylie]

@woylie MapSet has been fixed in elixir-lang/elixir@7c13e99.

@sabiwara That fix was released with 1.20.0-rc.1, but I'm still getting the warnings above with that version.

[sabiwara]

Oh I got it now, sorry for the misunderstanding @woylie.
Maybe we could just define it as %{optional(t) => []} rather than using :sets? (it's unfortunate but opaque is becoming really hard to use without a way to ignore it for generated code).

[woylie]

@sabiwara If there's no way to use :sets, I guess the type needs to be represented without it. Since Elixir always uses the map-based implementation, %{optional(t) => []} would be more precise anyway. Maybe %{optional(t) => term} would be better? The :sets type does not define a type for the map value either.

[maennchen]

Here it's probably not any MapSet, but rather something from expo. It has a module Expo.PluralForm that contains an ast or key and meta on messages.

I used opaque there to basically set an internal boundary / backwards compatibility. I would not issue a breaking change in case something inside those changes and it's not a functionality intended to be used from the outside.

Combined with the gettext backend macros for performance reasons, that seems to be a suboptimal combination.

@josevalim Elixir already dealt with this with URI, MapSet etc. How do you intend to establish those boundaries going forward?

[josevalim]

We are marking those as private instead of opaque.

[maennchen]

@woylie Do the errors go away if you use this PR? elixir-gettext/expo#157

[sabiwara]

How do you intend to establish those boundaries going forward?
We are marking those as private instead of opaque.

@josevalim do you mean it is just documented, but not enforceable by a tool anymore?
It would be great if we have a way to declare opaque fields when we add typed structs. Even if the type system doesn't have a way to check it at first, the information would be available for IDEs/docs... and the checking could be added later.

(sorry for the pure Elixir discussion on the gettext issue 😅)

[woylie]

@woylie Do the errors go away if you use this PR? elixir-gettext/expo#157

@maennchen Yes, that fixes it, thank you very much!

[maennchen]

@woylie Does this also fix your issue?

diff --git a/lib/gettext/compiler.ex b/lib/gettext/compiler.ex
index 133f1dd..9d3d848 100644
--- a/lib/gettext/compiler.ex
+++ b/lib/gettext/compiler.ex
@@ -567,7 +567,9 @@ defmodule Gettext.Compiler do
     quote do
       defp unquote(plural_fun)(n) do
         unquote(plural_mod).plural(
-          unquote(Macro.escape(Plural.plural_info(locale, messages_struct, plural_mod))),
+          unquote(
+            Macro.escape(Plural.plural_info(locale, messages_struct, plural_mod), generated: true)
+          ),
           n
         )
       end
diff --git a/lib/gettext/extractor.ex b/lib/gettext/extractor.ex
index c19b6a7..a6ce1a0 100644
--- a/lib/gettext/extractor.ex
+++ b/lib/gettext/extractor.ex
@@ -192,7 +192,7 @@ defmodule Gettext.Extractor do
          extracted_comments,
          format_flag
        ) do
-    %Message.Plural{
+    struct!(Message.Plural,
       msgid: [msgid],
       msgctxt: if(msgctxt != nil, do: [msgctxt], else: nil),
       msgid_plural: [msgid_plural],
@@ -200,18 +200,18 @@ defmodule Gettext.Extractor do
       flags: [[@extracted_messages_flag, format_flag]],
       references: [[{Path.relative_to_cwd(file), line}]],
       extracted_comments: extracted_comments
-    }
+    )
   end
 
   defp create_message_struct(msgid, msgctxt, file, line, extracted_comments, format_flag) do
-    %Message.Singular{
+    struct!(Message.Singular,
       msgid: [msgid],
       msgctxt: if(msgctxt != nil, do: [msgctxt], else: nil),
       msgstr: [""],
       flags: [[@extracted_messages_flag, format_flag]],
       references: [[{Path.relative_to_cwd(file), line}]],
       extracted_comments: extracted_comments
-    }
+    )
   end
 
   # Made public for testing.