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

Using Kernel.match?/2 to check if variable contains anonymous function AST increases size by 5 in Credo.Check.Refactor.ABCSize #1161

Open
Eiji7 opened this issue Nov 5, 2024 · 0 comments

Comments

@Eiji7
Copy link
Contributor

Eiji7 commented Nov 5, 2024

Environment

  • Credo version (mix credo -v):
$ mix credo -v
1.7.10-ref.main.1d74cf6+uncommittedchanges
  • Erlang/Elixir version (elixir -v):
$ elixir -v
Erlang/OTP 27 [erts-15.1] [source] [64-bit] [smp:32:32] [ds:32:32:10] [async-threads:1] [jit:ns]

Elixir 1.17.3 (compiled with Erlang/OTP 27)
  • Operating system:

Gentoo Linux using desktop/plasma profile for version 23.0 and with kernel version 6.6.41.

What were you trying to do?

Replaced if condition by true for debug ABCSize check

if match?({:fn, _meta, _args}, ast) do
  # …
end

Expected outcome

No idea how much points are expected, but 5 is definitely too much as just 6 lines with such a simple match? call would reach a limit.

The ABC size describes a metric based on assignments, branches and conditions.

  1. Since ast is assigned above if condition I guess it should not count, right?
  2. Branches are dot calls (like: mod.fun(…), right? Maybe it counts as +1 because of AST expansion to Kernel.match?(…) form …
  3. Kernel.match?/2 is not a condition (if, unless, for, try, case, cond, and, or, &&, ||)

If that's correct then it should count as +1 for size. Not sure from where 5 comes - it does not match even if we count skipped patterns (_some_name) and ast with match? call would give at most 4 points. If you agree it would be awesome to count is as just +1 size, but as above I may be wrong here …

Actual outcome

Such a change decreases size by 5:

# after
if true do
  # …
end
@Eiji7 Eiji7 changed the title Using Kernel.match?2 to check if variable contains anonymous function AST increases size by 5 in Credo.Check.Refactor.ABCSize Using Kernel.match?/2 to check if variable contains anonymous function AST increases size by 5 in Credo.Check.Refactor.ABCSize Nov 5, 2024
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

No branches or pull requests

1 participant