Skip to content

Commit

Permalink
Add various Dialyzer fixes (#324)
Browse files Browse the repository at this point in the history
  • Loading branch information
feld authored Mar 11, 2024
1 parent 584dac6 commit b7dc0be
Show file tree
Hide file tree
Showing 16 changed files with 61 additions and 53 deletions.
16 changes: 10 additions & 6 deletions lib/cachex.ex
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ defmodule Cachex do
import Kernel, except: [inspect: 2]

# the cache type
@type cache :: atom | Spec.cache()
@type cache :: atom | Cachex.Spec.cache()

# custom status type
@type status :: :ok | :error
Expand Down Expand Up @@ -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}
with {:ok, pid} <- start_link(name, options),
true <- :erlang.unlink(pid) do
{:ok, pid}
end
end

Expand All @@ -337,7 +338,10 @@ defmodule Cachex do
#
# This will start all cache services required using the `Cachex.Services`
# module and attach them under a Supervisor instance backing the cache.
@spec init(cache :: Spec.cache()) :: Supervisor.on_start()
@spec init(cache :: Cachex.Spec.cache()) ::
{:ok,
{Supervisor.sup_flags(),
[Supervisor.child_spec() | (old_erlang_child_spec :: Supervisor.child_spec())]}}
def init(cache() = cache) do
cache
|> Services.cache_spec()
Expand Down Expand Up @@ -621,7 +625,7 @@ defmodule Cachex do
{ :ok, [ { :entry, "key", 1538714590095, nil, "value" } ] }
"""
@spec export(cache, Keyword.t()) :: {status, [Spec.entry()]}
@spec export(cache, Keyword.t()) :: {status, [Cachex.Spec.entry()]}
def export(cache, options \\ []) when is_list(options),
do: Router.call(cache, {:export, [options]})

Expand Down Expand Up @@ -782,7 +786,7 @@ defmodule Cachex do
{ :ok, true }
"""
@spec import(cache, [Spec.entry()], Keyword.t()) :: {status, any}
@spec import(cache, [Cachex.Spec.entry()], Keyword.t()) :: {status, any}
def import(cache, entries, options \\ [])
when is_list(entries) and is_list(options),
do: Router.call(cache, {:import, [entries, options]})
Expand Down
6 changes: 3 additions & 3 deletions lib/cachex/actions.ex
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ defmodule Cachex.Actions do
This will return an instance of an entry record as defined in the main
`Cachex.Spec` module, rather than just the raw value.
"""
@spec read(Spec.cache(), any) :: Spec.entry() | nil
@spec read(Cachex.Spec.cache(), any) :: Cachex.Spec.entry() | nil
def read(cache(name: name) = cache, key) do
case :ets.lookup(name, key) do
[] ->
Expand All @@ -91,14 +91,14 @@ defmodule Cachex.Actions do
Note that updates are atomic; either all updates will take place, or none will.
"""
@spec update(Spec.cache(), any, [tuple]) :: {:ok, boolean}
@spec update(Cachex.Spec.cache(), any, [tuple]) :: {:ok, boolean}
def update(cache(name: name), key, changes),
do: {:ok, :ets.update_element(name, key, changes)}

@doc """
Writes a new entry into a cache.
"""
@spec write(Spec.cache(), [Spec.entry()]) :: {:ok, boolean}
@spec write(Cachex.Spec.cache(), [Cachex.Spec.entry()]) :: {:ok, boolean}
def write(cache(name: name), entries),
do: {:ok, :ets.insert(name, entries)}

