credo-language-server icon indicating copy to clipboard operation
credo-language-server copied to clipboard

feat: code action for refactoring FilterFilter

Open wesleimp opened this issue 1 year ago • 3 comments

Reference: https://hexdocs.pm/credo/Credo.Check.Refactor.FilterFilter.html

wesleimp avatar Apr 24 '23 13:04 wesleimp

the credo example docs make it look like it'll always be easy to combine a filter-filter, buuut there's a reason styler doesn't implement this rule...

# Not so easy to rewrite
stuff
|> Enum.filter(fn 
  x when is_list(x) -> good_list?(x)
  %{} = map -> keep_map?(map)
  _ -> true
end)
|> Enum.filter(fn  
  {:ok, good_stuff} -> true
  {:error, try_again} -> remote_check?(try_again)
  _ -> false
end)

without the knowledge of a programmer, the only way i've come up with for an ast rewrite to always solve this problem would be something semi hideous like:

stuff
|> Enum.filter(fn x ->
  a = fn 
    x when is_list(x) -> good_list?(x)
    %{} = map -> keep_map?(map)
    _ -> true
  end

  b = fn
    {:ok, good_stuff} -> true
    {:error, try_again} -> remote_check?(try_again)
    _ -> false
  end

  a.(x) && b.(x)
end)

at which point it's like, eugh. you lost so much in readability and gained whatever in list traversal speed. what would joe armstrong say?

that said, you could certainly come up with smarter and better rewrites for trivial situations like what credo presented. i'm just showing the worst-case fallback

so, that's why styler doesn't implement / care about this rule and ones like it

novaugust avatar May 23 '23 18:05 novaugust

yeah, would probably be best to return a "sorry not sorry" response if the callback was complex like in your example, but combine them otherwise.

mhanberg avatar May 23 '23 19:05 mhanberg

if you had the non-refactorable version, credo should be like "wut plz stop" 🤣

mhanberg avatar May 23 '23 19:05 mhanberg