Add a check for assignment in with conditions

.credo.exs

@@ -198,6 +198,7 @@
           {Credo.Check.Refactor.PassAsyncInTestCases, []},
           {Credo.Check.Refactor.PipeChainStart, []},
           {Credo.Check.Refactor.RejectFilter, []},
+          {Credo.Check.Refactor.WithAssignment, []},
           {Credo.Check.Refactor.VariableRebinding, []},
           {Credo.Check.Warning.LazyLogging, []},
           {Credo.Check.Warning.LeakyEnvironment, []},

lib/credo/check/refactor/with_assignment.ex

@@ -0,0 +1,88 @@
+defmodule Credo.Check.Refactor.WithAssignment do
+  use Credo.Check,
+    base_priority: :high,
+    category: :refactor,
+    explanations: [
+      check: ~S"""
+      `with` statements are designed for chaining pattern matches using the `<-` operator.
+      Using regular assignment (`=`) inside a `with` block defeats its purpose and can be misleading.
+
+      When you use `=` in a `with` block, the assignment always succeeds and doesn't provide
+      the early-exit behavior that makes `with` useful.
+
+      Example of incorrect usage:
+
+          with user = get_user(id),
+               {:ok, profile} <- get_profile(user),
+               settings = get_settings(user) do
+            {:ok, %{user: user, profile: profile, settings: settings}}
+          end
+
+      In this example, `user = get_user(id)` and `settings = get_settings(user)` will always
+      succeed, even if the functions return `nil` or an error tuple.
+
+      This should be refactored to either:
+
+      1. Move assignments outside the `with`:
+
+          user = get_user(id)
+          settings = get_settings(user)
+
+          with {:ok, profile} <- get_profile(user) do
+            {:ok, %{user: user, profile: profile, settings: settings}}
+          end
+
+      2. Or use pattern matching if you need to handle failures:
+
+          with {:ok, user} <- get_user(id),
+               {:ok, profile} <- get_profile(user),
+               {:ok, settings} <- get_settings(user) do
+            {:ok, %{user: user, profile: profile, settings: settings}}
+          end
+      """
+    ]
+
+  @message_assignment_in_with "Avoid using `=` in `with` blocks. Use `<-` for pattern matching or move assignments outside the `with`."
+
+  @doc false
+  @impl true
+  def run(%SourceFile{} = source_file, params) do
+    issue_meta = IssueMeta.for(source_file, params)
+    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
+  end
+
+  defp traverse({:with, _meta, [_, _ | _] = clauses_and_body} = ast, issues, issue_meta)
+       when is_list(clauses_and_body) do
+    # Split off the last element which should be the body (keyword list with :do)
+    {maybe_clauses, [maybe_body]} = Enum.split(clauses_and_body, -1)
+
+    if Keyword.keyword?(maybe_body) and Keyword.has_key?(maybe_body, :do) do
+      new_issues = check_clauses_for_assignments(maybe_clauses, issue_meta)
+      {ast, new_issues ++ issues}
+    else
+      {ast, issues}
+    end
+  end
+
+  defp traverse(ast, issues, _issue_meta) do
+    {ast, issues}
+  end
+
+  defp check_clauses_for_assignments(clauses, issue_meta) do
+    Enum.flat_map(clauses, fn clause ->
+      case clause do
+        {:=, meta, _args} ->
+          [
+            format_issue(issue_meta,
+              message: @message_assignment_in_with,
+              line_no: meta[:line],
+              trigger: "="
+            )
+          ]
+
+        _ ->
+          []
+      end
+    end)
+  end
+end

test/credo/check/refactor/with_assignment_test.exs

@@ -0,0 +1,100 @@
+defmodule Credo.Check.Refactor.WithAssignmentTest do
+  use Credo.Test.Case
+
+  @described_check Credo.Check.Refactor.WithAssignment
+
+  test "it should NOT report when using <- in with blocks" do
+    """
+    defmodule CredoSampleModule do
+      def some_function(id) do
+        with {:ok, user} <- get_user(id),
+             {:ok, profile} <- get_profile(user) do
+          {:ok, %{user: user, profile: profile}}
+        end
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
+  test "it should report when using = in with blocks" do
+    """
+    defmodule CredoSampleModule do
+      def some_function(id) do
+        with user = get_user(id),
+             {:ok, profile} <- get_profile(user) do
+          {:ok, %{user: user, profile: profile}}
+        end
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "="
+    end)
+  end
+
+  test "it should report multiple = assignments in with blocks" do
+    """
+    defmodule CredoSampleModule do
+      def some_function(id) do
+        with user = get_user(id),
+             {:ok, profile} <- get_profile(user),
+             settings = get_settings(user) do
+          {:ok, %{user: user, profile: profile, settings: settings}}
+        end
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> assert_issues(fn issues ->
+      assert length(issues) == 2
+      assert Enum.at(issues, 0).line_no == 3
+      assert Enum.at(issues, 1).line_no == 5
+    end)
+  end
+
+  test "it should NOT report regular assignments outside with blocks" do
+    """
+    defmodule CredoSampleModule do
+      def some_function(id) do
+        user = get_user(id)
+
+        with {:ok, profile} <- get_profile(user) do
+          settings = get_settings(user)
+          {:ok, %{user: user, profile: profile, settings: settings}}
+        end
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
+  test "it should handle with blocks with else clauses" do
+    """
+    defmodule CredoSampleModule do
+      def some_function(id) do
+        with user = get_user(id),
+             {:ok, profile} <- get_profile(user) do
+          {:ok, %{user: user, profile: profile}}
+        else
+          {:error, reason} -> {:error, reason}
+        end
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "="
+    end)
+  end
+end