Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: code action for refactoring FilterFilter #19

Open
wesleimp opened this issue Apr 24, 2023 · 3 comments
Open

feat: code action for refactoring FilterFilter #19

wesleimp opened this issue Apr 24, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@wesleimp
Copy link
Contributor

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

@mhanberg mhanberg added the enhancement New feature or request label Apr 24, 2023
@novaugust
Copy link

novaugust commented May 23, 2023

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

@mhanberg
Copy link
Contributor

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants