From fc3ea94a1c17e84033fa593c0ea987fbfa545447 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 09:58:03 -0400 Subject: [PATCH 1/7] Dialyzer: the pattern can never match the type --- lib/pleroma/object/fetcher.ex | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 9d9a201caf..ff7aa539fc 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -58,8 +58,11 @@ def refetch_object(%Object{data: %{"id" => id}} = object) do end end + @typep fetcher_errors :: + :error | :reject | :allowed_depth | :fetch | :containment | :transmogrifier + # Note: will create a Create activity, which we need internally at the moment. - @spec fetch_object_from_id(String.t(), list()) :: {:ok, Object.t()} | {:error | :reject, any()} + @spec fetch_object_from_id(String.t(), list()) :: {:ok, Object.t()} | {fetcher_errors(), any()} def fetch_object_from_id(id, options \\ []) do with {_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)}, {_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])}, From bc16f09d7b9c1a15c8c9be6d99092b9a8d867d18 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:12:13 -0400 Subject: [PATCH 2/7] Dialyzer: the pattern can never match the type The original error was for the chat controller: lib/pleroma/web/pleroma_api/controllers/chat_controller.ex:104:pattern_match The pattern can never match the type {:error, :content_too_long | :forbidden | :no_content | :not_found} | {:user, nil}. Improve typespecs for the Pipeline and apply them where it could be encountered --- lib/pleroma/object/fetcher.ex | 3 +- lib/pleroma/web/activity_pub/pipeline.ex | 19 ++++++---- lib/pleroma/web/common_api.ex | 44 ++++++++++++++---------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index ff7aa539fc..69a5f32685 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -62,7 +62,8 @@ def refetch_object(%Object{data: %{"id" => id}} = object) do :error | :reject | :allowed_depth | :fetch | :containment | :transmogrifier # Note: will create a Create activity, which we need internally at the moment. - @spec fetch_object_from_id(String.t(), list()) :: {:ok, Object.t()} | {fetcher_errors(), any()} + @spec fetch_object_from_id(String.t(), list()) :: + {:ok, Object.t()} | {fetcher_errors(), any()} | Pipeline.errors() def fetch_object_from_id(id, options \\ []) do with {_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)}, {_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])}, diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex index 7f11a4d67e..fc36935d57 100644 --- a/lib/pleroma/web/activity_pub/pipeline.ex +++ b/lib/pleroma/web/activity_pub/pipeline.ex @@ -22,22 +22,27 @@ defp mrf, do: Config.get([:pipeline, :mrf], MRF) defp activity_pub, do: Config.get([:pipeline, :activity_pub], ActivityPub) defp config, do: Config.get([:pipeline, :config], Config) - @spec common_pipeline(map(), keyword()) :: - {:ok, Activity.t() | Object.t(), keyword()} | {:error | :reject, any()} + @type results :: {:ok, Activity.t() | Object.t(), keyword()} + @type errors :: {:error | :reject, any()} + + # The Repo.transaction will wrap the result in an {:ok, _} + # and only returns an {:error, _} if the error encountered was related + # to the SQL transaction + @spec common_pipeline(map(), keyword()) :: results() | errors() def common_pipeline(object, meta) do case Repo.transaction(fn -> do_common_pipeline(object, meta) end, Utils.query_timeout()) do {:ok, {:ok, activity, meta}} -> side_effects().handle_after_transaction(meta) {:ok, activity, meta} - {:ok, value} -> - value + {:ok, {:error, _} = error} -> + error + + {:ok, {:reject, _} = error} -> + error {:error, e} -> {:error, e} - - {:reject, e} -> - {:reject, e} end end diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index 921e414c34..412424dae8 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -26,7 +26,7 @@ defmodule Pleroma.Web.CommonAPI do require Pleroma.Constants require Logger - @spec block(User.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec block(User.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def block(blocked, blocker) do with {:ok, block_data, _} <- Builder.block(blocker, blocked), {:ok, block, _} <- Pipeline.common_pipeline(block_data, local: true) do @@ -35,7 +35,7 @@ def block(blocked, blocker) do end @spec post_chat_message(User.t(), User.t(), String.t(), list()) :: - {:ok, Activity.t()} | {:error, any()} + {:ok, Activity.t()} | Pipeline.errors() def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) do with maybe_attachment <- opts[:media_id] && Object.get_by_id(opts[:media_id]), :ok <- validate_chat_attachment_attribution(maybe_attachment, user), @@ -58,7 +58,7 @@ def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) )} do {:ok, activity} else - {:common_pipeline, {:reject, _} = e} -> e + {:common_pipeline, e} -> e e -> e end end @@ -99,7 +99,8 @@ defp validate_chat_content_length(content, _) do end end - @spec unblock(User.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec unblock(User.t(), User.t()) :: + {:ok, Activity.t()} | {:ok, :no_activity} | Pipeline.errors() | {:error, :not_blocking} def unblock(blocked, blocker) do with {_, %Activity{} = block} <- {:fetch_block, Utils.fetch_latest_block(blocker, blocked)}, {:ok, unblock_data, _} <- Builder.undo(blocker, block), @@ -120,7 +121,9 @@ def unblock(blocked, blocker) do end @spec follow(User.t(), User.t()) :: - {:ok, User.t(), User.t(), Activity.t() | Object.t()} | {:error, :rejected} + {:ok, User.t(), User.t(), Activity.t() | Object.t()} + | {:error, :rejected} + | Pipeline.errors() def follow(followed, follower) do timeout = Pleroma.Config.get([:activitypub, :follow_handshake_timeout]) @@ -145,7 +148,7 @@ def unfollow(unfollowed, follower) do end end - @spec accept_follow_request(User.t(), User.t()) :: {:ok, User.t()} | {:error, any()} + @spec accept_follow_request(User.t(), User.t()) :: {:ok, User.t()} | Pipeline.errors() def accept_follow_request(follower, followed) do with %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), {:ok, accept_data, _} <- Builder.accept(followed, follow_activity), @@ -154,7 +157,7 @@ def accept_follow_request(follower, followed) do end end - @spec reject_follow_request(User.t(), User.t()) :: {:ok, User.t()} | {:error, any()} | nil + @spec reject_follow_request(User.t(), User.t()) :: {:ok, User.t()} | Pipeline.errors() | nil def reject_follow_request(follower, followed) do with %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), {:ok, reject_data, _} <- Builder.reject(followed, follow_activity), @@ -163,7 +166,8 @@ def reject_follow_request(follower, followed) do end end - @spec delete(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec delete(String.t(), User.t()) :: + {:ok, Activity.t()} | Pipeline.errors() | {:error, :not_found | String.t()} def delete(activity_id, user) do with {_, %Activity{data: %{"object" => _, "type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(activity_id, filter: [])}, @@ -213,7 +217,7 @@ def delete(activity_id, user) do end end - @spec repeat(String.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, any()} + @spec repeat(String.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, :not_found} def repeat(id, user, params \\ %{}) do with %Activity{data: %{"type" => "Create"}} = activity <- Activity.get_by_id(id), object = %Object{} <- Object.normalize(activity, fetch: false), @@ -231,7 +235,7 @@ def repeat(id, user, params \\ %{}) do end end - @spec unrepeat(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec unrepeat(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, :not_found | String.t()} def unrepeat(id, user) do with {_, %Activity{data: %{"type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(id)}, @@ -247,7 +251,8 @@ def unrepeat(id, user) do end end - @spec favorite(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec favorite(String.t(), User.t()) :: + {:ok, Activity.t()} | {:ok, :already_liked} | {:error, :not_found | String.t()} def favorite(id, %User{} = user) do case favorite_helper(user, id) do {:ok, _} = res -> @@ -285,7 +290,8 @@ defp favorite_helper(user, id) do end end - @spec unfavorite(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec unfavorite(String.t(), User.t()) :: + {:ok, Activity.t()} | {:error, :not_found | String.t()} def unfavorite(id, user) do with {_, %Activity{data: %{"type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(id)}, @@ -302,7 +308,7 @@ def unfavorite(id, user) do end @spec react_with_emoji(String.t(), User.t(), String.t()) :: - {:ok, Activity.t()} | {:error, any()} + {:ok, Activity.t()} | {:error, String.t()} def react_with_emoji(id, user, emoji) do with %Activity{} = activity <- Activity.get_by_id(id), object <- Object.normalize(activity, fetch: false), @@ -316,7 +322,7 @@ def react_with_emoji(id, user, emoji) do end @spec unreact_with_emoji(String.t(), User.t(), String.t()) :: - {:ok, Activity.t()} | {:error, any()} + {:ok, Activity.t()} | {:error, String.t()} def unreact_with_emoji(id, user, emoji) do with %Activity{} = reaction_activity <- Utils.get_latest_reaction(id, user, emoji), {_, {:ok, _}} <- {:cancel_jobs, maybe_cancel_jobs(reaction_activity)}, @@ -329,7 +335,7 @@ def unreact_with_emoji(id, user, emoji) do end end - @spec vote(Object.t(), User.t(), list()) :: {:ok, list(), Object.t()} | {:error, any()} + @spec vote(Object.t(), User.t(), list()) :: {:ok, list(), Object.t()} | Pipeline.errors() def vote(%Object{data: %{"type" => "Question"}} = object, %User{} = user, choices) do with :ok <- validate_not_author(object, user), :ok <- validate_existing_votes(user, object), @@ -461,7 +467,7 @@ def post(user, %{status: _} = data) do end end - @spec update(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, any()} + @spec update(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, nil} def update(orig_activity, %User{} = user, changes) do with orig_object <- Object.normalize(orig_activity), {:ok, new_object} <- make_update_data(user, orig_object, changes), @@ -497,7 +503,7 @@ defp make_update_data(user, orig_object, changes) do end end - @spec pin(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, term()} + @spec pin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def pin(id, %User{} = user) do with %Activity{} = activity <- create_activity_by_id(id), true <- activity_belongs_to_actor(activity, user.ap_id), @@ -537,7 +543,7 @@ defp activity_is_public(activity) do end end - @spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, term()} + @spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def unpin(id, user) do with %Activity{} = activity <- create_activity_by_id(id), {:ok, unpin_data, _} <- Builder.unpin(user, activity.object), @@ -552,7 +558,7 @@ def unpin(id, user) do end end - @spec add_mute(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, any()} + @spec add_mute(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, String.t()} def add_mute(activity, user, params \\ %{}) do expires_in = Map.get(params, :expires_in, 0) From 7eb579c1911f2eac175c2030f6bb80685b4ab4f8 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:18:12 -0400 Subject: [PATCH 3/7] Dialyzer: invalid contract --- lib/pleroma/user/import.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/user/import.ex b/lib/pleroma/user/import.ex index b79fa88eb2..ab6bdb8d41 100644 --- a/lib/pleroma/user/import.ex +++ b/lib/pleroma/user/import.ex @@ -12,7 +12,7 @@ defmodule Pleroma.User.Import do require Logger - @spec perform(atom(), User.t(), list()) :: :ok | list() | {:error, any()} + @spec perform(atom(), User.t(), String.t()) :: :ok | {:error, any()} def perform(:mute_import, %User{} = user, actor) do with {:ok, %User{} = muted_user} <- User.get_or_fetch(actor), {_, false} <- {:existing_mute, User.mutes_user?(user, muted_user)}, @@ -49,7 +49,7 @@ def perform(:follow_import, %User{} = user, actor) do defp handle_error(op, user_id, error) do Logger.debug("#{op} failed for #{user_id} with: #{inspect(error)}") - error + {:error, error} end def blocks_import(%User{} = user, [_ | _] = actors) do From 06d6febff960e5aafd44709d0a61311b45892a81 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:19:24 -0400 Subject: [PATCH 4/7] Dialyzer: The pattern variable _e@1 can never match the type, because it is covered by previous clauses. --- lib/pleroma/user/backup.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/pleroma/user/backup.ex b/lib/pleroma/user/backup.ex index 7feaa22bf5..70cf5b2a1c 100644 --- a/lib/pleroma/user/backup.ex +++ b/lib/pleroma/user/backup.ex @@ -92,9 +92,6 @@ def schedule_backup(backup) do else true -> {:error, "Backup is missing id. Please insert it into the Repo first."} - - e -> - {:error, e} end end From 1d0e3b1355c5a5883be1522f0f925c398c0e87a4 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:24:37 -0400 Subject: [PATCH 5/7] Dialyzer: The pattern variable _ can never match the type, because it is covered by previous clauses. --- lib/pleroma/user/backup.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/pleroma/user/backup.ex b/lib/pleroma/user/backup.ex index 70cf5b2a1c..c5038c8f46 100644 --- a/lib/pleroma/user/backup.ex +++ b/lib/pleroma/user/backup.ex @@ -294,9 +294,6 @@ defp write(query, dir, name, fun) do ) acc - - _ -> - acc end end) From 06ce5e3b43b4a6809397bdb0eb192a82e7243e93 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:27:07 -0400 Subject: [PATCH 6/7] Dialyzer: pattern_match The pattern can never match the type {:diff, false}. --- lib/pleroma/user/backup.ex | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/user/backup.ex b/lib/pleroma/user/backup.ex index c5038c8f46..d77d49890f 100644 --- a/lib/pleroma/user/backup.ex +++ b/lib/pleroma/user/backup.ex @@ -118,14 +118,13 @@ def schedule_delete(backup) do end defp permitted?(user) do - with {_, %__MODULE__{inserted_at: inserted_at}} <- {:last, get_last(user)}, - days = Config.get([__MODULE__, :limit_days]), - diff = Timex.diff(NaiveDateTime.utc_now(), inserted_at, :days), - {_, true} <- {:diff, diff > days} do - true + with {_, %__MODULE__{inserted_at: inserted_at}} <- {:last, get_last(user)} do + days = Config.get([__MODULE__, :limit_days]) + diff = Timex.diff(NaiveDateTime.utc_now(), inserted_at, :days) + + diff > days else {:last, nil} -> true - {:diff, false} -> false end end From 5b26c56624ad281987a18091a6ae245833d2fde1 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:34:06 -0400 Subject: [PATCH 7/7] Changelog --- changelog.d/dialyzer.skip | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 changelog.d/dialyzer.skip diff --git a/changelog.d/dialyzer.skip b/changelog.d/dialyzer.skip new file mode 100644 index 0000000000..e69de29bb2