ABC size with Ecto query

[Apelsinka223]

Environment

• Credo version (mix credo -v): 0.9.0-rc8
• Erlang/Elixir version (elixir -v): 1.6.1
• Operating system: macOS

Got a warning on this function

def fun() do
  Favorite
  |> where(user_id: ^user.id)
  |> join(:left, [f], t in Template, f.entity_id == t.id and f.entity_type == "template")
  |> join(:left, [f, t], d in Document, f.entity_id == d.id and f.entity_type == "document")
  |> join(:left, [f, t, d], dt in Template, dt.id == d.template_id)
  |> join(:left, [f, t, d, dt], c in Category, c.id == t.category_id or c.id == dt.category_id)
  |> select([f, t, d, dt, c], c)
  |> distinct(true)
  |> Repo.all()
end

Function is too complex (ABC size is 34, max is 30).

Not sure if it is correct behavior, but I didn't expect that Ecto query will be taken into ABC size

[rrrene]

@Apelsinka223 You are right! 👍

[roehst]

@rrrene want me to take a look at how this could work smoother?

[rrrene]

@roehst sure. The challenge will be how we can differentiate between cases where the check should raise an issue and where it doesn't. What are your thoughts on a solution here? White-listing some funs? Looking forward to hearing your thoughts!

[roehst]

The table names f t d dt are free variables in the sense that there is no assignment or formal parameter binding them in the function call, and Credo treats them as function calls. I think (not 100% sure) that without macros that would be enough, but within macros the DSL is not the same language as Elixir and what looks in Elixir as a free variable is something else.

Variable bindings and formal parameters are saved in the accumulator of the AST traversal in var_names.

Options:

• I could try to populate these names with function definitions and other available symbols by reworking the whole traversal and getting var_names closer to the lexical scope. In this manner free variables would not count as function calls because we would resolve free variables to functions. (e.g, adding functions to the var_names as they are defined). Also that would have to account for more exotic use cases when a variable is declared at the module lexical scope (eg, defmodule M do x = 4 end).

• Whitelist Ecto, which may be ugly but may be desirable given the importance of Ecto in the ecosystem

• Just disable the check in the specific file.

• A different option is to get more information into the AST, maybe by using the yecc/leex interface.

Comments, @rrrene ?

[tielur]

I seem to be still getting this issue.

Elixir 1.8.2 (compiled with Erlang/OTP 22)
Credo 1.1.0

[TheFirstAvenger]

Possible approach to this: If Ecto.Query is imported in the file, automatically ignore the problematic ecto functions. That would reduce the false positives generated by whitelisting the ecto function names across the board.