exercism / elixir-representer

GNU Affero General Public License v3.0
10 stars 6 forks source link

Quote in the representation? #39

Closed jiegillet closed 2 years ago

jiegillet commented 2 years ago

Here, I found quote marks in the representation around "PLACEHOLDER_2":

https://github.com/exercism/elixir-representer/blob/f48b44318792ffef4bd0085f2614b97a550e137f/test_data/local_function_calls/expected_representation.txt#L20-L25

Is this on purpose?

angelikatyborska commented 2 years ago

That's just what Macro.to_string does. I suspect we might be confusing it because after changing the names to placeholders, the AST no longer represents valid code. Function names cannot start with an uppercase letter.

I wonder if we should make an extra effort and do context-aware placeholder names? PascalCase for modules, snake_case for the rest.

jiegillet commented 2 years ago

Ah, I see, that's possible. An easy way might be to use lowercase placeholder everywhere and capitalize them all after Macro.to_string. The compiler isn't happy with lowercase module names, but Macro.to_string doesn't seem to mind.

iex(19)> quote do
...(19)> defmodule placeholder do end
...(19)> end
{:defmodule, [context: Elixir, import: Kernel],
 [{:placeholder, [], Elixir}, [do: {:__block__, [], []}]]}
iex(20)> |> Macro.to_string          
"defmodule(placeholder) do\n  \nend"
jiegillet commented 2 years ago

I asked the question on Slack:

I have a question about representers. Are they supposed to produce runnable code? Or just identifiable? For example in Elixir, functions may not start with capital letters so PLACEHOLDER_1 is illegal

Jeremy replied

The only requirement is that its identifiable. In Ruby, I followed normal rules, so variables are lowercase and constants uppercase, but really just because it made it easier for me to read if I was looking at something. Compilable/runnable might be a bonus for things like doing multi-passes on the code over multiple normalisations, but that's entirely down to y'all internally to the tooling :slightly_smiling_face:

What would you think of making the representation legitimate Elixir code? It would simply be a matter of capitalizing the module names and keeping all other variable names lowercase.

angelikatyborska commented 2 years ago

I would love for the representation to be valid Elixir code that compiles if the source compiles.