Skip to content

Conversation

ryanwinchester
Copy link

We generally don't want this and it can trip people up.

@rrrene
Copy link
Owner

rrrene commented Jul 10, 2025

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?

@ryanwinchester
Copy link
Author

ryanwinchester commented Jul 10, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants