From b7dc0befc969b987cbf4ece92c2c0c90fcf4b603 Mon Sep 17 00:00:00 2001 From: feld <40271278+feld@users.noreply.github.com> Date: Mon, 11 Mar 2024 18:50:27 -0400 Subject: [PATCH] Add various Dialyzer fixes (#324) --- lib/cachex.ex | 16 ++++++++++------ lib/cachex/actions.ex | 6 +++--- lib/cachex/actions/put.ex | 2 +- lib/cachex/actions/stats.ex | 2 +- lib/cachex/policy.ex | 2 +- lib/cachex/policy/lrw.ex | 2 +- lib/cachex/policy/lrw/evented.ex | 6 ++++-- lib/cachex/services.ex | 9 ++++----- lib/cachex/services/courier.ex | 4 ++-- lib/cachex/services/incubator.ex | 2 +- lib/cachex/services/informant.ex | 10 +++++----- lib/cachex/services/janitor.ex | 13 +++++++------ lib/cachex/services/locksmith.ex | 12 ++++++------ lib/cachex/services/locksmith/queue.ex | 6 +++--- lib/cachex/services/overseer.ex | 16 +++++++++------- lib/cachex/stats.ex | 6 +++--- 16 files changed, 61 insertions(+), 53 deletions(-) diff --git a/lib/cachex.ex b/lib/cachex.ex index 7897ffb..8ecb525 100644 --- a/lib/cachex.ex +++ b/lib/cachex.ex @@ -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 @@ -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 @@ -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() @@ -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]}) @@ -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]}) diff --git a/lib/cachex/actions.ex b/lib/cachex/actions.ex index 0d52063..1b825da 100644 --- a/lib/cachex/actions.ex +++ b/lib/cachex/actions.ex @@ -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 [] -> @@ -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)} diff --git a/lib/cachex/actions/put.ex b/lib/cachex/actions/put.ex index 5768cf2..7a12e26 100644 --- a/lib/cachex/actions/put.ex +++ b/lib/cachex/actions/put.ex @@ -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 diff --git a/lib/cachex/actions/stats.ex b/lib/cachex/actions/stats.ex index 0137002..135b21c 100644 --- a/lib/cachex/actions/stats.ex +++ b/lib/cachex/actions/stats.ex @@ -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 diff --git a/lib/cachex/policy.ex b/lib/cachex/policy.ex index b555499..9f844ba 100644 --- a/lib/cachex/policy.ex +++ b/lib/cachex/policy.ex @@ -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 # diff --git a/lib/cachex/policy/lrw.ex b/lib/cachex/policy/lrw.ex index 6fed953..95c2d29 100644 --- a/lib/cachex/policy/lrw.ex +++ b/lib/cachex/policy/lrw.ex @@ -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 diff --git a/lib/cachex/policy/lrw/evented.ex b/lib/cachex/policy/lrw/evented.ex index aac023b..8968258 100644 --- a/lib/cachex/policy/lrw/evented.ex +++ b/lib/cachex/policy/lrw/evented.ex @@ -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} diff --git a/lib/cachex/services.ex b/lib/cachex/services.ex index a5b85cd..c1e44b1 100644 --- a/lib/cachex/services.ex +++ b/lib/cachex/services.ex @@ -11,7 +11,6 @@ defmodule Cachex.Services do # add some aliases alias Cachex.Services - alias Supervisor.Spec ############## # Public API # @@ -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: [ %{ @@ -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)) @@ -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 @@ -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() diff --git a/lib/cachex/services/courier.ex b/lib/cachex/services/courier.ex index a8c5292..04ea3a6 100644 --- a/lib/cachex/services/courier.ex +++ b/lib/cachex/services/courier.ex @@ -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)) @@ -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()}) diff --git a/lib/cachex/services/incubator.ex b/lib/cachex/services/incubator.ex index 534dc3c..5f654ab 100644 --- a/lib/cachex/services/incubator.ex +++ b/lib/cachex/services/incubator.ex @@ -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 diff --git a/lib/cachex/services/informant.ex b/lib/cachex/services/informant.ex index 1f101f7..fb9d238 100644 --- a/lib/cachex/services/informant.ex +++ b/lib/cachex/services/informant.ex @@ -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 @@ -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) @@ -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} @@ -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 diff --git a/lib/cachex/services/janitor.ex b/lib/cachex/services/janitor.ex index 08cda1a..b4e8eec 100644 --- a/lib/cachex/services/janitor.ex +++ b/lib/cachex/services/janitor.ex @@ -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 @@ -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) @@ -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() @@ -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) @@ -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 diff --git a/lib/cachex/services/locksmith.ex b/lib/cachex/services/locksmith.ex index 72e5293..1072762 100644 --- a/lib/cachex/services/locksmith.ex +++ b/lib/cachex/services/locksmith.ex @@ -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() @@ -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"]}]) @@ -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 @@ -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.() @@ -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() @@ -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.() diff --git a/lib/cachex/services/locksmith/queue.ex b/lib/cachex/services/locksmith/queue.ex index b940f53..df2cf92 100644 --- a/lib/cachex/services/locksmith/queue.ex +++ b/lib/cachex/services/locksmith/queue.ex @@ -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}) diff --git a/lib/cachex/services/overseer.ex b/lib/cachex/services/overseer.ex index f5ddab8..365fc00 100644 --- a/lib/cachex/services/overseer.ex +++ b/lib/cachex/services/overseer.ex @@ -17,7 +17,6 @@ defmodule Cachex.Services.Overseer do # add any aliases alias Cachex.Services - alias Supervisor.Spec # add service aliases alias Services.Overseer @@ -59,7 +58,7 @@ defmodule Cachex.Services.Overseer do Ensuring a cache will map the provided argument to a cache record if available, otherwise a nil value. """ - @spec ensure(atom | Spec.cache()) :: Spec.cache() | nil + @spec ensure(atom | Cachex.Spec.cache()) :: Cachex.Spec.cache() | nil def ensure(cache() = cache), do: cache @@ -79,14 +78,14 @@ defmodule Cachex.Services.Overseer do @doc """ Registers a cache record against a name. """ - @spec register(atom, Spec.cache()) :: true + @spec register(atom, Cachex.Spec.cache()) :: true def register(name, cache() = cache) when is_atom(name), do: :ets.insert(@table_name, {name, cache}) @doc """ Retrieves a cache record, or `nil` if none exists. """ - @spec retrieve(atom) :: Spec.cache() | nil + @spec retrieve(atom) :: Cachex.Spec.cache() | nil def retrieve(name) do case :ets.lookup(@table_name, name) do [{^name, state}] -> @@ -107,7 +106,7 @@ defmodule Cachex.Services.Overseer do @doc """ Carries out a transaction against the state table. """ - @spec transaction(atom, (-> any)) :: any + @spec transaction(atom, (() -> any)) :: any def transaction(name, fun) when is_atom(name) and is_function(fun, 0), do: :sleeplocks.execute(@manager_name, fun) @@ -124,8 +123,11 @@ defmodule Cachex.Services.Overseer do This is atomic and happens inside a transaction to ensure that we don't get out of sync. Hooks are notified of the change, and the new state is returned. """ - @spec update(atom, Spec.cache() | (Spec.cache() -> Spec.cache())) :: - Spec.cache() + @spec update( + atom, + Cachex.Spec.cache() | (Cachex.Spec.cache() -> Cachex.Spec.cache()) + ) :: + Cachex.Spec.cache() def update(name, fun) when is_atom(name) and is_function(fun, 1) do transaction(name, fn -> cstate = retrieve(name) diff --git a/lib/cachex/stats.ex b/lib/cachex/stats.ex index dcfa2c4..66b1a91 100644 --- a/lib/cachex/stats.ex +++ b/lib/cachex/stats.ex @@ -36,21 +36,21 @@ defmodule Cachex.Stats do @doc """ Determines if stats are enabled for a cache. """ - @spec enabled?(Spec.cache()) :: boolean + @spec enabled?(Cachex.Spec.cache()) :: boolean def enabled?(cache() = cache), do: locate(cache) != nil @doc """ Locates a stats hook for a cache, if enabled. """ - @spec locate(Spec.cache()) :: Spec.hook() | nil + @spec locate(Cachex.Spec.cache()) :: Cachex.Spec.hook() | nil def locate(cache(hooks: hooks(post: post_hooks))), do: Enum.find(post_hooks, &match?(hook(module: Cachex.Stats), &1)) @doc """ Retrieves the latest statistics for a cache. """ - @spec retrieve(Spec.cache()) :: %{} + @spec retrieve(Cachex.Spec.cache()) :: {:ok, map()} | {:error, atom()} def retrieve(cache(name: name) = cache) do case enabled?(cache) do false ->