Implement representer

[tehsphinx]

Features

• creates representation
• creates mapping
• docker file created

Representation

• based on AST
• removes comments / comment groups
• removes the file level from AST
• top level is package level
• currently JSON format -- think about other format that might be easier to read

Mapping

• JSON format
• simple map of PLACEHOLDER to original name

[tehsphinx]

Next steps:

• I think JSON AST output is too verbose, so I'm going to improve that. Probably something like in the example here.
• reduce AST further to necessary nodes, combining some nodes into one with more information

[junedev]

@tehsphinx In the issue and on Slack there was a preference to have human readable "cleaned up code" as representation instead of some AST serialization. It sounds like you decided otherwise, can you share your reasoning?

Here is a link to the Slack discussion: https://exercism-team.slack.com/archives/CAQP7JL3T/p1635175129080800?thread_ts=1635171130.077500&cid=CAQP7JL3T

[tehsphinx]

Hi @junedev. Didn't see your comment but have come to the same conclusion. I wasn't happy with how hard to read, complicated and also large the representations where. I remembered how easy it would be to turn AST back into Go code after e.g. replacing names and removing comments.

That's why I rewrote the whole thing again. It now parses the AST (drops comments right away), iterates it to replace the names and converts the AST back to Go code. That also removes any differences in whitespace or different formatting, which is nice.

I think these steps are next for me:

• some code review / clean-up from the rewrite
• add more test cases and improve the output where needed

[tehsphinx]

If you have a moment (or anyone else), can you look at the example outputs in the ./representer/tests folders? Let me know if you'd change anything on the output.

[iHiD]

(This is exciting!)

[junedev]

@iHiD Is it possible to get a larger set of example solutions for some exercises in Go so we can check how many distinct representations our representer would produce and potentially identify additional things that need to be normalized?

[junedev]

@tehsphinx I looked through the representations and they looked good to me at first glance but I don't have experience with representers.

One thing that came to mind was whether we need to normalize things like var a string = "" vs var a string vs a := "". Would those produce different representations currently? Should they?

[andrerfcsantos]

@tehsphinx Awesome work, thanks for taking on this task!

The representations seem to be exactly what we want, so it looks great. June talked about the different representations variables can have and whether the representation should be the same. Looking at the examples, I'm wondering the same thing and if things like myvar++ should have the same representation as myvar = myvar + 1 or even myvar+=1. For now, I think there's value in keeping these representations separate. Even though they achieve the same thing in the end, we might want to give different feedback. For instance, we might look at var a string = "" and say there is a shorter way to write this. Same with things like myvar = myvar + 1. The worst that can happen with keeping these representations separate would be us having to repeat feedback on different representations, but I think that might be an OK trade-off for the flexibility it gives us.

I also briefly reviewed the code and it looks good, I found it very easy to follow. Since I'm not well versed in AST black magic, I didn't review that part in detail yet, but everything made sense looking at a glance.

I understand this might not be totally finished yet, but I approved the changes to signal how much I like this PR. This is a solid first iteration.

[ErikSchierboom]

This looks great! Lots of really nice tests too.

[ErikSchierboom]

Looking at the examples, I'm wondering the same thing and if things like myvar++ should have the same representation as myvar = myvar + 1 or even myvar+=1. For now, I think there's value in keeping these representations separate.

I'd also argue that they should be separate representations. Now for some concepts in some exercises, you could argue that you don't want to comment on this in a representation. In that case, you could perhaps build a system where certain transformations are only applied to certain exercises.

[junedev]

@ErikSchierboom Thanks for the feedback!

[tehsphinx]

Thanks for the feedback, everyone! ❤️

From my side we can merge this and then open separate MRs for further iterations, or I can continue to work on this MR, whatever you prefer.

About different variable definition, incrementing a variable, etc:
I think it makes sense to comment on that on earlier exercises and later on not. However my suggestion would be to have the same representer logic for all exercises and rather handle that with the analyzer. In my experience the analyzer is much more exercise specific anyway, so I think it fits better.

If you agree, I'd go on checking on how to generalize these things:

• variable declarations
• variable increment

(In a new branch/MR if you decide to merge this one)

What do think?

[junedev]

@tehsphinx Feel free to merge.

I will follow up with the discussion in the issue.