credo icon indicating copy to clipboard operation
credo copied to clipboard

RedundantWithClauseResult false positive when containing a map match

Open idkCpp opened this issue 11 months ago • 2 comments

Environment

  • Credo version (mix credo -v): 1.7.11
  • Erlang/Elixir version (elixir -v): Elixir 1.18.2 (compiled with Erlang/OTP 27)
  • Operating system: Arch-Linux

What were you trying to do?

defmodule Test do
  @moduledoc false

  def test do
    with :ok <- check(),
         {:ok, %{id: id}} <- much_data() do
      {:ok, %{id: id}}
    end
  end

  def check, do: :ok

  def much_data do
    %{
      id: "foo",
      secret_data: "do not send to caller"
    }
  end
end 

While it is technically correct (the best kind of correct), that the last match expression and the do-block expression are the same, they are functionally very much not the same. When employing a map-match in the with clause, additional data is discarded.

Therefore I would suggest this check to not report when the expression contains a map.

I am also open to the idea, that this construct is not communicating its intent (of discarding data) very clearly. I personally would not classify that as priority "high" though and it certainly is not "redundant".

Expected outcome

"found no issues"

Actual outcome

  Refactoring opportunities                                                     
┃ 
┃ [F] ↗ Last clause in `with` is redundant.
┃       lib/test.ex:5:5 #(Test.test)

idkCpp avatar Mar 12 '25 10:03 idkCpp

Thx for reporting! :+1:

rrrene avatar Mar 18 '25 04:03 rrrene