-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dialyzer fixes #324
Dialyzer fixes #324
Conversation
lib/cachex.ex:331:guard_fail The guard clause: when _ :: true === nil can never succeed.
lib/cachex/services/janitor.ex:136:guard_fail The guard clause: when _ :: reference() === nil can never succeed.
lib/cachex/policy/lrw/evented.ex:67:guard_fail The guard clause: when _ :: :ok === nil can never succeed.
lib/cachex.ex:341:callback_spec_type_mismatch The @SPEC return type does not match the expected return type for init/1 callback in Supervisor behaviour. Actual: @SPEC init(...) :: {:error, _} | {:ok, pid()} Expected: @SPEC init(...) :: :ignore | {:ok, {%{ :intensity => non_neg_integer(), :period => pos_integer(), :strategy => :one_for_all | :one_for_one | :rest_for_one }, [ {_, {atom(), atom(), :undefined | [any()]}, :permanent | :temporary | :transient, :brutal_kill | timeout(), :supervisor | :worker, :dynamic | [atom()]} | %{ :id => _, :start => {atom(), atom(), :undefined | [any()]}, :modules => :dynamic | [atom()], :restart => :permanent | :temporary | :transient, :shutdown => :brutal_kill | timeout(), :significant => boolean(), :type => :supervisor | :worker } ]}}
lib/cachex/actions/stats.ex:26:pattern_match The pattern can never match the type. Pattern: {:ok, _stats} Type: %{}
lib/cachex/services.ex:28:invalid_contract The @SPEC for the function does not match the success typing of the function. Function: Cachex.Services.app_spec/0 Success typing: @SPEC app_spec() :: [ %{ :id => Cachex.Services.Locksmith | Cachex.Services.Overseer, :start => {Cachex.Services.Locksmith, :start_link, []} | {Cachex.Services.Overseer, :start_link, []}, :type => :supervisor }, ... ]
lib/cachex/actions/incr.ex:30:no_return Function execute/4 has no local return. lib/cachex/actions/incr.ex:32:call The function call will not succeed. Cachex.Services.Janitor.expiration(_cache :: {:cache, _, _, _, _, _, _, _, _, _, _}, nil) breaks the contract (Cachex.Spec.cache(), integer()) :: integer()
lib/cachex/actions/put.ex:34:no_return The created anonymous function has no local return. lib/cachex/actions/put.ex:35:call The function call will not succeed. Cachex.Actions.write( _cache :: {:cache, atom(), map(), boolean(), {:expiration, non_neg_integer(), nil | non_neg_integer(), boolean()}, {:fallback, (... -> any), _}, {:hooks, [{:hook, atom(), _, atom() | pid() | {atom(), _} | {:via, atom(), _}}], [{:hook, atom(), _, atom() | pid() | {atom(), _} | {:via, atom(), _}}]}, {:limit, integer(), atom(), number(), Keyword.t()}, [atom()], boolean(), [{:warmer, atom(), _, boolean()}]}, _record :: {:entry, _, integer(), integer(), _} ) breaks the contract (Cachex.Spec.cache(), [Cachex.Spec.entry()]) :: {:ok, boolean()}
lib/cachex/services/locksmith/queue.ex:26:invalid_contract The @SPEC for the function does not match the success typing of the function. Function: Cachex.Services.Locksmith.Queue.start_link/1 Success typing: @SPEC start_link({:cache, _, _, _, _, _, _, _, _, _, _}) :: :ignore | {:error, _} | {:ok, pid()}
@@ -32,7 +32,7 @@ defmodule Cachex.Actions.Put do | |||
record = entry_now(key: key, ttl: expiry, value: value) | |||
|
|||
Locksmith.write(cache, [key], fn -> | |||
Actions.write(cache, record) | |||
Actions.write(cache, [record]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks strange, but underneath :ets.insert/2
will accept both a list or a single value
https://www.erlang.org/doc/man/ets#insert-2
Between making the @spec
extra long or just sending the data through as a list I opted for the smaller change of wrapping the singular value in a list.
@@ -327,8 +327,9 @@ defmodule Cachex do | |||
""" | |||
@spec start(atom, Keyword.t()) :: {atom, pid} | |||
def start(name, options \\ []) do | |||
with {:ok, pid} <- start_link(name, options) do | |||
:erlang.unlink(pid) && {:ok, pid} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dialyzer wags its finger at us for using &&
like this
btw I don't care if you want to throw away any of the changes like the |
Hi @feld ! Thanks for this, somehow I thought I'd already written on here but it appears I didn't so I'm sorry for that. I'm definitely happy to include all of the I personally don't care about dialyzer complaining about In this case there's only a couple so maybe I just accept it to save people a headache, it's not a big deal. I hope to get this merged tonight either way! |
I'm just going to accept as is, there's no real reason not to. I'll run dialyzer locally and double check everything after the fact. Thanks again @feld ! |
correct |
Here's a collection of dialyzer fixes. The commit logs include the original error message.
There are a few more to clean up but they are either related to
@spec
being wrong fordefmacro
inlib/cachex/spec.ex
which should all be set to returnMacro.t
when they return a quoted value, but that obscures the underlying intention if you wanted to check the code/docs quickly. Not sure what to do with that so it has been left alone.I opted to fix the
@spec
definitions to use the fully qualified type name (e.g.,Cachex.Spec.spec()
) instead of relying on the importedCachex.Spec
for clarity, but in those cases it really could have been shortened to just the spec name.