Automatic code formatting using Spotless

[petervdonovan]

I have gotten feedback from multiple people that the requirement that LF tests be formatted has the potential to be annoying if it is not documented properly (and perhaps automated or hooked into some established workflow).

Unfortunately, using client-side Git hooks for the formatter seems like it would require each person to manually add the Git hook (for some reason, client-side Git hooks apparently cannot be tracked). If we were to use a server-side Git hook, then people would find out about formatting problems after they have already pushed their work, which can be annoying.

Because there has already been plenty of talk about adding Spotless to our regular workflow (#1052), it seems to me like the overhead of dealing with formatting tools can be minimized by just using the Spotless Gradle plugin for both LF formatting and Java formatting.

If we cannot quickly reach consensus about Java formatting, I propose to comment Java formatting out of this PR so that we at least have an established way of invoking lff. Otherwise, people would have to get used to directly invoking the runLff task, and then when we have a Java formatter or linter, they would have to get used to that again. Additionally, Spotless has advantages over direct invocation of the formatter. For example, it only runs the formatter on files that have changed.

Development of a comprehensive CONTRIBUTING.md file and building of consensus around coding standards have already been started in #478; therefore, they are both non-goals of this PR.

[lhstrh]

Looks great! Left some comments.

[lhstrh]

Only minor nitpicks

[cmnrd]

I noticed that this PR created a new top-level directory buildSrc. What is this directory used for? We are currently trying to simplify the repository structure (see #1276) and adding a new directory seems counter productive. Is there a way to move the contents to other locations and integrate with existing files?

[petervdonovan]

I noticed that this PR created a new top-level directory buildSrc. What is this directory used for?

I do not think it would be a good idea to get rid of buildSrc. Here is an excerpt from a conversation I had with Marten:

Gradle files are like a DSL that is embedded in Groovy. This DSL is pretty limited. Consequently, you are allowed to write custom logic in Java instead of in Gradle files, and then import that (into your Gradle build scripts). This custom imperative code is not part of the core of the project; instead, it is part of the build system that builds the project. Therefore, it belongs in a buildSrc directory, and not an src directory

Also, regarding this particular buildSrc directory:

It's totally a hack... because my goal was to invoke Lff, which is indeed what we are trying to build.
But it works just fine because it invokes the jar of Lff instead of depending directly on the Java code. The jar is built without invoking the formatting step.

So in short, the my general approach to the problem at hand was a little weird -- I wrote Java code that executes a JAR that has to be built separately in order to avoid a circular dependency. However, unless/until we decide to use an entirely different strategy, it is indeed appropriate to keep the code in buildSrc.

Edit: I should also mention that buildSrc is the standard name for such a directory

[cmnrd]

Ok, I understand now what buildSrc means. Thanks for explaining. So ideally we would move all the org.lflang.* packages into a top-level src/ directory or out of this repo. Then we would have a structure where we have bin/ build/ buildSrc/ lib/ rsc/ in the top level.

I guess invoking the jar is part of the reason why formatting can be slow. I don't quite see the circular dependency though. Could you explain a bit? Since we refactor the repo and the build anyway, we should aim at resolving this problem in the process.

[petervdonovan]

I don't quite see the circular dependency though. Could you explain a bit?

Well, there is no circular dependency, but if buildSrc invoked the Java code directly instead of the JAR then there would be a circular dependency because usually (maybe not for us but in general), buildSrc has to be compiled before compiling anything else -- this is just the way Gradle sets it up.

[cmnrd]

Ok, I see now. Thanks!