Parse, format and parse again swarm scripts in CI

[xsebek]

The pretty-printer may have bugs in more complex scripts.

We should add formatting and parsing the formatted output of all our swarm scripts in integration tests.

This would make the pretty printer significantly more robust as it would work on our most complex scripts.

This idea was sparked by this bug:

• 1468

[xsebek]

Some other ideas, that may or may not be worth to do in this issue:

• we have lots of scripts in scenario descriptions (YAML) - apply the same testing to them too
• we have a pretty printer of types, but it only gets used in formatting explicit annotations - consider inferring the types of def, let and lambdas and printing those too
• with #1467 fixed, we could use the reformat in editors - this would force the parser to work even on partially valid inputs

[byorgey]

Yes, now that we actually have some rudimentary pretty-printing in place we absolutely need to test that we can round-trip with pretty-printing and parsing.

Another idea would be to test the round-trip in the other direction: have several scripts (it does not need to be all of them) which are formatted according to the output of --format, and we could then test that parsing + pretty-printing them does not modify them at all.

[xsebek]

Another idea would be to use QuickCheck Arbitrary instance and see what kind of code it cooks up.

[byorgey]

Arbitrary instances for large AST types can be problematic, but yes, in general, testing on some kind of randomly generated ASTs is a great idea.

[xsebek]

Turns out we already have a function to round trip parse and format in unit tests:

https://github.com/swarm-game/swarm/blob/4b5ecb05e55103ef2afdddb98e650d09fccb7d7c/test/unit/TestLanguagePipeline.hs#L388-L393

That specific function needs to be updated though, namely to not elaborate and to ignore source location.

[xsebek]

Actually, I would like to have this tested even before the comments (#1467) are fixed. Implementing the other issues from #1571 will be a lot of trial and error and having tests for round tripping would be helpful.

We can workaround the comments by doing the round trip twice, which will remove them the first time and we can still check terms match.

Any tests that do not pass we can mark as expected failure and link an Issue.

[byorgey]

There are actually two directions to think about here.

1. Generating random ASTs, and testing that we can pretty-print and parse them and get back the exact same AST. This is actually the best test of the pretty-printer and parser, but it requires being able to generate random ASTs, which is tricky. However, there are some known techniques and libraries for doing so which we might look into.
2. Taking code, and parsing then pretty-printing it. This round-trip direction is not guaranteed to be the identity function, so the best you can do is to test that it is idempotent, that is, running this round-trip twice is the same as running it once. In other words, once you format the code by parsing + pretty-printing, you have reached a "canonical form" which will no longer change if you parse + pretty-print it. So doing this double round trip will be necessary even if we do preserve comments, because we are not going to preserve all the other formatting.

[xsebek]

@byorgey indeed, I would like to test that the printer round trips and eventually that the canonical forms is how we write the scripts.

Essentially the same as Fourmolu&Restyled do for Haskell, but for Swarm code. At least that is the vision. 🙂

[byorgey]

I would like to test that the printer round trips and eventually that the canonical forms is how we write the scripts.

Ah, I see! Yes, that is a nice idea.