dialyzer issues with elixir 1.12.0

[bitboxer]

I just tried to update a project to elixir 1.12.0 and dialyzer spits out these messages:

deps/postgrex/lib/postgrex/type_module.ex:897:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is nonempty_improper_list(binary(), bitstring()).
________________________________________________________________________________
deps/postgrex/lib/postgrex/type_module.ex:897:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is bitstring().
________________________________________________________________________________
deps/postgrex/lib/postgrex/type_module.ex:897:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is binary().

Has anyone here seen those?

[J3RN]

I get this error as well. lib/postgrex/type_module.ex seems to be a common cause of Dialyzer errors; see also #543.

[josevalim]

I don’t run Dialyzer as part of my workflow, so I would appreciate if someone could investigate this and send PRs.

FWIW, the reason why we have Dialyzer warnings on TypeModule is because it does code generation, so it likely generates clauses that can never match for some types but it matches for others, and Dialyzer can see through those. Sometimes the proper fix is to add the generated: true to the quoted code.

[bitboxer]

For everyone here: the hot fix is to add a file called .dialyzer_ignore.exs and add this into it:

[
   {"deps/postgrex/lib/postgrex/type_module.ex"},
   {"lib/postgrex/type_module.ex"}
]

[thbar]

For some reason the previous hot fix did not work for me, I had to use a slightly different version. I wonder if this may be because the app I'm referring to is an umbrella app.

Current fix:

[
   # temporary fix for https://github.com/elixir-ecto/postgrex/issues/549
   ~r/deps\/postgrex\/lib\/postgrex\/type_module.ex/,
   ~r/lib\/postgrex\/type_module.ex/
 ]

[al2o3cr]

The warning is accurate, in the sense that it says "an improper list is being constructed here" and one is - the only report I got with a sensible line number pointed to line 358:

      defp decode_rows(<<?D, size::int32, rest::binary>>, rem, _, rows) do
        more = size + 1 - rem
        {:more, [?D, <<size::int32>> | rest], rows, more}   # <= MESSAGE POINTS HERE
      end

Tracing where this {:more, iolist(), _, _} tuple goes, seems like it's solely handled in Postgrex.Protocol.rows_recv - which ultimately conses a few more things on and then passes the lot to IO.iodata_to_binary/1.

That explains why the code works, since that function is totally fine with cons-ballon-animals.

I'm still confused about why Dialyzer only complains about the clause on line 358, when there's another equally-improper-seeming list on line 362 🤔

[josevalim]

This is all expected and improper lists are valid io lists, so it is just a matter of adding a @dialyzer annotation that says improper lists are expected?

[al2o3cr]

@josevalim the annotation is (IMO) a tool of last resort; I'm more curious about why code that has been in Postgrex for 5 years (the timestamp on the lines in decode_rows) has suddenly started alerting. Planning to poke around in the OTP changelog to see if some analysis switched from opt-in to opt-out or something.

Related to those lines, a question - in all three of these cases, the iolist that's constructed converts back to an identical binary under iodata_to_binary. What's the motivation for switching formats? (performance maybe?)

      defp decode_rows(<<?D, size::int32, rest::binary>>, rem, _, rows) do
        more = size + 1 - rem
        {:more, [?D, <<size::int32>> | rest], rows, more}
      end

      defp decode_rows(<<?D, rest::binary>>, _, _, rows) do
        {:more, [?D | rest], rows, 0}
      end

      defp decode_rows(<<rest::binary-size(0)>>, _, _, rows) do
        {:more, [], rows, 0}
      end

[al2o3cr]

I've created a demo repo that shows the issue - it appears to be triggered by using Postgrex.Types.define, even with an empty list of extensions.

https://github.com/al2o3cr/pg_dialyzer_test

Trying different versions with asdf shows that this behavior changes between Elixir 1.11 and Elixir 1.12.

[al2o3cr]

Bisecting through elixir-lang/elixir turns up this commit as the first one that makes mix dialyzer fail on that project:

elixir-lang/elixir@51de0e0

Could this be an "add enough metadata to make the error reportable, then the error appears" situation?

[josevalim]

Could this be an "add enough metadata to make the error reportable, then the error appears" situation?

Yup, that seems to be the case!

[al2o3cr]

@josevalim Confirmed with additional manual prodding - short-circuiting one line from that commit to instead always return 0 causes all the messages to go away:

translate_list([{'|', _, [_, _]=Args}], Acc, List, Ann) ->
  {[TLeft, TRight], TAcc} = lists:mapfoldl(fun translate/2, Acc, Args),
  % version from the commit, gives Dialyzer errors
  TAnn = if Ann == 0 -> binary_ann(TLeft, TRight); true -> Ann end,
  % alternative version that does NOT give Dialyzer errors
  % TAnn = 0,
  {build_list([TLeft | List], TRight, TAnn), TAcc};

I was able to silence some of the low-hanging fruit (that has sensible line numbers) with a judiciously-placed generated: true, will see if I can find where else that annotation is needed...

[al2o3cr]

Discovered debug_defaults: true and have become enlightened:

/Users/mattjones/elixir-ecto/postgrex/lib/postgrex/type_module.ex:897:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is Postgrex.Types.state().
________________________________________________________________________________
lib/postgrex/extensions/bit_string.ex:11:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is binary().
________________________________________________________________________________
lib/postgrex/extensions/bit_string.ex:21:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is nonempty_improper_list(binary(), bitstring()).
________________________________________________________________________________
lib/postgrex/extensions/bit_string.ex:21:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is bitstring().
________________________________________________________________________________
lib/postgrex/extensions/box.ex:13:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is nonempty_improper_list(<<_::128>>, <<_::128>>).
________________________________________________________________________________
lib/postgrex/extensions/box.ex:13:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is <<_::128>>.
________________________________________________________________________________
lib/postgrex/extensions/line_segment.ex:13:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is nonempty_improper_list(<<_::128>>, <<_::128>>).
________________________________________________________________________________
lib/postgrex/extensions/line_segment.ex:13:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is <<_::128>>.
________________________________________________________________________________
lib/postgrex/extensions/name.ex:11:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is binary().
________________________________________________________________________________
lib/postgrex/extensions/numeric.ex:10:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is <<_::64>> | nonempty_improper_list(<<_::64>>, binary()).
________________________________________________________________________________
lib/postgrex/extensions/numeric.ex:14:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is <<_::64>> | nonempty_improper_list(<<_::64>>, binary()).
________________________________________________________________________________
lib/postgrex/extensions/numeric.ex:18:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is <<_::64>> | nonempty_improper_list(<<_::64>>, binary()).
________________________________________________________________________________
lib/postgrex/extensions/path.ex:15:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is <<_::128>>.
________________________________________________________________________________
lib/postgrex/extensions/polygon.ex:15:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is <<_::128>>.
________________________________________________________________________________
lib/postgrex/extensions/raw.ex:20:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is binary().
________________________________________________________________________________
lib/postgrex/extensions/uuid.ex:11:improper_list_constr
List construction (cons) will produce an improper list, because its second argument is binary().
________________________________________________________________________________
done (warnings were emitted)
Halting VM with exit status 2

[al2o3cr]

Created #556 to resolve this.