elixir-analyzer icon indicating copy to clipboard operation
elixir-analyzer copied to clipboard

Elixir track snake_case misleading automatic recomendation

Open PQ4NOEH opened this issue 2 years ago • 4 comments

When the elixir track on the exercise log level when submitted I have got what I think is a wrong automatic recommendation about snake_case. The details of the recommendations are:

image

PQ4NOEH avatar Oct 01 '22 07:10 PQ4NOEH

Thank you for the feedback! I see what's going on. Do we agree that log_Label is indeed not snake case because of the L? However, what the analyzer is proposing is not snake case either, it should be log_label.

Would you be interested in helping fixing it? I can guide you and I would accept the contribution valid for hacktoberfest.

jiegillet avatar Oct 01 '22 12:10 jiegillet

Yes, I agree log_Label is not correct either. I am happy on helping to fix it, please tell me what should I do.

PQ4NOEH avatar Oct 01 '22 16:10 PQ4NOEH

Nice!

The culprit is here: https://github.com/exercism/elixir-analyzer/blob/9b380f6d115b67318d0930bf40672243921c93b6/lib/elixir_analyzer/exercise_test/common_checks/variable_names.ex#L71-L80

We might be able to get away with a |> String.replace("__", "_") to fix this particular problem, but maybe you can find something more robust? Like the comments say, we only care about identifiers that already compile.

We will need to add tests (starting with "log_Label") around here https://github.com/exercism/elixir-analyzer/blob/9b380f6d115b67318d0930bf40672243921c93b6/test/elixir_analyzer/exercise_test/common_checks/function_names_test.exs#L51-L71

You can create a helper function to check the result like get_result in the following test, or even make get_result available to the whole module to make things more concise.

Does that make sense?

jiegillet avatar Oct 01 '22 23:10 jiegillet

Yes, it does make sense. I do this one

PQ4NOEH avatar Oct 02 '22 07:10 PQ4NOEH

@PQ4NOEH are you still interested in taking this one?

jiegillet avatar Nov 05 '22 13:11 jiegillet

I'm having a look into this :slightly_smiling_face:

dmarcoux avatar Nov 23 '22 16:11 dmarcoux