credo icon indicating copy to clipboard operation
credo copied to clipboard

Add a check for assignment in with conditions

Open ryanwinchester opened this issue 7 months ago • 2 comments

Some folks hate this, and it can trip people up.

ryanwinchester avatar Jul 10 '25 00:07 ryanwinchester

Hi, thx for putting this together!

I am in favor of merging it, except there is one case not covered:

defmodule CredoSampleModule do
  def some_function(id) do
    with {:ok, user} <- get_user(id),
         token = ExternalLib.get_user_token(user.id),
         {:ok, profile} <- get_profile(user, token) do
      {:ok, %{user: user, profile: profile}}
    else
      {:error, :something}
    end
  end
end

Your check says "there can't be any assignments, ever" and I would love to have a configuration option to say: "Assignments only in the middle of the clauses, if their return value is needed for the chain" (token in my made-up example).

Because how would you model the above example otherwise?

rrrene avatar Jul 10 '25 04:07 rrrene

Because how would you model the above example otherwise?

with <- for consistency.

defmodule CredoSampleModule do
  def some_function(id) do
    with {:ok, user} <- get_user(id),
         token <- ExternalLib.get_user_token(user.id),
         {:ok, profile} <- get_profile(user, token) do
      {:ok, %{user: user, profile: profile}}
    else
      {:error, :something}
    end
  end
end

but also not confusing newcomers if they switch it to {:ok, token} and forget to change the = to <-

ran into almost the exact same scenario recently with someone who just started with Elixir

I would love to have a configuration option to say: "Assignments only in the middle of the clauses, if their return value is needed for the chain" (token in my made-up example).

There is already a check for the first and last condition in the with being = (but okay in the middle): https://github.com/rrrene/credo/blob/master/lib/credo/check/refactor/with_clauses.ex

I'm personally okay with = anywhere in with conditions, but I know several people who don't like them at all. So, I'm willing to stick with <- for some projects, but when doing so, would like an optional rule to enforce it

ryanwinchester avatar Jul 10 '25 21:07 ryanwinchester