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 415 forks source link

Detect ambiguity in function calls without parentheses #1145

Open Artur-Sulej opened 1 month ago

Artur-Sulej commented 1 month ago

Precheck

Environment

What were you trying to do?

I'm running mix credo --strict and getting this warning:

warning: missing parentheses for expression following "do:" keyword. Parentheses are required to solve ambiguity inside keywords.

This error happens when you have function calls without parentheses inside keywords. For example:

    function(arg, one: nested_call a, b, c)
    function(arg, one: if expr, do: :this, else: :that)

In the examples above, we don't know if the arguments "b" and "c" apply to the function "function" or "nested_call". Or if the keywords "do" and "else" apply to the function "function" or "if". You can solve this by explicitly adding parentheses:

    function(arg, one: if(expr, do: :this, else: :that))
    function(arg, one: nested_call(a, b, c))

Ambiguity found at:
└─ nofilename:5

It happens because of this piece of code:

foo do: rename table(:bar), :baz, to: :qux

Expected outcome

I'd like credo to detect this ambiguity issue and add a suggestion about it to its output. That would be especially useful because the warning doesn't contain a correct path to the problematic code.

Actual outcome

This issue doesn't appear in the output.

rrrene commented 1 month ago

The fact that the filename is nofilename on this is definitely bad and we should fix that πŸ‘ (this is completely Credo's fault, I believe). Could you provide an Elixir module that produces this warning?

I'd like credo to detect this ambiguity issue and add a suggestion about it to its output.

On this I am sorry, because we can not re-implement compiler checks/warnings as standard checks in Credo. Doubling the compiler's functionality would be like implementing our own formatter.

This of course does not say that this wouldn't be a great custom check or plugin, which we could also feature in the docs on how to implement custom checks πŸ’―