Is Dialyzer fetching incorrect specs, or are they incorrect in Meeseeks?

Questions / Help
, ,
1 
defmodule A do
  def x() do
    Meeseeks.parse("<div id=main><p>Hello, Meeseeks!</p></div>")
  end
end

This gives me this error:

The call:
Meeseeks.parse(<<_::336>>)

will never return since it differs in arguments with
positions 1 from the success typing arguments:

(
  [
    binary()
    | {:comment, binary()}
    | {:pi, binary()}
    | {:pi | binary(), binary() | [{_, _}], binary() | [any()]}
    | {:doctype, binary(), binary(), binary()}
  ]
  | {:comment, binary()}
  | {:pi, binary()}
  | {:pi | binary(), binary() | [{binary(), binary()}], binary() | [any()]}
  | {:doctype, binary(), binary(), binary()}
)

When the specs for Meeseeks.parse is @spec parse(Parser.source()) :: Document.t() | {:error, Error.t()} with Parser.source() being @type source :: String.t() | TupleTree.t() and then TupleTree.t() being

  @type comment :: {:comment, String.t()}
  @type doctype :: {:doctype, String.t(), String.t(), String.t()}
  @type element :: {String.t(), [{String.t(), String.t()}], [node_t]}
  @type processing_instruction ::
          {:pi, String.t()}
          | {:pi, String.t(), [{String.t(), String.t()}]}
          | {:pi, String.t(), String.t()}
  @type text :: String.t()
  @type node_t :: comment | doctype | element | processing_instruction | text
  @type t :: node_t | [node_t]

Dialyxir and Dialyzer helped ma lot, but I don’t understand what’s wrong with those specs, or is it bug with tools rather.

If source :: String.t() | TupleTree.t() or source :: binary() | TupleTree.t() then call Meeseeks.parse(<<_::336>>) where <<_::336>> is binary should satisfy those specs. Right?

Beside, even when dialyxir/dialyzer says [...] will never return since [...] actually when run always returns and simply works OK.

2 Likes
2

It looks like Dialyzer is finding the success type of the arg as list(Parser.source()) | Parser.source(), rather than the defined spec which is just Parser.source(). You could try fixing the spec in your local deps to and mix deps.compile meeseeks to try that out.

3

I really am not sure how to fix them, any pointers? :slight_smile:

4

What I’m suggesting is just change above spec to what appears to be the success typing:

@spec parse(list(Parser.source()) | Parser.source()) :: Document.t() | {:error, Error.t()}
1 Like
5

Actually that is a problem but probably not YOUR problem. Your problem appears to be that binary is not accepted unless its in a list, so you’d change your call to Meeseeks.parse(["<div id=main><p>Hello, Meeseeks!</p></div>"]). I’m not sure why it would work and not generate a compatible success typing, its an issue in that library though.

6

Changing it into list does not work sadly :slight_smile:

I am not sure why dialyxir thinks that success typings is binary in a list etc `[binary() | … ] that is what I mainly try to understand.

7

Its due to a discontinuity in that spec versus the implementation of that function. What happens if you just remove the @spec from parse ?

8

I think dialyzer does not recompile its spec internally after commenting specs in meeseks, because errors are the same. I’ll try with fresh environment and simple project later this week, but can’t do it now, too much work :wink:

9

Did you run mix deps.compile meeseeks ? Dependencies are not recompiled automatically.

10

I did mix deps.compile after changing and it did recompiled meeseeks.

11

I’m pretty sure that list in the success typing there is because of @type t :: node_t | [node_t] in the TupleTree.t(), which should list every node type as contents of a list, but also every node type singularly.

However, it’s very interesting that
a) while the list of node_t properly contains binaries (representing text nodes), binary() isn’t included as a single node outside of the list as it properly should
b) a top level binary() isn’t provided since it should be included because of String.t() | TupleTree.().

Edit: This does highlight a logical problem when it comes to parsing, but I don’t think it should affect Dialyzer.

12

I code dived a bit. And on a first glance everything looks like passing in a string should work (and actually does).

But, this string gets passed (deep in the stack of function calls) to MeeseeksHtml5ever.Native.parse_html/1. As you can guess from its name, its a NIF.

NIFs always have two implemantations. A native one (eg.: C or Rust) and a “naive” one (BEAM-compatible). Often the naive one is simply throwing.

This throw does happen as well for this function (check https://github.com/mischov/meeseeks_html5ever/blob/master/lib/meeseeks_html5ever/native.ex).

This is the only implementation dialyzer sees. So from dialyzers view, passing a string to Meeseex.parse/1 will always resuilt in a throw, so dialyzer removes String.t silently from the spec.

This has to be fixed upstream (meeseex_html5ever).

4 Likes
13

Ah! That’s great to know!

But how does one fix that in meeseeks_html5ever?

14

By changing the implementation, something like this:

def parse_html(_), do: {:error, :nif_not_loaded}

Also changing the involved specs and types to actually allow this return value.

15

Hmm. Would using :erlang.nif_error in the naive function where I previously raised be the standard solution? From the description of nif_error:

Works exactly like error/1, but Dialyzer thinks that this BIF will return an arbitrary term. When used in a stub function for a NIF to generate an exception when the NIF library is not loaded, Dialyzer does not generate false warnings.

1 Like
16

Should work as well, at least from the documentation you cited.

I never used it though.

17

I’ll be more than happy to beta test, if you just tell me exactly what to change :smiley:

18

Using :erlang.nif_error appears to resolve the error.

Thanks for bringing it to my attention- I’ll see if I can’t get releases out with the fix before too long.

4 Likes
19

@mischov Awesome, however Meeseeks does not accept 0.9.0 of your html5ever lib, only ~>0.8.1. :slight_smile:

20

Well Meeseeks 0.9.3 was available on Github already, I just hadn’t gotten it up on Hex yet. It’s up now.

1 Like