From ff42f24972f2ad95a9cc0455553b65f68f625ac1 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 21 May 2026 08:08:11 +0100 Subject: [PATCH] ML-195: Add credo check for put_toast/put_toast! usage in live components --- .credo.exs | 6 +- ...ck-for-LiveComponent-toast-helper-usage.md | 73 +++++++++ .../credo/no_live_component_put_toast.ex | 145 ++++++++++++++++++ .../components/record_form.ex | 10 +- .../no_live_component_put_toast_test.exs | 82 ++++++++++ 5 files changed, 310 insertions(+), 6 deletions(-) create mode 100644 backlog/tasks/ml-195 - Add-Credo-check-for-LiveComponent-toast-helper-usage.md create mode 100644 lib/music_library/credo/no_live_component_put_toast.ex create mode 100644 test/music_library/credo/no_live_component_put_toast_test.exs diff --git a/.credo.exs b/.credo.exs index 7b156d1d..283d0977 100644 --- a/.credo.exs +++ b/.credo.exs @@ -2,11 +2,15 @@ configs: [ %{ name: "default", - requires: ["./deps/phoenix_test/lib/phoenix_test/credo/**/*.ex"], + requires: [ + "./deps/phoenix_test/lib/phoenix_test/credo/**/*.ex", + "./lib/music_library/credo/**/*.ex" + ], plugins: [{ExSlop, []}], checks: %{ extra: [ {PhoenixTest.Credo.NoOpenBrowser, []}, + {MusicLibrary.Credo.NoLiveComponentPutToast, []}, {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []}, {Credo.Check.Refactor.DoubleBooleanNegation, []}, {Credo.Check.Refactor.CondStatements, []}, diff --git a/backlog/tasks/ml-195 - Add-Credo-check-for-LiveComponent-toast-helper-usage.md b/backlog/tasks/ml-195 - Add-Credo-check-for-LiveComponent-toast-helper-usage.md new file mode 100644 index 00000000..4edacfd8 --- /dev/null +++ b/backlog/tasks/ml-195 - Add-Credo-check-for-LiveComponent-toast-helper-usage.md @@ -0,0 +1,73 @@ +--- +id: ML-195 +title: Add Credo check for LiveComponent toast helper usage +status: Done +assignee: + - pi +created_date: "2026-05-21 06:57" +updated_date: "2026-05-21 07:05" +labels: [] +dependencies: [] +priority: medium +ordinal: 38000 +--- + +## Description + + + +Add a custom Credo check that flags calls to `put_toast/3` from LiveComponent modules so contributors use the LiveComponent-safe `put_toast!/2` helper instead. + + + +## Acceptance Criteria + + + +- [x] #1 Credo fails when a LiveComponent module calls `put_toast/3`. +- [x] #2 Credo does not flag allowed `put_toast!/2` calls inside LiveComponents. +- [x] #3 Credo does not flag `put_toast/3` usage outside LiveComponent modules. +- [x] #4 Automated tests cover the new check behavior. + + +## Implementation Plan + + + +1. Review Credo configuration and any existing custom checks/tests to match project conventions. +2. Add a custom Credo check that detects modules using `use MusicLibraryWeb, :live_component` and reports calls to `put_toast/3`, including pipeline form when applicable. +3. Register the check in Credo config so `mix credo` runs it. +4. Add focused tests for disallowed LiveComponent usage, allowed `put_toast!/2`, and allowed non-LiveComponent `put_toast/3` usage. +5. Run the relevant test file and Credo check to validate behavior. + + +## Implementation Notes + + + +Implemented `MusicLibrary.Credo.NoLiveComponentPutToast`, registered it in `.credo.exs`, and updated existing `RecordForm` LiveComponent toast calls to `put_toast!/2` so the new rule can be enabled cleanly. Validation: `mix test test/music_library/credo/no_live_component_put_toast_test.exs`; `mix test test/music_library_web/live/collection_live/show_test.exs --max-failures 1`; `mix test test/music_library_web/live/collection_live/index_test.exs --max-failures 1`; `mix credo --strict`. + + + +## Final Summary + + + +Summary: + +- Added `MusicLibrary.Credo.NoLiveComponentPutToast`, a custom Credo warning that detects `put_toast/3` calls inside LiveComponent modules and covers direct, piped, qualified, and captured call forms. +- Registered the custom check in `.credo.exs`. +- Cleaned up existing `RecordForm` LiveComponent toast usage by switching to `put_toast!/2` before returning/pushing patches. +- Added focused Credo check tests for disallowed LiveComponent usage and allowed LiveComponent/non-LiveComponent cases. + +Validation: + +- `mix test test/music_library/credo/no_live_component_put_toast_test.exs` +- `mix test test/music_library_web/live/collection_live/show_test.exs --max-failures 1` +- `mix test test/music_library_web/live/collection_live/index_test.exs --max-failures 1` +- `mix credo --strict` + +Docs: + +- No documentation update needed; `docs/project-conventions.md` already documents the LiveView vs LiveComponent toast helper rule. + diff --git a/lib/music_library/credo/no_live_component_put_toast.ex b/lib/music_library/credo/no_live_component_put_toast.ex new file mode 100644 index 00000000..6233e301 --- /dev/null +++ b/lib/music_library/credo/no_live_component_put_toast.ex @@ -0,0 +1,145 @@ +if Code.ensure_loaded?(Credo) do + defmodule MusicLibrary.Credo.NoLiveComponentPutToast do + @moduledoc false + + use Credo.Check, + base_priority: :high, + category: :warning, + explanations: [ + check: """ + LiveComponents should use `put_toast!/2`, not `put_toast/3`. + + `put_toast!/2` sends the toast request to the parent LiveView, whose + `ShowToast` hook owns the socket update. Calling `put_toast/3` directly + inside a LiveComponent mutates the component socket instead. + """ + ] + + @message "Use `put_toast!/2` instead of `put_toast/3` in LiveComponents." + + def run(source_file, params \\ []) do + issue_meta = IssueMeta.for(source_file, params) + + Credo.Code.prewalk(source_file, &traverse_module(&1, &2, issue_meta)) + end + + defp traverse_module( + {:defmodule, _meta, [_module_name, [do: body]]} = ast, + issues, + issue_meta + ) do + issues = + if live_component_body?(body) do + issues ++ collect_put_toast_issues(body, issue_meta) + else + issues + end + + {ast, issues} + end + + defp traverse_module(ast, issues, _issue_meta), do: {ast, issues} + + defp live_component_body?(body) do + Credo.Code.prewalk(body, &traverse_live_component_use/2, false) + end + + defp traverse_live_component_use({:defmodule, _meta, _args}, found?), do: {nil, found?} + + defp traverse_live_component_use( + {:use, _meta, [{:__aliases__, _alias_meta, [:MusicLibraryWeb]}, :live_component]} = ast, + _found? + ) do + {ast, true} + end + + defp traverse_live_component_use( + {:use, _meta, [{:__aliases__, _alias_meta, [:Phoenix, :LiveComponent]}]} = ast, + _found? + ) do + {ast, true} + end + + defp traverse_live_component_use(ast, found?), do: {ast, found?} + + defp collect_put_toast_issues(body, issue_meta) do + Credo.Code.prewalk(body, &traverse_put_toast_call(&1, &2, issue_meta)) + end + + defp traverse_put_toast_call({:defmodule, _meta, _args}, issues, _issue_meta), + do: {nil, issues} + + defp traverse_put_toast_call({:put_toast, meta, args} = ast, issues, issue_meta) + when is_list(args) and length(args) == 3 do + {ast, issues ++ [issue_for(meta, issue_meta)]} + end + + defp traverse_put_toast_call( + {{:., dot_meta, [_target, :put_toast]}, call_meta, args} = ast, + issues, + issue_meta + ) + when is_list(args) and length(args) == 3 do + {ast, issues ++ [issue_for(call_meta, dot_meta, issue_meta)]} + end + + defp traverse_put_toast_call({:|>, _meta, [_piped_value, call]} = ast, issues, issue_meta) do + case piped_put_toast_meta(call) do + nil -> {ast, issues} + meta -> {ast, issues ++ [issue_for(meta, issue_meta)]} + end + end + + defp traverse_put_toast_call( + {:&, _capture_meta, [{:/, _arity_meta, [{:put_toast, meta, nil}, 3]}]} = ast, + issues, + issue_meta + ) do + {ast, issues ++ [issue_for(meta, issue_meta)]} + end + + defp traverse_put_toast_call( + {:&, _capture_meta, + [ + {:/, _arity_meta, [{{:., dot_meta, [_target, :put_toast]}, call_meta, _args}, 3]} + ]} = ast, + issues, + issue_meta + ) do + {ast, issues ++ [issue_for(call_meta, dot_meta, issue_meta)]} + end + + defp traverse_put_toast_call(ast, issues, _issue_meta), do: {ast, issues} + + defp piped_put_toast_meta({:put_toast, meta, args}) when is_list(args) and length(args) == 2, + do: meta + + defp piped_put_toast_meta({{:., dot_meta, [_target, :put_toast]}, call_meta, args}) + when is_list(args) and length(args) == 2 do + merge_meta(call_meta, dot_meta) + end + + defp piped_put_toast_meta(_call), do: nil + + defp issue_for(meta, issue_meta) do + format_issue( + issue_meta, + message: @message, + trigger: "put_toast", + line_no: meta[:line], + column: meta[:column] + ) + end + + defp issue_for(call_meta, dot_meta, issue_meta) do + issue_for(merge_meta(call_meta, dot_meta), issue_meta) + end + + defp merge_meta(call_meta, dot_meta) do + [ + line: call_meta[:line] || dot_meta[:line], + column: call_meta[:column] || dot_meta[:column] + ] + end + end +end diff --git a/lib/music_library_web/components/record_form.ex b/lib/music_library_web/components/record_form.ex index ba4920fe..f0a6c5da 100644 --- a/lib/music_library_web/components/record_form.ex +++ b/lib/music_library_web/components/record_form.ex @@ -563,10 +563,11 @@ defmodule MusicLibraryWeb.Components.RecordForm do {:ok, record} -> notify_parent({:saved, record}) + put_toast!(:info, gettext("Cover art updated successfully")) + {:noreply, socket |> assign(:cover_search_loading, false) - |> put_toast(:info, gettext("Cover art updated successfully")) |> push_patch(to: socket.assigns.patch)} {:error, _changeset} -> @@ -602,10 +603,9 @@ defmodule MusicLibraryWeb.Components.RecordForm do {:ok, record} <- Records.update_record(socket.assigns.record, params) do notify_parent({:saved, record}) - {:noreply, - socket - |> put_toast(:info, gettext("Record updated successfully")) - |> push_patch(to: socket.assigns.patch)} + put_toast!(:info, gettext("Record updated successfully")) + + {:noreply, push_patch(socket, to: socket.assigns.patch)} else {:error, %Ecto.Changeset{} = changeset} -> {:noreply, assign(socket, form: to_form(changeset))} diff --git a/test/music_library/credo/no_live_component_put_toast_test.exs b/test/music_library/credo/no_live_component_put_toast_test.exs new file mode 100644 index 00000000..406c18be --- /dev/null +++ b/test/music_library/credo/no_live_component_put_toast_test.exs @@ -0,0 +1,82 @@ +defmodule MusicLibrary.Credo.NoLiveComponentPutToastTest do + use Credo.Test.Case + + alias MusicLibrary.Credo.NoLiveComponentPutToast + + setup_all do + {:ok, _apps} = Application.ensure_all_started(:credo) + :ok + end + + test "reports direct put_toast/3 calls in LiveComponents" do + """ + defmodule MusicLibraryWeb.Components.BadToast do + use MusicLibraryWeb, :live_component + + def handle_event("save", _params, socket) do + {:noreply, put_toast(socket, :info, "Saved")} + end + end + """ + |> to_source_file() + |> run_check(NoLiveComponentPutToast) + |> assert_issue(%{ + line_no: 5, + message: "Use `put_toast!/2` instead of `put_toast/3` in LiveComponents.", + trigger: "put_toast" + }) + end + + test "reports piped put_toast/3 calls in LiveComponents" do + """ + defmodule MusicLibraryWeb.Components.BadToast do + use MusicLibraryWeb, :live_component + + def handle_event("save", _params, socket) do + {:noreply, + socket + |> assign(:saved?, true) + |> put_toast(:info, "Saved")} + end + end + """ + |> to_source_file() + |> run_check(NoLiveComponentPutToast) + |> assert_issue(%{ + line_no: 8, + message: "Use `put_toast!/2` instead of `put_toast/3` in LiveComponents.", + trigger: "put_toast" + }) + end + + test "allows put_toast!/2 calls in LiveComponents" do + """ + defmodule MusicLibraryWeb.Components.GoodToast do + use MusicLibraryWeb, :live_component + + def handle_event("save", _params, socket) do + put_toast!(:info, "Saved") + {:noreply, socket} + end + end + """ + |> to_source_file() + |> run_check(NoLiveComponentPutToast) + |> refute_issues() + end + + test "allows put_toast/3 calls outside LiveComponents" do + """ + defmodule MusicLibraryWeb.CollectionLive.Index do + use MusicLibraryWeb, :live_view + + def handle_event("save", _params, socket) do + {:noreply, put_toast(socket, :info, "Saved")} + end + end + """ + |> to_source_file() + |> run_check(NoLiveComponentPutToast) + |> refute_issues() + end +end