sasa1977/fsm

Using fsm cause dependent dialyzer run fails

OleMchls opened this issue · 4 comments

👋 ✨ ❤️

I'm currently working on fixing the typespecs for https://github.com/commanded/eventstore running in a fork https://github.com/dnsimple/eventstore in the fix-typespecs branch.

lib/event_store/subscriptions/subscription_fsm.ex:7: The pattern <_@1 = #{'__struct__':='Elixir.EventStore.Subscriptions.SubscriptionFsm'}, _> can never match since previous clauses completely covered the type <#{'__struct__':='Elixir.EventStore.Subscriptions.SubscriptionFsm', 'data':=_, 'state':=_, _=>_},{'action_responses',[{_,_},...]}>

The code that dialyzer is referencing is

defmodule EventStore.Subscriptions.SubscriptionFsm do
  @moduledoc false

  alias EventStore.{AdvisoryLocks, RecordedEvent, Storage}
  alias EventStore.Subscriptions.{SubscriptionState, Subscription}

  use Fsm, initial_state: :initial, initial_data: %SubscriptionState{}

The macro use line is line 7.

You could check out the mix dialyzer run yourself:

git clone https://github.com/dnsimple/eventstore.git
cd eventstore && mix deps.get && mix deps.compile
git checkout fix-typespecs
mix dialyzer
$~ elixir --version
Erlang/OTP 20 [erts-9.3.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:10] [hipe] [kernel-poll:false] [dtrace]

Elixir 1.6.5 (compiled with OTP 20)

Thanks a lot for reading through this 👍


Update 06/13/2018: added elixir version

I "solved" the problem by including a dialyzer suppression attribute in the generated code. I opted for this approach, since proper solution would cause a breaking change, and I didn't want to do that. Could you please try with the latest GH master, and let me know if it works? Once you're happy, I can bump the version and publish to hex.

Also note that I'm not actively maintaining this library anymore. The reason is that I don't believe it's actually useful, and that you can get the same benefits using plain pattern matching on maps/structs. That might require a bit more code, but it leads to a less magical and a more explicit code.

In fact, this issue is a simple example of where it would be easier to understand the problem if Fsm wasn't used. Consider the following code:

defmodule Test do
  def test(), do: foo(bar())

  defp foo(:bar), do: :foobar
  defp foo(_other), do: :foo

  defp bar(), do: :bar
end

If you dialyze it, you'll get:

lib/test.ex:5: The variable _other@1 can never match since previous clauses completely covered the type 'bar'

Which points you to the proper place in the code, and I believe the message is informative enough, although it could be reworded in a slightly better way (e.g. "This clause will never match since previous clauses completely covered all possible cases").

Thanks a bunch for the quick look and the explanations @sasa1977 ❤️

I just tested with master and the error is gone 👍 I'd suggest to add a big indicator in the readme and maybe even archive the GitHub repo to indicate it's not actively maintained anymore. That said, I want to emphasise again how thankful I am for you looking at this <3

I pushed 0.3.1 to hex.

thank you soooo much @sasa1977 ❤️