rrrene / credo

A static code analysis tool for the Elixir language with a focus on code consistency and teaching.
http://credo-ci.org/
MIT License
4.91k stars 414 forks source link

NoAmbiguousAliases fails on function param with name alias #1116

Closed dkuku closed 6 months ago

dkuku commented 6 months ago

Precheck

Environment

What were you trying to do?

The code works but credo crashes when checking it. It makes sense because I use an elixir keyword but elixir allows this:

  def generate_index_name(alias) do
    ts = generate_index_with_timestamp()
    "#{alias}-#{ts}"
  end

renaming the param from alias to index fixes the problem

Expected outcome

It should not fail. Maybe only raise a warning.

Actual outcome

14:21:39.838 [error] Task #PID<0.45653.0> started from #PID<0.44911.0> terminating
** (CaseClauseError) no case clause matching: nil
    (credo_checks 0.1.0) lib/credo_checks/no_ambiguous_aliases.ex:44: CredoChecks.NoAmbiguousAliases.find_alias/2
    (elixir 1.16.1) lib/macro.ex:640: anonymous fn/4 in Macro.do_traverse_args/4
    (stdlib 5.2) lists.erl:1706: :lists.mapfoldl_1/3
    (elixir 1.16.1) lib/macro.ex:605: Macro.do_traverse/4
    (stdlib 5.2) lists.erl:1706: :lists.mapfoldl_1/3
    (elixir 1.16.1) lib/macro.ex:605: Macro.do_traverse/4
    (stdlib 5.2) lists.erl:1706: :lists.mapfoldl_1/3
    (stdlib 5.2) lists.erl:1707: :lists.mapfoldl_1/3
Function: &:erlang.apply/2
    Args: [#Function<1.15024489/1 in CredoChecks.NoAmbiguousAliases.do_run_on_all_source_files/3>, [%SourceFile<apps/customers_core/lib/customers_core/elastic_search/index.ex>]]
rrrene commented 6 months ago

@dkuku This does not look like an error caused by Credo.

Since the name of the check module (CredoChecks.NoAmbiguousAliases) starts with CredoChecks, I would guess that the failure comes from a plugin?

Judging from the error message, we can conclude that in that plugin there is a file named lib/credo_checks/no_ambiguous_aliases.ex and that there is a case statement on line 44, which does not seem to account for an occurence of nil.

rrrene commented 6 months ago

And the reason why this is happening is probably that the AST representation for an alias call is

{:alias, meta, [_|_] = args}

where args represents the parameters given to alias (like the module name and options).

And the representation for the parameter in your code snippet is

{:alias, meta, nil}

The function in which this happens is named find_alias/2, so I guess the plugin authors are simply not accounting for variables, parameters and likely module attributes named alias ...

dkuku commented 6 months ago

Oh, sorry. You're right. It's a plugin.

On Mon, 26 Feb 2024, 16:21 René Föhring, @.***> wrote:

And the reason why this is happening is probably that the AST representation for an alias call is

{:alias, meta, [|] = args}

where args represents the parameters given to alias (like the module name and options).

And the representation for the parameter in your code snippet is

{:alias, meta, nil}

The function in which this happens is named find_alias/2, so I guess the plugin authors are simply not accounting for variables, parameters and likely module attributes named alias ...

— Reply to this email directly, view it on GitHub https://github.com/rrrene/credo/issues/1116#issuecomment-1964564144, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG4X455FFNRZCVLVKGQKGLYVSZCJAVCNFSM6AAAAABD2JVNTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRUGU3DIMJUGQ . You are receiving this because you were mentioned.Message ID: @.***>