'Explain' instructions for pattern_match warning are confusing

[kenny-evitt]

Precheck

I didn't find any existing issues (open or closed) that cover this.

I think my Dialyzer and Erlex are up-to-date but I could be wrong – please let me know.

Environment

$ elixir --version
Erlang/OTP 21 [erts-10.2.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [hipe]

Elixir 1.8.0 (compiled with Erlang/OTP 20)
$ cat mix.lock | grep dialyxir
  "dialyxir": {:hex, :dialyxir, "1.0.0-rc.7", "6287f8f2cb45df8584317a4be1075b8c9b8a69de8eeb82b4d9e6c761cf2664cd", [:mix], [{:erlex, ">= 0.2.5", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm"},

Current behavior

I'm getting this warning with the default (Dialyxir) format:

lib/partially/translator.ex:112:pattern_match
The pattern can never match the type.

Pattern:
{:ok, _po}

Type:
{:error, pos_integer(), binary()}

Line 112 is the second line in the following:

          case Gettext.PO.parse_string(po_string) do
            {:ok, po} ->
              # add each translation
              Enum.each(po.translations, &add_translation(&1, table, locale, d))
            err ->
              Logger.error "Error processing localise po file for #{locale} #{inspect err}"
              nil
          end

Here's the docs for the Gettext.PO.parse_string/1 function:

> Gettext.PO.parse_string(str)

### Specs

`@spec parse_string(binary) :: {:ok, t} | parse_error`

Parses a string into a `Gettext.PO` struct.

This function parses a given `str` into a `Gettext.PO` struct.
It returns `{:ok, po}` if there are no errors,
otherwise `{:error, line, reason}`.

## Examples

    iex> {:ok, po} = Gettext.PO.parse_string """
    ...> msgid "foo"
    ...> msgstr "bar"
    ...> """
    iex> [t] = po.translations
    iex> t.msgid
    ["foo"]
    iex> t.msgstr
    ["bar"]
    iex> po.headers
    []

    iex> Gettext.PO.parse_string "foo"
    {:error, 1, "unknown keyword 'foo'"}

I totally missed the pattern_match at the end of the first line of the warning output.

But I still had trouble figuring out how to call the 'explain' command on a warning. From the README:

Explanations are available for classes of warnings by passing the --explain warning_name flag.

I guess that's because the --explain flag has been replaced by a separate task, e.g. mix dialyzer.explain warning_name.

The explanation for my warning seems unclear to me:

$ mix dialyzer.explain pattern_match
The pattern matching is never given a value that satisfies all of
its clauses.

Example:

defmodule Example do

  def ok() do
    unmatched(:ok)
  end

  defp unmatched(:ok), do: :ok

  defp unmatched(:error), do: :error
end

Is my warning ultimately that my code doesn't pattern match all possibilities in the case? Or is it that po_string might not be a binary value?

For the former, adding an explicit _ -> ... clause didn't help.

For the latter, wrapping the code code with an explicit check, e.g. if is_binary(po_string) ... doesn't resolve the warning.

Or is the warning that Dialyzer expects the call Gettext.PO.parse_string(po_string) to always return the error condition?

I followed the code that produces the po_string value and discovered that the code that produces it is (effectively) specced to return a value with a typespec of term(), i.e. any type. But then why doesn't adding an explicit check, e.g. guard clause, resolve it?

Expected behavior

I'd 'expect' (ideally) to be able to understand the warning, its cause, and how I could resolve it.

[kenny-evitt]

The warning seems to be due to a known issue with Gettext:

• Dialyzer errors on Gettext.PO.parse_string · Issue #205 · elixir-gettext/gettext

Maybe the warning could more clearly indicate that the particular pattern match clause will never be matched (according to Dialyzer)?

[jeremyjh]

@kenny-evitt The warning says "The pattern can never match the type."
Do you have a suggested alternate wording?

[kenny-evitt]

@jeremyjh Thanks for Dialyxir by-the-way!

It just occurred to me that maybe what would make this warning clearer is another warning – that the type that the "pattern can never match" doesn't match its spec.

The resolution of this warning turned out to be fixing the spec for another function called by Gettext.PO.parse_string/1. The spec had just not been updated when the function had been so Dialyzer reasoned that calling parse_string would always fail, hence Dialyzer warning me that my pattern could never match what would always be an {:error, ...} return value type.

Or is this kind of thing, e.g. resolving warnings in dependencies, just routine and expected when using Dialyzer?

Is there a way to see warnings for (all of my) dependencies too?

Maybe the warning could say "The pattern can never be matched to the type.". But then it'd also be nice to also mention that "the type", if it's the return value of a function, doesn't match the spec of the function.

[jeremyjh]

It’s pretty routine and expected. The dependency might not even have a warning in its own code; the warning might only surface on the caller. If there is any path that could possibly match the spec in the dependency it won’t raise an error; that is the principle of success typing. Dialyzer will warn if it can determine specs are flat out wrong, but that code has to be under analysis to do so. There are instructions in the readme for adding code paths to dialyxir - you could use that to analyze dependencies but it takes too long to be practical to just run it in all dependencies. I’m ok with adjusting the wording a bit as you suggest.

…

On Fri, Nov 15, 2019 at 7:11 PM kenny-evitt ***@***.***> wrote: @jeremyjh <https://github.com/jeremyjh> Thanks for Dialyxir by-the-way! It just occurred to me that maybe what would make this warning clearer is another warning – that the type that the "pattern can never match" doesn't match its spec. The resolution of this warning turned out to be fixing the spec for another function called by Gettext.PO.parse_string/1. The spec had just not been updated when the function had been so Dialyzer reasoned that calling parse_string would *always* fail, hence Dialyzer warning me that my pattern could never match what would always be an {:error, ...} return value type. Or is this kind of thing, e.g. resolving warnings in dependencies, just routine and expected when using Dialyzer? Is there a way to see warnings for (all of my) dependencies too? Maybe the warning could say "The pattern can never *be matched* to the type.". But then it'd also be nice to also mention that "the type", if it's the return value of a function, doesn't match the spec of the function. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#375?email_source=notifications&email_token=AAAWDDUSJH2LLGG2QDJQTBLQT43D5A5CNFSM4JNU26E2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHDEOA#issuecomment-554578488>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAWDDUWKEC7LVWLUIU5JEDQT43D5ANCNFSM4JNU26EQ> .

[kenny-evitt]

It’s pretty routine and expected.

Cool – that's good to know! Maybe this sentence from the "With Explaining Stuff" section in the README could be emphasized:

For more information I highly recommend the Success Typings paper that describes the theory behind the tool.

@jeremyjh Would you mind if I submitted a PR with some changes to the README? One thing I think might be clearer would be to rename the section ""With Explaining Stuff" to "Understanding Dialyzer Warnings".

The dependency might not even have a warning in its own code; the warning might only surface on the caller.

I actually expected that and feel like my example warning is a good example of why that's hard to understand given the next thing you mention:

If there is any path that could possibly match the spec in the dependency it won’t raise an error; that is the principle of success typing.

This seems really key to understanding warnings like mine. I'll try to read the success typings paper soon to better understand how Dialyzer works.

Dialyzer will warn if it can determine specs are flat out wrong, but that code has to be under analysis to do so. There are instructions in the readme for adding code paths to dialyxir - you could use that to analyze dependencies but it takes too long to be practical to just run it in all dependencies.

I figured that's what the PLT files did – cache the Dialyzer analysis, which would include warnings. I'm not specifying a PLT dependencies option so I figured I'd see warnings for all of my app's dependencies too.

Some ideas on making this warning clearer:

1. Noting that the PLT contains a different 'analyzed' spec for a dependency function than the function's stated spec.
2. If [1] is not feasible, just mention that the general scenario for [1] is possible.

[jeremyjh]

I'm open to a PR with changes along the lines you suggest to the README. I'm less enthusiastic about adding specific caveats about the analysis method to warnings or even explanations. A link to a wiki or similar resource might make sense, if such a thing existed.

[kenny-evitt]

@jeremyjh Great – I'll work on a PR soonish.

For the "specific caveats", what about adding a page to the docs to which the warnings or explanations could link? (I personally prefer docs stored in the repo over something outside the repo like a wiki.)

[jeremyjh]

What you are proposing to document is not a feature of this project though. It doesn't belong to dialyxir, its facts about the Erlang dialyzer.

[kenny-evitt]

@jeremyjh Yes, that's true. Where's the best place, or a good one, to document this kind of thing? Stack Overflow? A blog post? I'm guessing that you'd prefer not to document this kind of thing in GitHub issues on this project. Would the wiki for this project be okay? Maybe in a page for this specific warning? I could also document this in the issues or wiki of my fork of this project on GitHub, or add to the warnings output in my fork for my own use.

Sorry for any aggravation about this!

[kenny-evitt]

@jeremyjh I just saw this older issue:

• Can Dialyxir help to clarify the success typing rationale? · Issue #327 · jeremyjh/dialyxir

Based on what you commented on that issue, I understand better your intended scope of Dialyxer:

dialyxir is an integration tool, not an analysis tool; the analysis tool is dialyzer.

[jeremyjh]

@kenny-evitt Yes exactly we don't control the analysis engine, just the integration and messaging, but we can still document some "tips" if there is no better place to start. The wiki in this repo is open, you can add an article to it and then open a PR to link that article in the explain.