Expand Down
2 changes: 1 addition & 1 deletion lib/cachex/actions/put.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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])
end)
end
end
2 changes: 1 addition & 1 deletion lib/cachex/actions/stats.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ defmodule Cachex.Actions.Stats do
If the provided cache does not have statistics enabled, an error will be returned.
"""
@spec execute(Spec.cache(), Keyword.t()) ::
@spec execute(Cachex.Spec.cache(), Keyword.t()) ::
{:ok, %{}} | {:error, :stats_disabled}
def execute(cache() = cache, _options) do
with {:ok, stats} <- Stats.retrieve(cache) do
Expand Down
2 changes: 1 addition & 1 deletion lib/cachex/policy.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ defmodule Cachex.Policy do
@doc """
Returns any hook definitions required for this policy.
"""
@callback hooks(Spec.limit()) :: [Spec.hook()]
@callback hooks(Cachex.Spec.limit()) :: [Cachex.Spec.hook()]

##################
# Implementation #
Expand Down
2 changes: 1 addition & 1 deletion lib/cachex/policy/lrw.ex
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ defmodule Cachex.Policy.LRW do
# before attempting to trim older cache entries.
#
# Please see module documentation for options available inside the limits.
@spec apply_limit(Spec.cache(), Spec.limit()) :: :ok
@spec apply_limit(Cachex.Spec.cache(), Cachex.Spec.limit()) :: :ok
def apply_limit(cache() = cache, limit() = limit) do
limit(size: max_size, reclaim: reclaim, options: options) = limit

Expand Down
6 changes: 4 additions & 2 deletions lib/cachex/policy/lrw/evented.ex
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ defmodule Cachex.Policy.LRW.Evented do
# Note that this will ignore error results and only operates on actions which are
# able to cause a net gain in cache size (so removals are also ignored).
def handle_notify(_message, {status, _value}, {cache, limit} = opts)
when status not in @ignored,
do: LRW.apply_limit(cache, limit) && {:ok, opts}
when status not in @ignored do
LRW.apply_limit(cache, limit)
{:ok, opts}
end

def handle_notify(_message, _result, opts),
do: {:ok, opts}
Expand Down
9 changes: 4 additions & 5 deletions lib/cachex/services.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ defmodule Cachex.Services do

# add some aliases
alias Cachex.Services
alias Supervisor.Spec

##############
# Public API #
Expand All @@ -26,7 +25,7 @@ defmodule Cachex.Services do
At the time of writing, the order does not matter - but that does not
mean this will always be the case, so please be careful when modifying.
"""
@spec app_spec :: [Spec.spec()]
@spec app_spec :: [Supervisor.child_spec()]
def app_spec,
do: [
%{
Expand All @@ -50,7 +49,7 @@ defmodule Cachex.Services do
Definition order here matters, as there's inter-dependency between each
of the child processes (such as the Janitor -> Locksmith).
"""
@spec cache_spec(Spec.cache()) :: [Spec.spec()]
@spec cache_spec(Cachex.Spec.cache()) :: [Supervisor.Spec.spec()]
def cache_spec(cache() = cache) do
[]
|> Enum.concat(table_spec(cache))
Expand All @@ -66,7 +65,7 @@ defmodule Cachex.Services do
This will return `nil` if the service does not exist, or is not running.
"""
@spec locate(Spec.cache(), atom) :: pid | nil
@spec locate(Cachex.Spec.cache(), atom) :: pid | nil
def locate(cache() = cache, service) do
Enum.find_value(services(cache), fn
{^service, pid, _tag, _id} -> pid
Expand All @@ -80,7 +79,7 @@ defmodule Cachex.Services do
This is used to view the children of the specified cache, whilst filtering
out any services which may not have been started based on the cache options.
"""
@spec services(Spec.cache()) :: [Spec.spec()]
@spec services(Cachex.Spec.cache()) :: [Supervisor.Spec.spec()]
def services(cache(name: cache)) do
cache
|> Supervisor.which_children()
Expand Down
4 changes: 2 additions & 2 deletions lib/cachex/services/courier.ex
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ defmodule Cachex.Services.Courier do
@doc """
Starts a new Courier process for a cache.
"""
@spec start_link(Spec.cache()) :: GenServer.on_start()
@spec start_link(Cachex.Spec.cache()) :: GenServer.on_start()
def start_link(cache(name: name) = cache),
do: GenServer.start_link(__MODULE__, cache, name: name(name, :courier))

Expand All @@ -39,7 +39,7 @@ defmodule Cachex.Services.Courier do
simplify the interfaces internally. This is a blocking remote
call which will wait until a result can be loaded.
"""
@spec dispatch(Spec.cache(), any, (-> any)) :: any
@spec dispatch(Cachex.Spec.cache(), any, (() -> any)) :: any
def dispatch(cache() = cache, key, task) when is_function(task, 0),
do: service_call(cache, :courier, {:dispatch, key, task, local_stack()})

Expand Down
2 changes: 1 addition & 1 deletion lib/cachex/services/incubator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ defmodule Cachex.Services.Incubator do
the provided cache record. If no warmers are attached in the cache record,
this will skip creation to avoid unnecessary processes running.
"""
@spec start_link(Spec.cache()) :: Supervisor.on_start()
@spec start_link(Cachex.Spec.cache()) :: Supervisor.on_start()
def start_link(cache(warmers: [])),
do: :ignore

Expand Down
10 changes: 5 additions & 5 deletions lib/cachex/services/informant.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ defmodule Cachex.Services.Informant do
the provided cache record. If no hooks are attached in the cache record,
this will skip creating an unnecessary Supervisor process.
"""
@spec start_link(Spec.cache()) :: Supervisor.on_start()
@spec start_link(Cachex.Spec.cache()) :: Supervisor.on_start()
def start_link(cache(hooks: hooks(pre: [], post: []))),
do: :ignore

Expand All @@ -36,14 +36,14 @@ defmodule Cachex.Services.Informant do
This will send a nil result, as the result does not yet exist.
"""
@spec broadcast(Spec.cache(), tuple) :: :ok
@spec broadcast(Cachex.Spec.cache(), tuple) :: :ok
def broadcast(cache(hooks: hooks(pre: pre_hooks)), action),
do: broadcast_action(pre_hooks, action, nil)

@doc """
Broadcasts an action and result to all post-hooks in a cache.
"""
@spec broadcast(Spec.cache(), tuple, any) :: :ok
@spec broadcast(Cachex.Spec.cache(), tuple, any) :: :ok
def broadcast(cache(hooks: hooks(post: post_hooks)), action, result),
do: broadcast_action(post_hooks, action, result)

Expand All @@ -54,7 +54,7 @@ defmodule Cachex.Services.Informant do
are not named in a deterministic way. It will look up all hooks using
the Supervisor children and place them in a modified cache record.
"""
@spec link(Spec.cache()) :: {:ok, Spec.cache()}
@spec link(Cachex.Spec.cache()) :: {:ok, Cachex.Spec.cache()}
def link(cache(hooks: hooks(pre: [], post: [])) = cache),
do: {:ok, cache}

Expand All @@ -80,7 +80,7 @@ defmodule Cachex.Services.Informant do
This is the underlying implementation for `broadcast/2` and `broadcast/3`,
but it's general purpose enough that it's exposed as part of the public API.
"""
@spec notify([Spec.hook()], tuple, any) :: :ok
@spec notify([Cachex.Spec.hook()], tuple, any) :: :ok
def notify(hooks, {_name, _args} = action, result) when is_list(hooks) do
Enum.each(hooks, fn
# not running, so skip
Expand Down
13 changes: 7 additions & 6 deletions lib/cachex/services/janitor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ defmodule Cachex.Services.Janitor do
At this point customization is non-existent, in order to keep the service
as simple as possible and avoid the space for error and edge cases.
"""
@spec start_link(Spec.cache()) :: GenServer.on_start()
@spec start_link(Cachex.Spec.cache()) :: GenServer.on_start()
def start_link(cache(name: name) = cache),
do: GenServer.start_link(__MODULE__, cache, name: name(name, :janitor))

@doc """
Pulls an expiration associated with an entry.
"""
@spec expiration(Spec.cache(), integer) :: integer
@spec expiration(Cachex.Spec.cache(), integer | nil) :: integer
def expiration(cache(expiration: expiration(default: default)), nil),
do: default

Expand All @@ -51,7 +51,7 @@ defmodule Cachex.Services.Janitor do
This will take cache lazy expiration settings into account.
"""
@spec expired?(Spec.cache(), Spec.entry()) :: boolean
@spec expired?(Cachex.Spec.cache(), Cachex.Spec.entry()) :: boolean
def expired?(cache(expiration: expiration(lazy: lazy)), entry() = entry),
do: lazy and expired?(entry)

Expand All @@ -60,7 +60,7 @@ defmodule Cachex.Services.Janitor do
This will not cache lazy expiration settings into account.
"""
@spec expired?(Spec.entry()) :: boolean
@spec expired?(Cachex.Spec.entry()) :: boolean
def expired?(entry(touched: touched, ttl: ttl)) when is_number(ttl),
do: touched + ttl < now()

Expand All @@ -72,7 +72,7 @@ defmodule Cachex.Services.Janitor do
If the service is disabled on the cache, an error is returned.
"""
@spec last_run(Spec.cache()) :: %{}
@spec last_run(Cachex.Spec.cache()) :: %{}
def last_run(cache(expiration: expiration(interval: nil))),
do: error(:janitor_disabled)

Expand Down Expand Up @@ -133,6 +133,7 @@ defmodule Cachex.Services.Janitor do
# Schedules a check to occur after the designated interval. Once scheduled,
# returns the state - this is just sugar for pipelining with a state.
defp schedule_check(cache(expiration: expiration(interval: interval)) = cache) do
:erlang.send_after(interval, self(), :ttl_check) && cache
:erlang.send_after(interval, self(), :ttl_check)
cache
end
end
12 changes: 6 additions & 6 deletions lib/cachex/services/locksmith.ex
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ defmodule Cachex.Services.Locksmith do
returned boolean will signal if the lock was successful. A lock can fail
if one of the provided keys is already locked.
"""
@spec lock(Spec.cache(), [any]) :: boolean
@spec lock(Cachex.Spec.cache(), [any]) :: boolean
def lock(cache(name: name), keys) do
t_proc = self()

Expand All @@ -69,7 +69,7 @@ defmodule Cachex.Services.Locksmith do
This uses some ETS matching voodoo to pull back the locked keys. They
won't be returned in any specific order, so don't rely on it.
"""
@spec locked(Spec.cache()) :: [any]
@spec locked(Cachex.Spec.cache()) :: [any]
def locked(cache(name: name)),
do: :ets.select(@table_name, [{{{name, :"$1"}, :_}, [], [:"$1"]}])

Expand All @@ -79,7 +79,7 @@ defmodule Cachex.Services.Locksmith do
For a key to be writeable, it must either have no lock or be locked by the
calling process.
"""
@spec locked?(Spec.cache(), [any]) :: true | false
@spec locked?(Cachex.Spec.cache(), [any]) :: true | false
def locked?(cache(name: name), keys) when is_list(keys) do
Enum.any?(keys, fn key ->
case :ets.lookup(@table_name, {name, key}) do
Expand All @@ -101,7 +101,7 @@ defmodule Cachex.Services.Locksmith do
This is mainly shorthand to avoid having to handle row locking explicitly.
"""
@spec transaction(Spec.cache(), [any], (-> any)) :: any
@spec transaction(Cachex.Spec.cache(), [any], (() -> any)) :: any
def transaction(cache() = cache, keys, fun) when is_list(keys) do
case transaction?() do
true -> fun.()
Expand Down Expand Up @@ -138,7 +138,7 @@ defmodule Cachex.Services.Locksmith do
is a little less desirable, but needs must.
"""
# TODO: figure out how to remove atomically
@spec unlock(Spec.cache(), [any]) :: true
@spec unlock(Cachex.Spec.cache(), [any]) :: true
def unlock(cache(name: name), keys) do
keys
|> List.wrap()
Expand All @@ -155,7 +155,7 @@ defmodule Cachex.Services.Locksmith do
transactions executed against it we skip the lock check as any of
our ETS writes are atomic and so do not require a lock.
"""
@spec write(Spec.cache(), any, (-> any)) :: any
@spec write(Cachex.Spec.cache(), any, (() -> any)) :: any
def write(cache(transactional: false), _keys, fun),
do: fun.()

Expand Down
6 changes: 3 additions & 3 deletions lib/cachex/services/locksmith/queue.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@ defmodule Cachex.Services.Locksmith.Queue do
This is little more than starting a GenServer process using this module,
although it does use the provided cache record to name the new server.
"""
@spec start_link(Spec.cache()) :: [GenServer.on_start()]
@spec start_link(Cachex.Spec.cache()) :: GenServer.on_start()
def start_link(cache(name: name) = cache),
do: GenServer.start_link(__MODULE__, cache, name: name(name, :locksmith))

@doc """
Executes a function in a lock-free context.
"""
@spec execute(Spec.cache(), (-> any)) :: any
@spec execute(Cachex.Spec.cache(), (() -> any)) :: any
def execute(cache() = cache, func) when is_function(func, 0),
do: service_call(cache, :locksmith, {:exec, func})

@doc """
Executes a function in a transactional context.
"""
@spec transaction(Spec.cache(), [any], (-> any)) :: any
@spec transaction(Cachex.Spec.cache(), [any], (() -> any)) :: any
def transaction(cache() = cache, keys, func)
when is_list(keys) and is_function(func, 0),
do: service_call(cache, :locksmith, {:transaction, keys, func})
Expand Down
Loading

0 comments on commit b7dc0be

Please sign in to comment.